diff --git a/COHERENCE.md b/COHERENCE.md index e9cb970..c12acfc 100644 --- a/COHERENCE.md +++ b/COHERENCE.md @@ -570,9 +570,11 @@ descriptions, indexed by `M-x help`. It needed **no Rust** — the data was all reachable from Lua, and even the settings completion source is a Lua function through `CompletionSource::Custom`. -**What is still missing** is itemized below and unchanged by that stage: -`Command` has no title/category/aliases/flags/arg-schema; the predicate -is still never evaluated; M-x rows are still bare name strings; the Rust +**What is still missing** is itemized below. Discovery Stage 2 took one +item — **M-x rows now carry each command's description** — and the rest +is unchanged by both stages: `Command` has no +title/category/aliases/flags/arg-schema; the predicate +is still never evaluated; the Rust help layer is still orphaned (Stage 1 funnels every command through one Lua seam so the eventual migration is enumerated per subject rather than per call site); **packages** have no discovery surface, and workers, @@ -621,12 +623,21 @@ the sharpest instance of §1.1.** M-x filtering, or the menu. The doc comment's claim that "the command palette (T M2.7) uses it to gray out unavailable entries" describes something that never shipped. -- **M-x shows bare name strings.** `CompletionSource::Commands` returns - `Vec` of names; the wire type `MinibufferPrompt.candidates` - is `Vec` (`pmacs-protocol/src/message.rs:994-1006`). No - description, no keybinding, no category alongside candidates — while - `CompletionPopupRow` (`:1231`) already carries `kind` and `detail`, - proving richer rows are a solved wire problem in this codebase. +- **M-x rows carry a description — Discovery Stage 2 (protocol v23).** + `CompletionSource::Commands` still returns `Vec` of names, but + the row the user reads is no longer one. The GPU receives + `InstanceMessage::MinibufferPromptRows` — `MinibufferRow { label, + detail }`, a new type rather than a borrowed `CompletionPopupRow`, + whose `kind` is an LSP code with no honest value for a command — and + the grid TUI renders `[name — description]` inline from the registry + **in-process**, since `src/editor.rs` never consumed the wire variant + at all. The bump is additive: `MinibufferPrompt` is FROZEN and still + sent to every `12..=22` peer (postcard is positional, so widening it + would mis-decode there rather than be ignored), and exactly one of the + two variants reaches any peer. + **What is still missing here:** no keybinding and no category + alongside the candidate — those wait on `Command` gaining the fields + at all. - **The entire Rust help layer is orphaned** (§1.1). Consequence: two parallel `*help*` implementations exist — `help.rs`'s cross-referenced renderer and the Lua `show_help_text` in @@ -684,6 +695,8 @@ the sharpest instance of §1.1.** provenance in the config registry, (c) a dozen interactive commands and richer M-x candidate rows over introspection that **already exists**. This is the highest payoff-per-effort concern in the document. +*(c) is done: Stage 1 shipped the command family, Stage 2 the richer +rows. (a) and (b) remain.* --- diff --git a/docs/active-work.md b/docs/active-work.md index b13f606..3323bca 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -265,7 +265,7 @@ also removed: this branch's "R8 NEEDS A LANE" investigation block, and durable facts are in the retired registry row and the handoff §6 census. -## Discovery Stage 2 — BRANCHED, pre-implementation +## Discovery Stage 2 — IMPLEMENTED, no PR yet **Written with the lane's first commit**, per the standing correction from #171 and #215. @@ -299,9 +299,34 @@ tip** — the ref, not a SHA. Recover with `format!(" [{cand}]")` (`src/editor.rs:5484`), so its half is a local formatting change reading the registry directly. A multi-row TUI chooser is explicitly NOT this lane. -- **Gates:** `scripts/gate --protocol --acceptance ` — - the strengthened two-configuration sweep, which is what `--protocol` - exists for. +- **Gates:** `scripts/gate --protocol --acceptance + discovery_stage2_acceptance` — the strengthened two-configuration + sweep, which is what `--protocol` exists for. +- **IMPLEMENTED.** `PROTOCOL_VERSION` is 23, + `ADVERTISED_PROTOCOL_VERSION` is untouched at 20. New suite + `tests/discovery_stage2_acceptance.rs`; the daemon half is + `crdt`-gated (a semantic session is necessarily a text replica) and + runs one daemon serving a v22 and a v23 session simultaneously. +- **The freeze is enforced by LITERAL byte fixtures**, not a round-trip + — `minibuffer_prompt_v12_wire_bytes_are_frozen` in `src/protocol.rs`, + the first such fixture in this repo. Bite-verified: reordering two + fields of `MinibufferPrompt` leaves + `minibuffer_prompt_round_trips_through_postcard` **passing** and fails + the fixture, which is exactly the hazard a round-trip cannot see. +- **Version assertions updated (five, each read before editing):** + `src/protocol.rs` — the `PROTOCOL_VERSION == 22` tripwire (renamed + `protocol_version_is_twenty_three_for_minibuffer_prompt_rows`) and + `supported_protocol_versions_resume_ladder_on_v6_floor`'s + accepted/rejected ranges; `tests/statusline_segments_acceptance.rs` + (version + supported range + the `!supported` ceiling); + `tests/bottom_panel_stage2b_gpu_acceptance.rs`; + `tests/vterm_stage3_acceptance.rs`. **No `ADVERTISED_PROTOCOL_VERSION` + assertion fired**, which is the pin doing its job. +- **No cross-version cache test, deliberately** (framing §3.2/§6): + `SemanticRenderState::for_peer` bakes the negotiated version in at + attach and is dropped at detach, so a cache cannot span two versions. + A test for an impossible condition passes forever while teaching the + next reader that the hazard is real. ## LSP LaTeX coverage — IMPLEMENTED, gates green, no PR yet diff --git a/docs/agent-handoff.md b/docs/agent-handoff.md index 65e7374..ca53c70 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -2598,11 +2598,30 @@ cannot advertise 21 without stranding existing v20 clients before `AttachRequest`. v15 = `CompletionPopup` + `StatusFacts.message`; v16 = `ThemeFacts`; v17 = `FontFacts`; v18 = `StatuslineSegments`; v19 = the vterm terminal family; v20 = semantic `SessionBootstrapRequest` plus appended -`InitialTargetResult`; v21 reserves the panel frame/event family. New wire +`InitialTargetResult`; v21 reserves the panel frame/event family; +v22 = `LineWrapFacts`; v23 = `MinibufferPromptRows`. New wire surface ⇒ bump + both-frontends support + acceptance. An APPENDED variant must be guarded by a byte pin on the PREVIOUS final variant — its own round-trip cannot detect a discriminant shift. +**A SUPERSEDED variant can be frozen rather than widened, and v23 is the +first case.** Discovery Stage 2 needed richer minibuffer rows. +Widening `MinibufferPrompt` in place was not an option — postcard +encodes fields positionally, so every v12–v22 peer would **mis-decode** +the bytes rather than ignore them — and gating the widened form at +`>= 23` would have left those peers with **no minibuffer message at +all**, because there would have been only one variant to gate. +Compatibility requires the old shape to still exist *and still be sent*. +So `MinibufferPrompt` is retained unchanged for `12..=22`, +`MinibufferPromptRows` is appended for `>= 23`, and the daemon gate is a +**range on both sides** so exactly one variant reaches any peer. +Two consequences worth carrying forward: a frozen variant needs a +**literal byte fixture** (`assert_eq!(encoded, LEGACY_BYTES)`), because a +round-trip encodes and decodes with the same types and so freezes +nothing; and the CLOSE message must use the same variant family as the +OPEN, or a session closed by the other family's clear leaves its surface +on screen forever. + **Fake LSP** (`src/bin/pmacs_fake_lsp.rs`) modes: `fullonly`, `rangeonly`, `rangeonly16` (UTF-16 + fail-closed bounds validation), `sighelp`. Use these for capability-matrix tests, not real servers. diff --git a/pmacs-gpu/src/main.rs b/pmacs-gpu/src/main.rs index 52a2a29..cbb58e3 100644 --- a/pmacs-gpu/src/main.rs +++ b/pmacs-gpu/src/main.rs @@ -44,9 +44,10 @@ use pmacs_protocol::{ CompletionPopupRow, CrdtOp, Decoration, DecorationKind, DecorationSegment, FrontendId, InlineAdornment, InstanceMessage, InstanceSignal, Key as ProtocolKey, LineNumberMode, MAX_STATUSLINE_FACE_BYTES, MAX_STATUSLINE_PROVIDERS, MAX_STATUSLINE_SEGMENT_BYTES, - MAX_STATUSLINE_TOTAL_TEXT_BYTES, MenuPromptRow, Modifiers, MouseButton as ProtocolMouseButton, - MouseKind as ProtocolMouseKind, PointerKind, SelectionSnapshot, StatuslineSegment, - StyleSegment, StyleSpan, TAB_STOP_COLUMNS, TerminalFrame, UnderlineStyle, + MAX_STATUSLINE_TOTAL_TEXT_BYTES, MenuPromptRow, MinibufferRow, Modifiers, + MouseButton as ProtocolMouseButton, MouseKind as ProtocolMouseKind, PointerKind, + SelectionSnapshot, StatuslineSegment, StyleSegment, StyleSpan, TAB_STOP_COLUMNS, TerminalFrame, + UnderlineStyle, cell::{Color as CellColor, Style as CellStyle}, is_builtin_pair_char, is_modeline_face_name, panel::{PANEL_MIN_VERSION, PanelFrame, PanelFramePayload}, @@ -2259,16 +2260,25 @@ struct SearchPromptLocal { invalid: bool, } -/// The live minibuffer (Q#MB1, protocol v12), mirrored from a -/// `MinibufferPrompt` whose `prompt` was `Some`. The prompt+input draw -/// in the bottom band with a caret; `candidates` (a windowed slice) feed -/// the dropdown. +/// The live minibuffer (Q#MB1, protocol v12), mirrored from whichever +/// minibuffer variant this session's negotiated version carries, when +/// its `prompt` was `Some`. The prompt+input draw in the bottom band +/// with a caret; `rows` (a windowed slice) feed the dropdown. +/// +/// **One local shape for two wire variants.** A `>= 23` daemon sends +/// `MinibufferPromptRows` with per-row details; a `12..=22` daemon sends +/// the frozen `MinibufferPrompt` with bare strings, which land here as +/// rows whose `detail` is `None`. Both are live: this binary offers its +/// own `PROTOCOL_VERSION` only when the daemon advertises the current +/// baseline, and echoes an older baseline verbatim — so an older daemon +/// still negotiates an older session, and the legacy arm is reachable +/// rather than dead code. #[derive(Clone, Debug, PartialEq)] struct MinibufferLocal { prompt: String, input: String, cursor: u32, - candidates: Vec, + rows: Vec, selected: Option, total: u32, } @@ -5085,7 +5095,10 @@ impl State { None } // Q#MB1 — the minibuffer prompt/input/candidates. `prompt: - // None` closes it. + // None` closes it. This is the FROZEN legacy variant, which + // only a `12..=22` daemon sends; its candidates carry no + // detail, so they become rows with `detail: None` and render + // exactly as they did before v23. InstanceMessage::MinibufferPrompt { prompt, input, @@ -5098,7 +5111,38 @@ impl State { prompt, input, cursor, - candidates, + rows: candidates + .into_iter() + .map(|label| MinibufferRow { + label, + detail: None, + }) + .collect(), + selected, + total, + }); + self.request_redraw(); + None + } + // Discovery Stage 2 — the v23 rows form of the same surface, + // carrying an optional per-row detail (a command's + // description). `prompt: None` closes it, and the close + // arrives in THIS family because the daemon picks the family + // per peer: a rows session closed by a legacy clear would + // leave the dropdown on screen forever. + InstanceMessage::MinibufferPromptRows { + prompt, + input, + cursor, + rows, + selected, + total, + } => { + self.minibuffer = prompt.map(|prompt| MinibufferLocal { + prompt, + input, + cursor, + rows, selected, total, }); @@ -7619,11 +7663,22 @@ impl State { /// Re-shape the minibuffer dropdown candidates (Q#MB1), one line per /// candidate, best match first. Empty when there are no candidates. + /// + /// Discovery Stage 2: a row with a `detail` renders `label detail`, + /// the same two-space form the completion dropdown already uses. A + /// row without one renders the bare label, so a file-path or + /// buffer-name prompt looks exactly as it did before v23. fn refresh_mb_buffer(&mut self) { - let text = self - .minibuffer - .as_ref() - .map_or_else(String::new, |mb| mb.candidates.join("\n")); + let text = self.minibuffer.as_ref().map_or_else(String::new, |mb| { + mb.rows + .iter() + .map(|row| match row.detail.as_deref() { + Some(detail) => format!("{} {detail}", row.label), + None => row.label.clone(), + }) + .collect::>() + .join("\n") + }); let family = self.resolved_family.clone(); self.mb_buffer.set_text( &mut self.font_system, @@ -7644,7 +7699,7 @@ impl State { let mb = self.minibuffer.as_ref()?; let band_top = status_band_top(self.config.height, self.fm); mb_dropdown_window( - mb.candidates.len(), + mb.rows.len(), mb.selected.map_or(0, |s| s as usize), band_top, self.fm, @@ -10868,6 +10923,7 @@ fn instance_message_label(msg: &InstanceMessage) -> &'static str { InstanceMessage::SearchPrompt { .. } => "SearchPrompt", InstanceMessage::MenuPrompt { .. } => "MenuPrompt", InstanceMessage::MinibufferPrompt { .. } => "MinibufferPrompt", + InstanceMessage::MinibufferPromptRows { .. } => "MinibufferPromptRows", InstanceMessage::BlockAdornments { .. } => "BlockAdornments", InstanceMessage::FoldState { .. } => "FoldState", InstanceMessage::ResourceOffer { .. } => "ResourceOffer", @@ -13766,6 +13822,22 @@ mod tests { // They skip (not fail) when no wgpu adapter is available — a dev box // without working Vulkan, or CI without lavapipe. + /// Detail-free minibuffer rows from bare labels — what a `12..=22` + /// daemon's frozen `MinibufferPrompt` lands as. + fn detailless_rows(labels: I) -> Vec + where + I: IntoIterator, + S: Into, + { + labels + .into_iter() + .map(|label| MinibufferRow { + label: label.into(), + detail: None, + }) + .collect() + } + /// Build a headless `State`, or return `None` and log when there's no /// adapter so the caller can skip. When `PMACS_REQUIRE_GPU` is set /// (CI, where lavapipe is installed) a missing adapter is a hard @@ -14888,7 +14960,7 @@ mod tests { prompt: "M-x ".to_owned(), input: "find".to_owned(), cursor: 4, - candidates: Vec::new(), + rows: Vec::new(), selected: None, total: 0, }); @@ -15291,7 +15363,7 @@ mod tests { prompt: "M-x ".into(), input: "theme".into(), cursor: 5, - candidates: Vec::new(), + rows: Vec::new(), selected: None, total: 0, }); @@ -15335,7 +15407,7 @@ mod tests { prompt: "M-x ".into(), input: "the".into(), cursor: 3, - candidates: vec!["theme-set".into(), "theme-clear".into()], + rows: detailless_rows(["theme-set", "theme-clear"]), selected: Some(0), total: 2, }); @@ -15357,6 +15429,101 @@ mod tests { ); } + /// Discovery Stage 2: a row's `detail` reaches the shaped dropdown + /// line, and BOTH wire families land in the same local shape. + /// + /// Driven through `apply_attach_message` rather than by assigning + /// `state.minibuffer` — the mapping from wire variant to local row + /// is exactly what this asserts, so constructing the local value + /// would skip the thing under test. The shaped `layout_runs()` text + /// is what glyphon rasterizes, so a description present there is a + /// description on screen. + #[test] + fn a_minibuffer_row_detail_reaches_the_shaped_dropdown_line() { + let Some(mut state) = headless_or_skip(600, 400, "hello") else { + return; + }; + + // The v23 rows form: a row with a detail, and a row without. + let _ = state.apply_attach_message(InstanceMessage::MinibufferPromptRows { + prompt: Some("M-x ".into()), + input: "buf".into(), + cursor: 3, + rows: vec![ + MinibufferRow { + label: "buffer.save".into(), + detail: Some("Write the buffer to its file".into()), + }, + MinibufferRow { + label: "buffer.kill".into(), + detail: None, + }, + ], + selected: Some(0), + total: 2, + }); + state.refresh_mb_buffer(); + let lines: Vec = state + .mb_buffer + .layout_runs() + .map(|run| run.text.to_owned()) + .collect(); + assert!( + lines + .iter() + .any(|l| l.contains("buffer.save") && l.contains("Write the buffer to its file")), + "the detail must be shaped into the row: {lines:?}" + ); + assert_eq!( + lines + .iter() + .find(|l| l.contains("buffer.kill")) + .map(String::as_str), + Some("buffer.kill"), + "a row with no detail renders the bare label, exactly as before v23: {lines:?}" + ); + + // The frozen `12..=22` form, which an older daemon still sends: + // bare strings become detail-free rows. + let _ = state.apply_attach_message(InstanceMessage::MinibufferPrompt { + prompt: Some("M-x ".into()), + input: "buf".into(), + cursor: 3, + candidates: vec!["buffer.save".into()], + selected: Some(0), + total: 1, + }); + assert_eq!( + state.minibuffer.as_ref().map(|mb| mb.rows.clone()), + Some(vec![MinibufferRow { + label: "buffer.save".into(), + detail: None, + }]), + "the legacy variant lands as a detail-free row" + ); + state.refresh_mb_buffer(); + let legacy: Vec = state + .mb_buffer + .layout_runs() + .map(|run| run.text.to_owned()) + .collect(); + assert_eq!(legacy, vec!["buffer.save".to_owned()]); + + // Either family closes the surface with `prompt: None`. + let _ = state.apply_attach_message(InstanceMessage::MinibufferPromptRows { + prompt: None, + input: String::new(), + cursor: 0, + rows: Vec::new(), + selected: None, + total: 0, + }); + assert!( + state.minibuffer.is_none(), + "a rows clear closes the surface" + ); + } + #[test] fn headless_diag_face_recolors_band_counter_despite_unchanged_text() { // Acceptance 22 — the round-1 finding-3 bite. The E: counter @@ -15954,7 +16121,7 @@ mod tests { prompt: "P: ".into(), input: String::new(), cursor: 0, - candidates: vec![long.clone(), long.clone()], + rows: detailless_rows([long.clone(), long.clone()]), selected: Some(1), total: 2, }); @@ -16172,7 +16339,7 @@ mod tests { prompt: "M-x ".into(), input: String::new(), cursor: 0, - candidates: (0..30).map(|i| format!("candidate-{i}")).collect(), + rows: detailless_rows((0..30).map(|i| format!("candidate-{i}"))), selected: Some(1), total: 30, }); @@ -17095,7 +17262,7 @@ mod tests { prompt: ":".into(), input: String::new(), cursor: 0, - candidates: Vec::new(), + rows: Vec::new(), selected: None, total: 0, }); diff --git a/pmacs-protocol/src/lib.rs b/pmacs-protocol/src/lib.rs index 9a0a2dc..6976a8c 100644 --- a/pmacs-protocol/src/lib.rs +++ b/pmacs-protocol/src/lib.rs @@ -65,11 +65,12 @@ pub use message::{ InstanceMessage, InstanceSignal, Key, KeyEvent, LineNumberMode, MAX_INITIAL_TARGET_ERROR_BYTES, MAX_INITIAL_TARGET_PATH_BYTES, MAX_STATUSLINE_FACE_BYTES, MAX_STATUSLINE_PROVIDER_NAME_BYTES, MAX_STATUSLINE_PROVIDERS, MAX_STATUSLINE_SEGMENT_BYTES, MAX_STATUSLINE_TOTAL_TEXT_BYTES, - MenuPromptRow, Modifiers, MouseButton, MouseEvent, MouseKind, NegotiatedCapabilities, - PROTOCOL_VERSION, PointerKind, ResourceBody, SUPPORTED_PROTOCOL_VERSIONS, SelectionSnapshot, - SessionBootstrapRequest, StatuslineSegment, StyleSegment, StyleSpan, ThemeFace, - is_builtin_pair_char, is_modeline_face_name, is_supported_protocol_version, is_ui_face_name, - negotiate_capabilities, negotiated_session_version, requested_protocol_version, + MenuPromptRow, MinibufferRow, Modifiers, MouseButton, MouseEvent, MouseKind, + NegotiatedCapabilities, PROTOCOL_VERSION, PointerKind, ResourceBody, + SUPPORTED_PROTOCOL_VERSIONS, SelectionSnapshot, SessionBootstrapRequest, StatuslineSegment, + StyleSegment, StyleSpan, ThemeFace, is_builtin_pair_char, is_modeline_face_name, + is_supported_protocol_version, is_ui_face_name, negotiate_capabilities, + negotiated_session_version, requested_protocol_version, }; pub use panel::{ MAX_PANEL_VISIBLE_CELLS, PANEL_MIN_VERSION, PanelFrame, PanelFrameError, PanelFramePayload, diff --git a/pmacs-protocol/src/message.rs b/pmacs-protocol/src/message.rs index b4c8e7e..8516d4d 100644 --- a/pmacs-protocol/src/message.rs +++ b/pmacs-protocol/src/message.rs @@ -1098,7 +1098,24 @@ pub enum InstanceMessage { /// v12). The minibuffer is a single *global* core instance, so this /// is bufferless; the producer still emits it from the active-buffer /// viewport. `prompt: None` clears the GUI. Cached-compare - /// suppressed like `SearchPrompt`; daemon-gated `>= 12`. + /// suppressed like `SearchPrompt`; daemon-gated `12..=22`. + /// + /// # FROZEN — this variant's encoding must not move + /// + /// Discovery Stage 2 (v23) needed richer rows, and postcard is not + /// self-describing: enum variants encode by index and fields by + /// position, so widening `candidates` in place would make every + /// v12–v22 peer **mis-decode** these bytes rather than ignore them. + /// Gating the widened shape at `>= 23` would not rescue them either + /// — with only one variant to send, they would receive no minibuffer + /// message at all. So the rich form went into a new appended + /// variant, [`Self::MinibufferPromptRows`], and this one is retained + /// unchanged as what a `12..=22` peer receives. + /// + /// Its bytes are pinned literally by + /// `minibuffer_prompt_v12_wire_bytes_are_frozen` in + /// `src/protocol.rs` — a round-trip cannot detect a field addition, + /// because both sides simply learn the new shape. MinibufferPrompt { /// The prompt string (e.g. `"M-x "`), or `None` when no /// minibuffer is open. @@ -1299,6 +1316,59 @@ pub enum InstanceMessage { /// Whether that buffer's long lines wrap. wrap: bool, }, + /// Discovery Stage 2 (protocol v23): the minibuffer prompt with + /// **structured rows** — a label and an optional one-line detail — + /// instead of bare candidate strings. + /// + /// # Why a second variant rather than a wider `MinibufferPrompt` + /// + /// `Command.description` already exists and is already rendered by + /// `help.list-commands`; it is missing at the one moment it would + /// change a decision, which is the `M-x` row. Carrying it means + /// widening the minibuffer's candidate shape — and postcard encodes + /// fields **positionally**, so changing `candidates: Vec` in + /// place is a wire break, not an evolution: a v22 peer mis-decodes + /// the bytes rather than skipping them. Gating the changed variant + /// at `>= 23` does not rescue it either, because a `12..=22` peer + /// would then receive no minibuffer message at all. Compatibility + /// requires the old shape to still exist *and still be sent*, so + /// [`Self::MinibufferPrompt`] is frozen and this is appended beside + /// it. + /// + /// # Exactly one of the two reaches any peer + /// + /// The producer selects on the session's negotiated version and the + /// daemon's write loop gates both directions: `>= 23` receives this + /// and never the legacy variant; `12..=22` receives the legacy + /// variant and never this. Sending both would double-render; sending + /// neither is the bug gating alone would have caused. The close + /// message must use the same family as the open — a rows session + /// closed by a legacy clear leaves a popup on screen forever. + /// + /// Otherwise this mirrors [`Self::MinibufferPrompt`] exactly: + /// bufferless (one global core minibuffer), `prompt: None` clears + /// the GUI, cached-compare suppressed, emitted from the + /// active-buffer viewport. + /// + /// Appended after [`Self::LineWrapFacts`], the final v22 variant, so + /// no existing postcard discriminant moves. + MinibufferPromptRows { + /// The prompt string (e.g. `"M-x "`), or `None` when no + /// minibuffer is open. + prompt: Option, + /// The text typed so far. + input: String, + /// Codepoints before the cursor within `input` (the caret + /// position). + cursor: u32, + /// A windowed slice of the completion candidates (best-first, + /// already filtered/sorted by the core), `<= MB_VISIBLE`. + rows: Vec, + /// Highlighted row *within* `rows`, or `None`. + selected: Option, + /// Total candidate count (the window is a slice of this). + total: u32, + }, } /// One resolved UI face for [`InstanceMessage::ThemeFacts`]: a full @@ -1394,6 +1464,35 @@ pub struct CompletionPopupRow { pub detail: Option, } +/// One row of the minibuffer's candidate list on the wire +/// ([`InstanceMessage::MinibufferPromptRows`], Discovery Stage 2, +/// protocol v23). +/// +/// # Why this is not `CompletionPopupRow` +/// +/// Reuse was tempting and is wrong. [`CompletionPopupRow::kind`] is an +/// LSP `CompletionItemKind` code with a documented contract, and an +/// `M-x` command is not an LSP completion item — it has no honest value +/// for that field. Reusing it would mean inventing a fake kind or +/// declaring unknown everywhere: a type whose invariant is "meaningless +/// in half its uses". If a category is wanted later it arrives with +/// `Command.category`, typed as what it actually is rather than +/// borrowed from LSP. +/// +/// `detail` is optional **per row** because `pmacs.minibuffer.read` +/// serves many sources — file paths, buffer names, settings — and only +/// some have a natural detail. A source with none leaves it `None` and +/// renders exactly as it did before v23. +#[derive(serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq, Eq)] +pub struct MinibufferRow { + /// Display label — the candidate itself, and the value acceptance + /// resolves to. + pub label: String, + /// Optional one-line detail rendered after the label (a command's + /// description, for `M-x`). + pub detail: Option, +} + /// Flat selection state for the wire. /// /// Mirrors [`crate::window::Selection`] but as a self-contained pair @@ -1731,7 +1830,17 @@ pub enum ResourceBody { /// directions: a v20 peer neither receives `PanelFrame` nor is placed in /// a side window, because denying only the events would leave its /// window invisible. -pub const PROTOCOL_VERSION: u32 = 22; +/// +/// Discovery Stage 2: bumped 22 → 23 for +/// [`InstanceMessage::MinibufferPromptRows`] — the minibuffer's +/// candidate rows gaining an optional per-row detail. Appended after +/// `LineWrapFacts`, the final v22 variant, so no existing discriminant +/// moves; [`InstanceMessage::MinibufferPrompt`] is retained **frozen** +/// and still sent to `12..=22` peers, because postcard's positional +/// encoding makes an in-place widening a wire break rather than an +/// evolution, and gating the widened form would have left those peers +/// with no minibuffer message at all. +pub const PROTOCOL_VERSION: u32 = 23; /// Protocol version placed in the daemon's server-first [`Hello`]. /// @@ -1905,8 +2014,15 @@ pub fn negotiated_session_version(frontend_offer: u32) -> u32 { /// [`ADVERTISED_PROTOCOL_VERSION`] does not move — a v21 frontend /// negotiates v21, never receives the variant, and keeps its own /// behavior. +/// +/// Discovery Stage 2: extended to `[6, ..., 23]` for +/// [`InstanceMessage::MinibufferPromptRows`]. Additive and daemon-gated, +/// and unusually the gate is a **range on both sides**: a `12..=22` peer +/// keeps receiving the frozen [`InstanceMessage::MinibufferPrompt`], a +/// `>= 23` peer receives only the rows form, and no peer ever receives +/// both. [`ADVERTISED_PROTOCOL_VERSION`] does not move. pub const SUPPORTED_PROTOCOL_VERSIONS: &[u32] = &[ - 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, + 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, ]; /// T M10.5: predicate for the handshake check. Returns `true` if diff --git a/src/daemon.rs b/src/daemon.rs index fff5e21..e5c5a94 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -1420,9 +1420,23 @@ fn dispatcher_loop( let peer_knows_menu_prompt = session_registry .session_state(*fid) .is_some_and(|s| s.negotiated_protocol_version >= 11); - let peer_knows_minibuffer_prompt = session_registry - .session_state(*fid) - .is_some_and(|s| s.negotiated_protocol_version >= 12); + // Q#MB1 / Discovery Stage 2 — the minibuffer is the one + // surface with TWO live variants, and the gate is a + // RANGE on both sides rather than a floor. The legacy + // `MinibufferPrompt` is frozen and belongs to `12..=22`; + // `MinibufferPromptRows` belongs to `>= 23`. Writing the + // legacy gate as a bare `>= 12` would let a v23 peer + // receive both and double-render its dropdown. + let peer_knows_minibuffer_prompt = + session_registry.session_state(*fid).is_some_and(|s| { + (12..crate::semantic_render::MINIBUFFER_ROWS_MIN_VERSION) + .contains(&s.negotiated_protocol_version) + }); + let peer_knows_minibuffer_rows = + session_registry.session_state(*fid).is_some_and(|s| { + s.negotiated_protocol_version + >= crate::semantic_render::MINIBUFFER_ROWS_MIN_VERSION + }); // UX gutter — `LineNumbers` carries a `LineNumberMode` since // v14 (was `enabled: bool` in v13); a peer below 14 keeps // its gutter off rather than mis-decoding the wider shape. @@ -1470,12 +1484,24 @@ fn dispatcher_loop( continue; } // Q#MB1 — MinibufferPrompt gated at v12; a v11 peer - // simply can't render the GUI minibuffer. + // simply can't render the GUI minibuffer. Discovery + // Stage 2 closed the range at the top: a v23 peer + // gets the rows form instead, never both. if !peer_knows_minibuffer_prompt && matches!(msg, InstanceMessage::MinibufferPrompt { .. }) { continue; } + // Discovery Stage 2 — MinibufferPromptRows gated at + // v23. A `12..=22` peer keeps the frozen legacy + // variant above, which is why gating alone was never + // enough: with one variant it would have lost the + // minibuffer entirely. + if !peer_knows_minibuffer_rows + && matches!(msg, InstanceMessage::MinibufferPromptRows { .. }) + { + continue; + } if !peer_knows_line_numbers && matches!(msg, InstanceMessage::LineNumbers { .. }) { diff --git a/src/editor.rs b/src/editor.rs index 8d80a6c..7050023 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -4732,7 +4732,10 @@ pub fn paint_frame( paint_search_prompt(grid, core, term_size, &theme); None } else if core.minibuffer.is_active() { - Some(paint_minibuffer(grid, core, term_size, &theme)) + // The command registry is a separate `RefCell` from the core, so + // this borrow does not contend with the one held above. + let commands = state.lua_host.commands().borrow(); + Some(paint_minibuffer(grid, core, &commands, term_size, &theme)) } else { None }; @@ -5464,9 +5467,42 @@ fn minibuffer_style(theme: &crate::highlight::Theme) -> crate::cell::Style { }) } +/// The inline candidate suffix for the minibuffer's bottom row, given +/// the columns still free after the prompt and the typed input. +/// +/// Discovery Stage 2 §3.4 — three ORDERED steps, and the guarantee is +/// **"never a partial name"**, not "the name always survives". The +/// latter is unachievable: the prompt and the typed input consume the +/// budget first, so the remainder can be too small even for the bare +/// name. +/// +/// 1. If the whole name does not fit, emit **nothing**. A truncated +/// `[buffer.sa…]` is worse than no suffix, because it reads as a +/// different command. +/// 2. Only once the whole name fits is a description attempted. +/// 3. If the description does not fit whole, drop it — leaving exactly +/// today's `[name]`. No ellipsis stub. +/// +/// Measured in `char`s, matching the painter below: it writes one cell +/// per `char`. +fn minibuffer_candidate_suffix(name: &str, detail: Option<&str>, remaining: u32) -> String { + let bare = format!(" [{name}]"); + if bare.chars().count() as u32 > remaining { + return String::new(); + } + if let Some(detail) = detail.map(str::trim).filter(|d| !d.is_empty()) { + let full = format!(" [{name} — {detail}]"); + if full.chars().count() as u32 <= remaining { + return full; + } + } + bare +} + fn paint_minibuffer( grid: &mut crate::cell::CellGrid<'_>, core: &EditorCore, + commands: &crate::command::CommandRegistry, term_size: crate::cell::CellSize, theme: &crate::highlight::Theme, ) -> u32 { @@ -5477,12 +5513,6 @@ fn paint_minibuffer( .expect("called only when active"); let prompt = &session.prompt; let contents = core.minibuffer.contents(); - let mut suffix = String::new(); - if let Some(idx) = session.selected - && let Some(cand) = session.candidates.get(idx) - { - suffix = format!(" [{cand}]"); - } let row = term_size.rows - 1; let mut col: u32 = 0; let mut written: u32 = 0; @@ -5537,6 +5567,26 @@ fn paint_minibuffer( cursor_col = prompt_end; } + // Discovery Stage 2 (§3.4): the selected candidate's suffix now + // carries the command's DESCRIPTION, read from the registry + // in-process. The grid TUI never consumes `MinibufferPrompt` — it + // paints from `core.minibuffer` — so this half of the lane involves + // no wire at all and is independent of the v23 bump. + // + // Q#D2-2: only the command source has a detail. A file-path or + // buffer-name prompt renders exactly as it did before. + let suffix = match session.selected.and_then(|idx| session.candidates.get(idx)) { + Some(cand) => { + let detail = matches!( + session.source, + crate::minibuffer::CompletionSource::Commands + ) + .then(|| commands.get(cand).map(|c| c.description.as_str())) + .flatten(); + minibuffer_candidate_suffix(cand, detail, max.saturating_sub(col)) + } + None => String::new(), + }; for ch in suffix.chars() { if col >= max { break; diff --git a/src/frontend.rs b/src/frontend.rs index 1d04e9f..60d7449 100644 --- a/src/frontend.rs +++ b/src/frontend.rs @@ -406,6 +406,10 @@ impl Frontend { // surface; the TUI paints the minibuffer via its own bottom // row, so it drops this silently too. | InstanceMessage::MinibufferPrompt { .. } + // Discovery Stage 2 — the v23 rows form of the same surface. + // The TUI reads `Command.description` from the registry + // in-process instead, so this reaches it not at all. + | InstanceMessage::MinibufferPromptRows { .. } // UX gutter — LineNumbers is the semantic-frontend gutter // toggle; the cell-grid TUI reads its window's mode directly, // so it drops this silently like the other semantic families. diff --git a/src/protocol.rs b/src/protocol.rs index 1a1c723..33beee0 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -1683,7 +1683,7 @@ mod tests { // --- M5.5a handshake & postcard round-trips --- #[test] - fn protocol_version_is_twenty_two_for_line_wrap_facts() { + fn protocol_version_is_twenty_three_for_minibuffer_prompt_rows() { // 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). T M11.6 @@ -1732,7 +1732,15 @@ mod tests { // daemon-gated, appended after the final v21 variant). The // GPU lays out locally and would otherwise never hear the wrap // setting; the advertised baseline is deliberately unmoved. - assert_eq!(PROTOCOL_VERSION, 22); + // Discovery Stage 2 bumps 22→23 (`InstanceMessage:: + // MinibufferPromptRows`, daemon-gated, appended after the final + // v22 variant). The first bump to leave the SUPERSEDED variant + // live rather than widening it: postcard is positional, so + // widening `MinibufferPrompt` would break every v12–v22 peer, + // and gating the wider form would have left them with no + // minibuffer at all. `MinibufferPrompt` is therefore frozen and + // pinned by literal bytes below. + assert_eq!(PROTOCOL_VERSION, 23); } #[test] @@ -1809,17 +1817,18 @@ mod tests { // (`CompletionPopup`), v16 (`ThemeFacts`), v17 (`FontFacts`), // v18 (`StatuslineSegments`), v19 (the vterm terminal family), // v20 (semantic initial-target bootstrap), v21 (the bottom - // panel band), and v22 (`LineWrapFacts`) all interoperate. - for accepted in 6..=22 { + // panel band), v22 (`LineWrapFacts`), and v23 + // (`MinibufferPromptRows`) all interoperate. + for accepted in 6..=23 { assert!( is_supported_protocol_version(accepted), "v{accepted} must be accepted" ); } - for rejected in [0, 1, 2, 3, 4, 5, 23, u32::MAX] { + for rejected in [0, 1, 2, 3, 4, 5, 24, u32::MAX] { assert!( !is_supported_protocol_version(rejected), - "v{rejected} must be rejected by a v22 binary" + "v{rejected} must be rejected by a v23 binary" ); } } @@ -2406,6 +2415,130 @@ mod tests { } } + #[test] + fn minibuffer_prompt_v12_wire_bytes_are_frozen() { + // Discovery Stage 2 (v23) froze `MinibufferPrompt` and put the + // richer shape in an appended `MinibufferPromptRows`. THIS is + // what makes the freeze real, and the round-trip above is not: + // a round-trip encodes and decodes with the SAME types, so + // adding a field to `MinibufferPrompt` leaves it passing while + // every v12–v22 peer in the field mis-decodes the bytes. Only a + // comparison against bytes captured now can fail when the + // encoding changes. + // + // Two fixtures, the two shapes the producer emits: an open + // prompt with a windowed candidate list and a selection, and a + // cleared band. Discriminant 20, then the fields positionally + // (postcard is not self-describing). + let open = InstanceMessage::MinibufferPrompt { + prompt: Some("M-x ".to_owned()), + input: "ed".to_owned(), + cursor: 2, + candidates: vec!["edit.copy".to_owned(), "edit.cut".to_owned()], + selected: Some(1), + total: 7, + }; + assert_eq!( + postcard::to_allocvec(&open).expect("encode open"), + [ + 20, // InstanceMessage::MinibufferPrompt + 1, 4, b'M', b'-', b'x', b' ', // prompt: Some("M-x ") + 2, b'e', b'd', // input: "ed" + 2, // cursor + 2, 9, b'e', b'd', b'i', b't', b'.', b'c', b'o', b'p', b'y', 8, b'e', b'd', b'i', + b't', b'.', b'c', b'u', b't', // candidates + 1, 1, // selected: Some(1) + 7, // total + ], + "MinibufferPrompt's v12 wire bytes changed. It is FROZEN for \ + v12..=22 — a widening here mis-decodes on every already-shipped \ + frontend rather than being ignored. Richer minibuffer rows \ + belong in MinibufferPromptRows." + ); + + let clear = InstanceMessage::MinibufferPrompt { + prompt: None, + input: String::new(), + cursor: 0, + candidates: Vec::new(), + selected: None, + total: 0, + }; + assert_eq!( + postcard::to_allocvec(&clear).expect("encode clear"), + [20, 0, 0, 0, 0, 0, 0], + "MinibufferPrompt's cleared-band v12 wire bytes changed — see the \ + open-prompt fixture above" + ); + } + + #[test] + fn line_wrap_facts_encoding_is_unchanged_by_the_v23_build() { + // Discovery Stage 2 placement pin: `MinibufferPromptRows` must + // be APPENDED after `LineWrapFacts` — the final v22 variant, + // whose ordinal moves if anything is inserted before any v22 + // variant. The new variant's own round-trip cannot detect a + // shift, which is why the pin sits on the PREVIOUS final variant + // (handoff §4). + let msg = InstanceMessage::LineWrapFacts { + buffer_id: pmacs_protocol::BufferId::from_raw(4), + wrap: true, + }; + let bytes = postcard::to_allocvec(&msg).expect("encode"); + assert_eq!( + bytes, + [29, 4, 1], + "LineWrapFacts' v22 wire bytes changed — a variant was \ + inserted before it; append new InstanceMessage variants \ + at the end" + ); + } + + #[test] + fn minibuffer_prompt_rows_round_trips_and_appends_after_line_wrap_facts() { + // The v23 variant itself: both shapes, a detail present and a + // detail absent (Q#D2-2 — a source with no detail leaves it + // `None` and renders as it always did), plus the cleared band. + let cases = [ + ( + Some("M-x ".to_owned()), + "ed".to_owned(), + 2u32, + vec![ + MinibufferRow { + label: "edit.copy".to_owned(), + detail: Some("Copy the region".to_owned()), + }, + MinibufferRow { + label: "notes.txt".to_owned(), + detail: None, + }, + ], + Some(1u32), + 7u32, + ), + (None, String::new(), 0, Vec::new(), None, 0), + ]; + for (prompt, input, cursor, rows, selected, total) in cases { + let msg = InstanceMessage::MinibufferPromptRows { + prompt: prompt.clone(), + input: input.clone(), + cursor, + rows: rows.clone(), + selected, + total, + }; + let bytes = postcard::to_allocvec(&msg).expect("encode"); + assert_eq!( + bytes.first(), + Some(&30), + "MinibufferPromptRows must be appended after v22 LineWrapFacts" + ); + let decoded: InstanceMessage = postcard::from_bytes(&bytes).expect("decode"); + assert_eq!(decoded, msg); + } + } + #[test] fn key_event_to_crossterm_round_trips() { // Build a protocol KeyEvent, translate to crossterm, translate diff --git a/src/semantic_render.rs b/src/semantic_render.rs index de9f417..2e62952 100644 --- a/src/semantic_render.rs +++ b/src/semantic_render.rs @@ -38,7 +38,7 @@ use crate::cell::{CellSize, Style}; use crate::editor::EditorState; use crate::protocol::{ AdornmentContent, AdornmentPlacement, ByteRange, Decoration, DecorationKind, DecorationSegment, - FrontendId, InlineAdornment, InstanceMessage, MenuPromptRow, PANEL_MIN_VERSION, + FrontendId, InlineAdornment, InstanceMessage, MenuPromptRow, MinibufferRow, PANEL_MIN_VERSION, StatuslineSegment, StyleSegment, StyleSpan, }; use crate::statusline::{ @@ -95,10 +95,26 @@ type SearchPromptFacts = (Option, Option, u32, bool, bool); /// menu. type MenuPromptFacts = (Vec, Option); -/// Cached `MinibufferPrompt` payload for cached-compare suppression -/// (Q#MB1): `(prompt, input, cursor, candidates-window, selected, total)`. -/// A `None` prompt means the minibuffer is closed. -type MinibufferFacts = (Option, String, u32, Vec, Option, u32); +/// Cached minibuffer payload for cached-compare suppression (Q#MB1): +/// `(prompt, input, cursor, rows-window, selected, total)`. A `None` +/// prompt means the minibuffer is closed. +/// +/// **ONE cache per peer, not one per variant.** [`SemanticRenderState`] +/// is constructed by [`SemanticRenderState::for_peer`] with the +/// session's negotiated version baked in on attach and dropped on +/// detach, so a cache can never span two negotiated versions and a +/// per-variant key would guard nothing. The rows are the cached form +/// either way: for a `12..=22` peer every `detail` is `None` (the +/// producer does not resolve details it cannot ship), so the cache +/// describes exactly what that peer received. +type MinibufferFacts = ( + Option, + String, + u32, + Vec, + Option, + u32, +); /// Cached `CompletionPopup` payload for cached-compare suppression /// (Arc 1a Q#C5): `(anchor, prefix_len, rows-window, selected, total)`. @@ -115,10 +131,20 @@ type CompletionPopupFacts = ( /// scrolled window around the selection, not the full (≤1024) list. const MB_VISIBLE: usize = 10; +/// The first protocol version that carries +/// [`InstanceMessage::MinibufferPromptRows`] (Discovery Stage 2). +/// +/// Named rather than written as a literal `23` at each site, and NOT +/// derived from `PROTOCOL_VERSION`: the contract is "the version this +/// variant was introduced at", which is an absolute fact, while +/// `PROTOCOL_VERSION` moves with every later bump. Handoff §5 records +/// five defects of exactly that shape from one previous bump. +pub const MINIBUFFER_ROWS_MIN_VERSION: u32 = 23; + /// A window of up to [`MB_VISIBLE`] candidates around `selected`, plus /// the selection's index *within* that window. Keeps the selected row /// visible as the user cycles a long list. -fn minibuffer_window(candidates: &[String], selected: Option) -> (Vec, Option) { +fn minibuffer_window(candidates: &[T], selected: Option) -> (Vec, Option) { if candidates.is_empty() { return (Vec::new(), None); } @@ -216,10 +242,18 @@ pub struct SemanticRenderState { /// Last emitted `MenuPrompt` payload per buffer (Q#CM1), for /// cached-compare suppression (see [`MenuPromptFacts`]). last_menu_prompt: HashMap, - /// Last emitted `MinibufferPrompt` payload (Q#MB1) — a single value, - /// not per-buffer, because the minibuffer is one global core - /// instance. + /// Last emitted minibuffer payload (Q#MB1) — a single value, not + /// per-buffer, because the minibuffer is one global core instance, + /// and a single value across both wire variants, because this state + /// belongs to one peer at one negotiated version (see + /// [`MinibufferFacts`]). last_minibuffer: Option, + /// Whether the peer negotiated protocol >= 23 (Discovery Stage 2). + /// `true` ⇒ it receives `MinibufferPromptRows` and never the legacy + /// variant; `false` ⇒ the frozen `MinibufferPrompt` and never the + /// rows form. Also gates the per-row detail lookup: a peer that + /// cannot carry a detail does not pay to resolve one. + peer_knows_minibuffer_rows: bool, /// Last emitted `CompletionPopup` payload per buffer (Arc 1a /// Q#C5), for cached-compare suppression (see /// [`CompletionPopupFacts`]). @@ -478,6 +512,7 @@ impl SemanticRenderState { s.peer_knows_theme_facts = negotiated_protocol_version >= 16; s.peer_knows_font_facts = negotiated_protocol_version >= 17; s.peer_knows_line_wrap = negotiated_protocol_version >= 22; + s.peer_knows_minibuffer_rows = negotiated_protocol_version >= MINIBUFFER_ROWS_MIN_VERSION; s.peer_knows_statusline_segments = negotiated_protocol_version >= 18; s.peer_knows_terminal_frames = negotiated_protocol_version >= 19; s.peer_knows_panel_frames = negotiated_protocol_version >= PANEL_MIN_VERSION; @@ -500,6 +535,7 @@ impl SemanticRenderState { last_search_prompt: HashMap::new(), last_menu_prompt: HashMap::new(), last_minibuffer: None, + peer_knows_minibuffer_rows: true, last_completion_popup: HashMap::new(), last_summary: HashMap::new(), last_status: HashMap::new(), @@ -1637,11 +1673,20 @@ impl SemanticRenderState { Some(msg) } - /// The `MinibufferPrompt` message for this frame, or `None` when the + /// The minibuffer message for this frame, or `None` when the /// (global) minibuffer state is unchanged (Q#MB1). Emitted only from /// the active buffer's viewport so the bufferless message ships once /// per frame. Closed = `prompt: None`; first sight while closed stays - /// silent. The daemon keeps the variant off wires negotiated `< 12`. + /// silent. + /// + /// **Exactly one variant, chosen by the peer's negotiated version** + /// (Discovery Stage 2). `>= 23` gets `MinibufferPromptRows` with + /// per-row details; `12..=22` gets the frozen `MinibufferPrompt` + /// carrying bare labels. Because the choice is made here, the CLOSE + /// necessarily uses the same family as the OPEN — a rows session + /// closed by a legacy clear would leave a popup on screen forever. + /// The daemon's write loop gates both directions again as + /// belt-and-braces. fn minibuffer_prompt_msg( &mut self, state: &EditorState, @@ -1662,13 +1707,44 @@ impl SemanticRenderState { .take_while(|(i, _)| *i < cursor_byte) .count() as u32; let total = session.candidates.len() as u32; - let (candidates, selected) = + let (labels, selected) = minibuffer_window(&session.candidates, session.selected); + // Q#D2-2: the detail is per row and optional. Only + // the command source has one today; a file-path or + // buffer-name prompt leaves it `None` and renders + // exactly as it did before v23. Resolved only for a + // peer that can carry it, so the cached facts + // describe what that peer actually received. + let detail_source = self.peer_knows_minibuffer_rows + && matches!( + session.source, + crate::minibuffer::CompletionSource::Commands + ); + let rows = if detail_source { + let commands = state.lua_host.commands().borrow(); + labels + .into_iter() + .map(|label| { + let detail = commands + .get(&label) + .map(|command| command.description.clone()); + MinibufferRow { label, detail } + }) + .collect() + } else { + labels + .into_iter() + .map(|label| MinibufferRow { + label, + detail: None, + }) + .collect() + }; ( Some(session.prompt.clone()), input, cursor, - candidates, + rows, selected, total, ) @@ -1684,13 +1760,24 @@ impl SemanticRenderState { self.last_minibuffer = Some(facts); return None; } - let msg = InstanceMessage::MinibufferPrompt { - prompt: facts.0.clone(), - input: facts.1.clone(), - cursor: facts.2, - candidates: facts.3.clone(), - selected: facts.4, - total: facts.5, + let msg = if self.peer_knows_minibuffer_rows { + InstanceMessage::MinibufferPromptRows { + prompt: facts.0.clone(), + input: facts.1.clone(), + cursor: facts.2, + rows: facts.3.clone(), + selected: facts.4, + total: facts.5, + } + } else { + InstanceMessage::MinibufferPrompt { + prompt: facts.0.clone(), + input: facts.1.clone(), + cursor: facts.2, + candidates: facts.3.iter().map(|row| row.label.clone()).collect(), + selected: facts.4, + total: facts.5, + } }; self.last_minibuffer = Some(facts); Some(msg) @@ -5773,9 +5860,28 @@ mod tests { let short: Vec = vec!["a".into(), "b".into(), "c".into()]; assert_eq!(minibuffer_window(&short, Some(2)), (short.clone(), Some(2))); // Empty. - assert_eq!(minibuffer_window(&[], Some(0)), (Vec::new(), None)); + assert_eq!( + minibuffer_window::(&[], Some(0)), + (Vec::new(), None) + ); } + /// The v23 rows form: `(prompt, input, rows)`. + fn minibuffer_rows_of( + msgs: &[InstanceMessage], + ) -> Option<(Option, String, Vec)> { + msgs.iter().find_map(|m| match m { + InstanceMessage::MinibufferPromptRows { + prompt, + input, + rows, + .. + } => Some((prompt.clone(), input.clone(), rows.clone())), + _ => None, + }) + } + + /// The frozen `12..=22` form: `(prompt, input, candidates)`. fn minibuffer_prompt_of( msgs: &[InstanceMessage], ) -> Option<(Option, String, Vec)> { @@ -5798,7 +5904,7 @@ mod tests { s.set_viewport(bid, ByteRange { start: 0, end: 64 }, 0); // No minibuffer: the producer stays silent on first sight. - assert!(minibuffer_prompt_of(&s.render_frame(&state)).is_none()); + assert!(minibuffer_rows_of(&s.render_frame(&state)).is_none()); // Open an `M-x` prompt (command completion) via the Lua API. state @@ -5807,28 +5913,133 @@ mod tests { .load("pmacs.minibuffer.read{ prompt = 'M-x ', source = 'commands', on_accept = function() end }") .exec() .expect("open minibuffer"); - let (prompt, input, cands) = - minibuffer_prompt_of(&s.render_frame(&state)).expect("minibuffer prompt emitted"); + let (prompt, input, rows) = + minibuffer_rows_of(&s.render_frame(&state)).expect("minibuffer prompt emitted"); assert_eq!(prompt.as_deref(), Some("M-x ")); assert_eq!(input, ""); // Empty input matches every command; the wire carries a window. - assert!(!cands.is_empty(), "M-x seeds command candidates"); - assert!(cands.len() <= MB_VISIBLE, "candidates ship windowed"); + assert!(!rows.is_empty(), "M-x seeds command candidates"); + assert!(rows.len() <= MB_VISIBLE, "candidates ship windowed"); // Unchanged → suppressed (cached-compare). - assert!(minibuffer_prompt_of(&s.render_frame(&state)).is_none()); + assert!(minibuffer_rows_of(&s.render_frame(&state)).is_none()); - // Cancel: the prompt clears (None). + // Cancel: the prompt clears (None), in the SAME family as the + // open — a rows session closed by a legacy clear would leave the + // dropdown on screen forever. state .lua_host .lua() .load("pmacs.minibuffer.cancel()") .exec() .expect("cancel"); - let (prompt, _, _) = minibuffer_prompt_of(&s.render_frame(&state)).expect("clear emitted"); + let frame = s.render_frame(&state); + assert!( + minibuffer_prompt_of(&frame).is_none(), + "a v23 peer must never see the legacy variant, not even to close" + ); + let (prompt, _, _) = minibuffer_rows_of(&frame).expect("clear emitted"); assert!(prompt.is_none(), "cancel clears the minibuffer band"); } + #[test] + fn a_v22_peer_gets_the_frozen_variant_and_a_v23_peer_gets_rows_with_details() { + // The producer half of the exclusivity guarantee, at the two + // versions that straddle the boundary. The real-daemon half — + // two sessions negotiating simultaneously — is in + // `tests/discovery_stage2_acceptance.rs`. + let state = empty_state(); + let bid = active_buffer(&state); + state + .lua_host + .lua() + .load( + "pmacs.command.define{ name = 'mb.probe', description = 'Probe the row detail.', \ + fn = function() end }", + ) + .exec() + .expect("define probe command"); + + let mut v22 = SemanticRenderState::for_peer(FrontendId::LOCAL, 22); + let mut v23 = SemanticRenderState::for_peer(FrontendId::LOCAL, 23); + for s in [&mut v22, &mut v23] { + s.set_viewport(bid, ByteRange { start: 0, end: 64 }, 0); + let _ = s.render_frame(&state); + } + + state + .lua_host + .lua() + .load( + "pmacs.minibuffer.read{ prompt = 'M-x ', source = 'commands', \ + on_accept = function() end }", + ) + .exec() + .expect("open minibuffer"); + state + .lua_host + .lua() + .load("pmacs.minibuffer.set_contents('mb.probe')") + .exec() + .expect("narrow to the probe command"); + + let v22_frame = v22.render_frame(&state); + assert!( + minibuffer_rows_of(&v22_frame).is_none(), + "a v22 peer must never receive the v23 rows variant" + ); + let (_, _, candidates) = + minibuffer_prompt_of(&v22_frame).expect("v22 gets the frozen variant"); + assert!( + candidates.iter().any(|c| c == "mb.probe"), + "the frozen variant still carries the candidate names: {candidates:?}" + ); + + let v23_frame = v23.render_frame(&state); + assert!( + minibuffer_prompt_of(&v23_frame).is_none(), + "a v23 peer must never receive the frozen variant" + ); + let (_, _, rows) = minibuffer_rows_of(&v23_frame).expect("v23 gets the rows variant"); + let probe = rows + .iter() + .find(|r| r.label == "mb.probe") + .expect("the probe command is a candidate"); + assert_eq!( + probe.detail.as_deref(), + Some("Probe the row detail."), + "the row carries the command's registered description" + ); + } + + #[test] + fn a_source_with_no_detail_ships_rows_with_none() { + // Q#D2-2: only the command source has a detail today. A + // buffer-name prompt leaves it `None`, and the GPU then renders + // exactly what it rendered before v23. + let state = empty_state(); + let mut s = local(); + let bid = active_buffer(&state); + s.set_viewport(bid, ByteRange { start: 0, end: 64 }, 0); + let _ = s.render_frame(&state); + + state + .lua_host + .lua() + .load( + "pmacs.minibuffer.read{ prompt = 'Buffer: ', source = 'buffers', \ + on_accept = function() end }", + ) + .exec() + .expect("open buffer prompt"); + let (_, _, rows) = minibuffer_rows_of(&s.render_frame(&state)).expect("prompt emitted"); + assert!(!rows.is_empty(), "the buffer registry seeds candidates"); + assert!( + rows.iter().all(|r| r.detail.is_none()), + "a source with no detail leaves every row's detail None: {rows:?}" + ); + } + #[test] fn status_facts_emit_on_change_and_freeze_counts_while_stale() { let state = empty_state(); diff --git a/tests/bottom_panel_stage2b_gpu_acceptance.rs b/tests/bottom_panel_stage2b_gpu_acceptance.rs index 257680a..32c197d 100644 --- a/tests/bottom_panel_stage2b_gpu_acceptance.rs +++ b/tests/bottom_panel_stage2b_gpu_acceptance.rs @@ -337,8 +337,9 @@ fn one_daemon_serves_a_v21_panel_session_and_a_shipped_v20_client() { #[test] fn the_baseline_stays_and_the_counter_offer_activates() { // A deliberate tripwire: bumping the wire must be a conscious edit - // here, not a silent one. v22 is `LineWrapFacts` (long-lines Stage 3). - assert_eq!(PROTOCOL_VERSION, 22); + // here, not a silent one. v23 is `MinibufferPromptRows` (Discovery + // Stage 2); v22 was `LineWrapFacts` (long-lines Stage 3). + assert_eq!(PROTOCOL_VERSION, 23); assert_eq!( ADVERTISED_PROTOCOL_VERSION, 20, "moving this is the incompatible act the mechanism exists to avoid" @@ -352,10 +353,10 @@ fn the_baseline_stays_and_the_counter_offer_activates() { // This replaces `assert_eq!(PANEL_MIN_VERSION, PROTOCOL_VERSION)`, // which asserted a **coincidence**: panel frames were the newest // feature when it was written, so their minimum happened to equal - // the current wire. Any later feature falsifies that — v22 is the - // first, and the equality would have had to be edited on every - // subsequent bump while telling a reader something that was never - // the contract. + // the current wire. Any later feature falsifies that — v22 was the + // first and v23 the second, and the equality would have had to be + // edited on every subsequent bump while telling a reader something + // that was never the contract. // `const` blocks, matching the line above: these are compile-time // constants, so a runtime `assert!` is both a clippy error and a // weaker check than the language already offers. diff --git a/tests/discovery_stage2_acceptance.rs b/tests/discovery_stage2_acceptance.rs new file mode 100644 index 0000000..f290368 --- /dev/null +++ b/tests/discovery_stage2_acceptance.rs @@ -0,0 +1,537 @@ +// discovery_stage2_acceptance.rs --- Discovery Stage 2 +// (docs/discovery-stage2-framing.md §6). + +//! `M-x` rows stop being bare names. +//! +//! `Command.description` already existed and was already rendered by +//! `help.list-commands`; it was missing at the one moment it would +//! change a decision. Carrying it to the row is two independent halves, +//! and this suite keeps them separate because they fail separately: +//! +//! - **The wire half** is a protocol bump, v22 → v23, and it is +//! *additive*. `MinibufferPrompt` is FROZEN and still sent to every +//! `12..=22` peer, because postcard encodes fields positionally — a +//! widened `candidates` would make those peers mis-decode rather than +//! ignore, and gating the widened form would have left them with no +//! minibuffer message at all. The rich shape lives in an appended +//! `MinibufferPromptRows`, and **exactly one of the two reaches any +//! peer, ever**. +//! - **The TUI half involves no wire at all.** `src/editor.rs` contains +//! zero references to `MinibufferPrompt`: `paint_minibuffer` reads +//! `core.minibuffer` directly and renders the selected candidate as an +//! inline suffix. So it reads `Command.description` from the registry +//! in-process, which is why this half is independent of the bump. +//! +//! The daemon fixtures are `crdt`-gated because a semantic session is +//! necessarily a text replica: a non-CRDT build advertises no +//! `semantic_render` and cannot host one. They run in the +//! `--features crdt` sweep that `scripts/gate --protocol` adds. + +mod common; + +use std::path::Path; + +use pmacs::bootstrap::BootstrapRoots; +use pmacs::editor::EditorState; +use pmacs_protocol::{ + ADVERTISED_PROTOCOL_VERSION, PROTOCOL_VERSION, is_supported_protocol_version, +}; + +#[cfg(feature = "crdt")] +use std::os::unix::net::UnixStream; +#[cfg(feature = "crdt")] +use std::time::{Duration, Instant}; + +#[cfg(feature = "crdt")] +use pmacs_protocol::cell::CellSize; +#[cfg(feature = "crdt")] +use pmacs_protocol::message::{ + AttachRequest, FrontendCapabilities, FrontendEvent, Hello, InstanceMessage, Key, KeyEvent, + Modifiers, SessionBootstrapRequest, +}; +#[cfg(feature = "crdt")] +use pmacs_protocol::transport::{read_message, write_message}; +#[cfg(feature = "crdt")] +use pmacs_protocol::{ByteRange, MinibufferRow}; + +#[cfg(feature = "crdt")] +use common::daemon::{TestDaemon, build_default_caps}; + +// --------------------------------------------------------------------------- +// Version-bump discipline (§6, last bullet) +// --------------------------------------------------------------------------- + +/// The bump is deliberate, and the advertised baseline does NOT move. +/// +/// `ADVERTISED_PROTOCOL_VERSION` is pinned at 20 and is the one constant +/// that must never be edited (handoff §3/§5): the handshake is +/// server-first, so moving it locks out every already-shipped frontend +/// before it can counter-offer. An additive family never needs it. +#[test] +fn the_wire_is_v23_and_the_advertised_baseline_is_unmoved() { + assert_eq!( + PROTOCOL_VERSION, 23, + "v23 is MinibufferPromptRows (Discovery Stage 2)" + ); + assert_eq!( + ADVERTISED_PROTOCOL_VERSION, 20, + "moving this is the incompatible act the counter-offer mechanism exists to avoid" + ); + // The whole v12..=22 population this lane is compatible with is + // still supported, and the set ends at the new wire — a widened set + // is a failure rather than a silent pass. + for version in 6..=23 { + assert!( + is_supported_protocol_version(version), + "v{version} must still be supported" + ); + } + assert!(!is_supported_protocol_version(24)); +} + +// --------------------------------------------------------------------------- +// The TUI half: no wire involvement (§3.4, §6) +// --------------------------------------------------------------------------- + +fn session(name: &str) -> EditorState { + let base = Path::new(env!("CARGO_TARGET_TMPDIR")) + .join("discovery-stage2") + .join(name); + let _ = std::fs::remove_dir_all(&base); + let roots = BootstrapRoots::isolated_under(&base); + for (_, dir) in roots.child_env() { + std::fs::create_dir_all(&dir).expect("create controlled root"); + } + let state = EditorState::new_with_roots(&roots); + state.install_state_dirs(); + state +} + +fn exec(s: &EditorState, src: &str) { + s.lua_host.lua().load(src.to_string()).exec().unwrap(); +} + +fn eval(s: &EditorState, src: &str) -> T { + s.lua_host.lua().load(src.to_string()).eval().unwrap() +} + +/// Render one frame at `cols` columns and return the bottom row's text. +/// +/// Through `RenderState` and the wire rather than by calling the painter +/// directly: the spans are what the TUI actually consumes, so this +/// asserts on the cells that reach a screen. +fn bottom_row(s: &EditorState, rows: u32, cols: u32) -> String { + use std::collections::HashMap; + + let size = pmacs::cell::CellSize::new(rows, cols); + let mut rs = pmacs::instance_render::RenderState::new(size); + let msgs = rs.render_frame(s, pmacs::protocol::FrontendId::LOCAL, &HashMap::new(), &[]); + + let mut row = vec![' '; cols as usize]; + for msg in &msgs { + if let pmacs_protocol::InstanceMessage::CellDelta { spans, .. } = msg { + for span in spans { + if span.start.row != rows - 1 { + continue; + } + for (i, cell) in span.cells.iter().enumerate() { + let c = span.start.col as usize + i; + if c < cols as usize + && let pmacs::cell::Glyph::Char(ch) = cell.glyph + { + row[c] = ch; + } + } + } + } + } + row.into_iter().collect::().trim_end().to_owned() +} + +/// Open `M-x`, narrowed to exactly one command with a known +/// description, and report the bottom row at `cols` columns. +fn mx_bottom_row(s: &EditorState, cols: u32) -> String { + exec( + s, + "pmacs.minibuffer.read{ prompt = 'M-x ', source = 'commands', on_accept = function() end }", + ); + exec(s, "pmacs.minibuffer.set_contents('zzprobe')"); + bottom_row(s, 24, cols) +} + +const PROBE_DESCRIPTION: &str = "Probe the description row."; + +fn define_probe(s: &EditorState) { + exec( + s, + &format!( + "pmacs.command.define{{ name = 'zzprobe', description = '{PROBE_DESCRIPTION}', \ + fn = function() end }}" + ), + ); +} + +#[test] +fn the_tui_renders_the_description_beside_the_selected_name() { + let s = session("tui-wide"); + define_probe(&s); + let row = mx_bottom_row(&s, 120); + assert!( + row.contains(&format!("[zzprobe — {PROBE_DESCRIPTION}]")), + "the selected candidate carries its description: {row:?}" + ); +} + +#[test] +fn the_tui_drops_the_description_then_the_whole_suffix_as_width_shrinks() { + // §3.4's three ORDERED steps, at the three widths that separate + // them. The guarantee is "never a PARTIAL name", which is + // achievable; "the name always survives" is not, because the prompt + // and the typed input consume the budget first. + let s = session("tui-clip"); + define_probe(&s); + + // 1. Wide: name + description. + let wide = mx_bottom_row(&s, 120); + assert!( + wide.contains(&format!("[zzprobe — {PROBE_DESCRIPTION}]")), + "wide: {wide:?}" + ); + + // 2. Room for the whole name but not the whole description: the + // description is dropped, leaving exactly today's `[name]`. No + // ellipsis stub, and no prefix of the description either. + let medium = mx_bottom_row(&s, 30); + assert!(medium.contains("[zzprobe]"), "medium: {medium:?}"); + assert!( + !medium.contains('—'), + "a description that does not fit whole is dropped entirely: {medium:?}" + ); + + // 3. Too narrow for even the whole name: the suffix vanishes. The + // assertion is that no PREFIX of the name is emitted — `[zzpr` + // would read as a different command, which is worse than nothing. + let narrow = mx_bottom_row(&s, 18); + assert!( + !narrow.contains('['), + "a suffix that cannot hold the whole name is omitted entirely: {narrow:?}" + ); + assert!( + narrow.starts_with("M-x zzprobe"), + "the prompt and the typed input still own the row: {narrow:?}" + ); + for cut in 1.."zzprobe".len() { + assert!( + !narrow.contains(&format!("[{}", &"zzprobe"[..cut])), + "no prefix of the name may be emitted: {narrow:?}" + ); + } +} + +#[test] +fn a_source_with_no_detail_renders_exactly_as_before_in_the_tui() { + // Q#D2-2: the file-path prompt is the witness. It has no detail, so + // its suffix is the pre-v23 `[name]` and nothing else. + let s = session("tui-files"); + let dir = Path::new(env!("CARGO_TARGET_TMPDIR")).join("discovery-stage2-files"); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).expect("create file-prompt dir"); + std::fs::write(dir.join("zznotes.txt"), b"x").expect("seed a file"); + exec( + &s, + &format!( + "pmacs.minibuffer.read{{ prompt = 'File: ', source = 'files', \ + source_root = '{}', on_accept = function() end }}", + dir.display() + ), + ); + exec(&s, "pmacs.minibuffer.set_contents('zznotes.txt')"); + let row = bottom_row(&s, 24, 120); + assert!(row.contains("[zznotes.txt]"), "file prompt row: {row:?}"); + assert!( + !row.contains('—'), + "a source with no detail gains no separator: {row:?}" + ); +} + +#[test] +fn typed_but_unmatched_input_is_still_accepted() { + // Q#D2-5, the trap this lane arrives with: richer rows make `M-x` + // LOOK like a closed set, which invites making acceptance reject + // unmatched input. That would be a behaviour change, and it is out + // of scope. `resolve_accepted_value` still returns the literal typed + // text when nothing is selected. + let s = session("open-set"); + exec( + &s, + "_G.ACCEPTED = nil + pmacs.minibuffer.read{ prompt = 'M-x ', source = 'commands', + on_accept = function(v) _G.ACCEPTED = v end }", + ); + exec( + &s, + "pmacs.minibuffer.set_contents('no-such-command-at-all')", + ); + assert_eq!( + eval::(&s, "return #pmacs.minibuffer.candidates()"), + 0, + "the probe input must match nothing, or this asserts the wrong thing" + ); + exec(&s, "pmacs.minibuffer.accept()"); + assert_eq!( + eval::(&s, "return _G.ACCEPTED"), + "no-such-command-at-all", + "completion is assistance, not validation" + ); +} + +// --------------------------------------------------------------------------- +// The wire half: one real daemon, two negotiated versions (§6) +// --------------------------------------------------------------------------- + +/// An `init.lua` that registers the probe command whose description the +/// wire must carry. +#[cfg(feature = "crdt")] +const PROBE_INIT: &str = r#" +pmacs.command.define { + name = "zzprobe", + description = "Probe the description row.", + fn = function() end, +} +"#; + +/// A minibuffer message, in whichever family it arrived. +#[cfg(feature = "crdt")] +#[derive(Debug)] +enum Mb { + Legacy { + prompt: Option, + candidates: Vec, + }, + Rows { + prompt: Option, + rows: Vec, + }, +} + +#[cfg(feature = "crdt")] +fn semantic_caps() -> FrontendCapabilities { + FrontendCapabilities { + multi_frontend: true, + crdt_replica: true, + semantic_render: true, + ..build_default_caps() + } +} + +/// Attach a semantic session offering exactly `offer`, declare a +/// viewport so the projection producer is live, and hand back the +/// stream plus this session's frontend id. +#[cfg(feature = "crdt")] +fn attach_semantic(daemon: &TestDaemon, offer: u32) -> (UnixStream, pmacs_protocol::FrontendId) { + let mut stream = daemon.connect(); + stream + .set_read_timeout(Some(Duration::from_secs(10))) + .expect("set read timeout"); + let hello: Hello = read_message(&mut stream).expect("read daemon Hello"); + assert_eq!( + hello.protocol_version, ADVERTISED_PROTOCOL_VERSION, + "the server-first Hello must stay at the compatibility baseline" + ); + let fid = hello.assigned_frontend_id; + write_message( + &mut stream, + &AttachRequest { + protocol_version: offer, + frontend_capabilities: semantic_caps(), + initial_size: CellSize::new(24, 80), + }, + ) + .expect("write AttachRequest"); + // A v20-or-later semantic session sends the bootstrap envelope; the + // daemon reads it unconditionally for those, so skipping it would + // desynchronize the stream. + if offer >= 20 { + write_message( + &mut stream, + &SessionBootstrapRequest { + initial_target: None, + }, + ) + .expect("write bootstrap"); + } + let document = pump(&mut stream, "first BufferSnapshot", |msg| match msg { + InstanceMessage::BufferSnapshot { buffer_id, .. } => Some(*buffer_id), + _ => None, + }); + write_message( + &mut stream, + &FrontendEvent::Viewport { + frontend_id: fid, + buffer_id: document, + visible: ByteRange { start: 0, end: 0 }, + generation: 0, + }, + ) + .expect("declare a viewport"); + (stream, fid) +} + +#[cfg(feature = "crdt")] +fn pump( + stream: &mut UnixStream, + what: &str, + mut want: impl FnMut(&InstanceMessage) -> Option, +) -> T { + let deadline = Instant::now() + Duration::from_secs(20); + while Instant::now() < deadline { + match read_message::(stream) { + Ok(msg) => { + if let Some(found) = want(&msg) { + return found; + } + } + Err(error) => panic!("{what}: read stopped: {error}"), + } + } + panic!("timed out waiting for {what}"); +} + +/// Collect every minibuffer message this session receives, up to and +/// including the first one `done` accepts. +/// +/// Collecting rather than filtering is the point: "a v23 peer receives +/// the rows form" is only half the guarantee, and the other half — that +/// it never receives the legacy form — can only be checked against +/// everything that arrived. +#[cfg(feature = "crdt")] +fn collect_minibuffer( + stream: &mut UnixStream, + what: &str, + mut done: impl FnMut(&Mb) -> bool, +) -> Vec { + let mut seen = Vec::new(); + let deadline = Instant::now() + Duration::from_secs(20); + while Instant::now() < deadline { + match read_message::(stream) { + Ok(InstanceMessage::MinibufferPrompt { + prompt, candidates, .. + }) => { + seen.push(Mb::Legacy { prompt, candidates }); + } + Ok(InstanceMessage::MinibufferPromptRows { prompt, rows, .. }) => { + seen.push(Mb::Rows { prompt, rows }); + } + Ok(_) => continue, + Err(error) => panic!("{what}: read stopped: {error}"), + } + if done(seen.last().expect("just pushed")) { + return seen; + } + } + panic!("timed out waiting for {what}; saw {seen:?}"); +} + +#[cfg(feature = "crdt")] +fn send_key(stream: &mut UnixStream, fid: pmacs_protocol::FrontendId, key: Key, mods: Modifiers) { + write_message( + stream, + &FrontendEvent::Key(KeyEvent { + frontend_id: fid, + key, + mods, + timestamp_ns: 0, + }), + ) + .expect("write key"); +} + +/// The whole exclusivity guarantee, on one live daemon: a v22 peer and a +/// v23 peer attached **simultaneously** each receive their own variant +/// and only their own — open and close alike. +/// +/// One daemon rather than two, and both directions in one fixture. Two +/// daemons could each pass their own half while the same build was +/// incapable of serving both, which is the only property that matters; +/// and a test that only proved "v23 gets rows" would pass with the +/// compatibility half broken. +#[cfg(feature = "crdt")] +#[test] +fn one_daemon_serves_a_v23_rows_session_and_a_frozen_v22_session() { + let daemon = TestDaemon::spawn_with_config(PROBE_INIT); + + // The compatibility half attaches FIRST, deliberately: it is the + // half an over-eager bump destroys, so a regression fails here + // rather than after the interesting half has already passed. + let (mut legacy, _legacy_fid) = attach_semantic(&daemon, 22); + let (mut current, current_fid) = attach_semantic(&daemon, PROTOCOL_VERSION); + assert_eq!(PROTOCOL_VERSION, 23); + + // Open the real `M-x` through the real key path, then narrow to the + // probe command by typing it — the candidate window is ten rows out + // of well over a hundred commands, so an unnarrowed prompt would + // assert nothing about the probe. + send_key(&mut current, current_fid, Key::Char('x'), Modifiers::ALT); + for ch in "zzprobe".chars() { + send_key(&mut current, current_fid, Key::Char(ch), Modifiers::NONE); + } + + let on_current = collect_minibuffer(&mut current, "v23 open", |mb| match mb { + Mb::Rows { prompt, rows } => { + prompt.is_some() && rows.iter().any(|row| row.label == "zzprobe") + } + Mb::Legacy { .. } => false, + }); + assert!( + on_current.iter().all(|mb| matches!(mb, Mb::Rows { .. })), + "a v23 peer must never receive the frozen legacy variant: {on_current:?}" + ); + let Some(Mb::Rows { rows, .. }) = on_current.last() else { + unreachable!("collect_minibuffer returns on a Rows match") + }; + let probe = rows + .iter() + .find(|row| row.label == "zzprobe") + .expect("the probe command is a candidate"); + assert_eq!( + probe.detail.as_deref(), + Some(PROBE_DESCRIPTION), + "the description reaches the row through the real prompt path" + ); + + // The same session state, seen by the v22 peer, in the frozen shape. + let on_legacy = collect_minibuffer(&mut legacy, "v22 open", |mb| match mb { + Mb::Legacy { prompt, candidates } => { + prompt.is_some() && candidates.iter().any(|c| c == "zzprobe") + } + Mb::Rows { .. } => false, + }); + assert!( + on_legacy.iter().all(|mb| matches!(mb, Mb::Legacy { .. })), + "a v22 peer must never receive the v23 rows variant: {on_legacy:?}" + ); + + // The close must arrive in the SAME family as the open. A rows + // session closed by a legacy clear leaves the dropdown on screen + // forever, and the witness for "it actually cleared" is a `prompt: + // None` in the family the frontend is mirroring. + send_key(&mut current, current_fid, Key::Escape, Modifiers::NONE); + let closed_current = collect_minibuffer(&mut current, "v23 close", |mb| { + matches!(mb, Mb::Rows { prompt: None, .. }) + }); + assert!( + closed_current + .iter() + .all(|mb| matches!(mb, Mb::Rows { .. })), + "the v23 close must not arrive as a legacy clear: {closed_current:?}" + ); + let closed_legacy = collect_minibuffer(&mut legacy, "v22 close", |mb| { + matches!(mb, Mb::Legacy { prompt: None, .. }) + }); + assert!( + closed_legacy + .iter() + .all(|mb| matches!(mb, Mb::Legacy { .. })), + "the v22 close must stay in the frozen family: {closed_legacy:?}" + ); +} diff --git a/tests/statusline_segments_acceptance.rs b/tests/statusline_segments_acceptance.rs index b583c83..a4dc5f1 100644 --- a/tests/statusline_segments_acceptance.rs +++ b/tests/statusline_segments_acceptance.rs @@ -789,7 +789,8 @@ fn a13_17_26_protocol_semantic_init_late_join_and_version_cost() { // Vterm Stage 3 appended the terminal family as v19; GPU initial targets // appended the semantic bootstrap family as v20; bottom-panel Stage 2B-1 // appended the panel family as v21; long-lines Stage 3 appended - // `LineWrapFacts` as v22. This acceptance owns the STATUSLINE + // `LineWrapFacts` as v22; Discovery Stage 2 appended + // `MinibufferPromptRows` as v23. This acceptance owns the STATUSLINE // variant's placement and gate, so it tracks the current wire version // rather than pinning 18: the v18 floor it actually cares about is asserted // below and in `peer_accepts_statusline_message`. @@ -798,11 +799,11 @@ fn a13_17_26_protocol_semantic_init_late_join_and_version_cost() { // three lines on purpose. The ceiling assertion is the load-bearing // one — it says the supported set ENDS here, which is what makes an // accidentally-widened set a failure rather than a silent pass. - assert_eq!(PROTOCOL_VERSION, 22); - for version in 6..=22 { + assert_eq!(PROTOCOL_VERSION, 23); + for version in 6..=23 { assert!(is_supported_protocol_version(version)); } - assert!(!is_supported_protocol_version(23)); + assert!(!is_supported_protocol_version(24)); let sample = InstanceMessage::StatuslineSegments { buffer_id: BufferId::from_raw(9), left: vec![StatuslineSegment { diff --git a/tests/vterm_stage3_acceptance.rs b/tests/vterm_stage3_acceptance.rs index b5a71c5..92c287b 100644 --- a/tests/vterm_stage3_acceptance.rs +++ b/tests/vterm_stage3_acceptance.rs @@ -888,9 +888,10 @@ fn terminal_mode_keeps_reporting_presence_so_peers_drop_the_stale_caret() { panic!("timed out waiting for {what}"); } - // Tripwire: a wire bump must be a conscious edit here. v22 is + // Tripwire: a wire bump must be a conscious edit here. v23 is + // `MinibufferPromptRows` (Discovery Stage 2); v22 was // `LineWrapFacts` (long-lines Stage 3). - assert_eq!(PROTOCOL_VERSION, 22); + assert_eq!(PROTOCOL_VERSION, 23); let daemon = common::daemon::TestDaemon::spawn_with_env_and_init( &[ ("PMACS_INSTANCE_SEMANTIC_RENDER", "1"),