diff --git a/docs/active-work.md b/docs/active-work.md index cbed93f..fede39e 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -270,6 +270,92 @@ hazard in a shape that looks committed. **A documented error message that never appears is worse than no documentation**, because the reader waits for a signal that is not coming. +## GUI arc Stage 1a — `TextInput` at v24 — PR #239 OPEN + +**Written with the branch's first commit**, per the standing correction +from #171 and #215. + +- **PR #239** — https://github.com/levineuwirth/pmacs/pull/239. +- **Branch `gui-stage1a-textinput`**, base `githubsucks/main` @ + `4f77491` exactly. **`githubsucks/gui-stage1a-textinput` is the + authoritative tip** — the ref, not a SHA. Recover with + `git fetch githubsucks && git checkout gui-stage1a-textinput`. +- **No new framing.** `docs/gui-stage1-input-framing.md` already governs + every Stage 1 slice; A1–A9, the eight Q#S1-9 precedence rules, §8's + wire contract and §11's gates are ruled there. Writing a 1a framing + would duplicate an approved document. +- **First commit is a GROUND-TRUTH RE-MEASUREMENT of §2 (revision 12), + not code.** §2 was taken at `a994f37`, before 1-pre moved almost every + GPU-side coordinate in it. It is refreshed here rather than in its own + PR so the contract and the code that depends on it are reviewed + together. **No ruling changes.** + - Moved: `window_event` `:2734`/655 lines → **`:4450`/four lines**; + `translate_key` `:10975` → **`:12053`**; the eight-arms description + → three family decision functions over nine variants. + - **CORRECTION, wrong at BOTH anchors: "`KeyEvent.text` is never + read" is false** — the AltGr rule reads it (`a994f37:2800`, now + `main.rs:3251`). The true claim is narrower: `text` is never read as + the text a keypress **inserts**, only as a *discriminator*. This is + load-bearing for A5, because §5's rule 2 already exempts "printable + Ctrl+Alt recognized by the existing AltGr rule" — **1a widens `text` + from discriminator to payload, and that is the change of kind the + old wording hid.** + - **CORRECTION: A4's exit site.** 1-pre moved the mechanism without + changing behaviour. **A4 edits `apply_keyboard`'s branch and return + type, not `window_event`**, and **`EventOutcome` survives A4** — a + native close still returns `Exit`. +- **PROTOCOL-BEARING at v24 and therefore SERIALIZED.** `TextInput` is + an **appended** `FrontendEvent` variant; never widen a field in place, + because postcard is positional. `PROTOCOL_VERSION` is **23** at this + base and `ADVERTISED_PROTOCOL_VERSION` stays pinned at **20** and must + not be edited to chase it. A frozen-byte pin goes on `FrontendEvent`'s + **previous final variant**, since an appended variant's own round-trip + cannot detect a discriminant shift. +- **A `PROTOCOL_VERSION` bump's blast radius is every version-sensitive + test and NONE of them appear in the diff** — handoff §1a records eight + such failures across six suites on the last bump, of which CI showed + one because cargo stops at the first failing target. **Sort them:** a + tripwire `assert_eq!(PROTOCOL_VERSION, N)` is meant to fire; an + absolute contract expressed as arithmetic on a moving constant is a + defect. **`ADVERTISED_PROTOCOL_VERSION == 20` must NOT fire.** +- **IMPLEMENTED. All sixteen gates green** under an isolated `TMPDIR` + (log `20260812T204615Z-215223`): + + ``` + ./scripts/gate --protocol \ + --acceptance gui_stage1a_acceptance \ + --acceptance gui_stage1a_wire_acceptance \ + --acceptance auto_pair_acceptance \ + --acceptance discovery_stage2_acceptance \ + --acceptance statusline_segments_acceptance \ + --acceptance vterm_stage3_acceptance + ``` + + **`--protocol` is required** — 1a changes the wire — and it is what + adds the crdt build and the second sweep. `gui_stage1a_wire_acceptance` + is `#![cfg(feature = "crdt")]`, so it runs **2 tests in the crdt sweep + and 0 in the default one**; checked in the logs rather than assumed, + because a suite that compiles to nothing reports `ok`. +- **Evidence: 8 + 2 acceptance rows, 6 producer/router rows, and the + A1–A4 unit witnesses; mutations M-1a-1 … M-1a-6b, each failing the row + it targets.** + - `M-1a-1` single-scalar back through the generic insert → the record + row **and** the auto-pair row. + - `M-1a-2` `break_command_chain` deleted → the primed-chain row alone. + - `M-1a-3` `TextInput` selection back below the intercept return → + the intercepting-producer row alone. + - `M-1a-4` typed text through `encode_paste` → A8, with the forbidden + bytes at the PTY. + - `M-1a-5` inbound gate disabled → the v23 row, with `REFUSED` inside + the CRDT op. + - `M-1a-6b` a variant inserted **before** `PanelPointer` → the + frozen-byte pin, discriminant 15 → 16. +- **`M-1a-6` (the first attempt) was a WRONG MUTATION, not a vacuous + pin**, and is recorded because the failure mode is instructive: the + wedge went in *after* `PanelPointer` — where an append belongs — so + nothing shifted and the pin passed, correctly. A mutation aimed at the + wrong side of a boundary reports a sound pin as worthless. + ## GUI arc Stage 1 — 1-pre MERGED as #237 (`d038f71`); 1a is next, NOT STARTED **The lane is rewritten, not removed.** Rule 4 removes a lane when its diff --git a/docs/gui-stage1-input-framing.md b/docs/gui-stage1-input-framing.md index c5553ff..fd1a647 100644 --- a/docs/gui-stage1-input-framing.md +++ b/docs/gui-stage1-input-framing.md @@ -1,47 +1,108 @@ # GUI arc, Stage 1 — input foundation (framing) -**Status: revision 11 — APPROVED.** Revisions 1–8 rejected; revision 9 +**Status: revision 12 — APPROVED.** Revision 12 is §2's ground-truth +re-measurement for Stage 1a and changes no ruling; it carries two +corrections to claims that were wrong at the original anchor too. + +**Previously, revision 11 — APPROVED.** Revisions 1–8 rejected; revision 9 is the approved design. Revision 10 recorded a scope correction found against the 1-pre implementation and **also made a claim about P2 that review overturned; revision 11 retracts it and P2 is implemented as written** (§6). **Q#S1-8, Q#S1-9 and Q#S1-10 are RULED.** **1-pre is IMPLEMENTED**; 1a onward may begin from this document. -**Verification base:** checked in the `gui-arc-stage0` worktree at -`a994f37`, whose tree for these files is what `f8ad3e7` merged. +**Verification base:** §2 is **re-measured at `4f77491`** (2026-08-12), +the tip after 1-pre; it was originally taken at `a994f37`. Sections +other than §2 were written against `a994f37` and their *rulings* are +unaffected by 1-pre, which changed no behaviour — but **any line number +outside §2 predates 1-pre and should be re-checked before it is relied +on.** ## 1. What this stage closes Journey **step 5**. **Not step 12** (Stage 4b, P2-gated). Five of nine §3.1 blockers die here. -## 2. Ground truth at `a994f37` +## 2. Ground truth — RE-MEASURED at `4f77491` (2026-08-12) -`App::window_event` (`main.rs:2734`) is **655 lines**; **eight** -`WindowEvent` arms handled, the rest fall to `_`. +**Originally taken at `a994f37`, before 1-pre.** 1-pre (#237) moved +almost every GPU-side coordinate below, so the section is re-measured +rather than left to rot — **a framing whose ground truth points at the +wrong lines is how an implementation ends up arguing with the tree**. +Two claims were *wrong at both anchors* and are corrected, not merely +renumbered; they are marked **CORRECTION**. -**Two text producers.** `translate_key(logical: &Key, …)` -(`main.rs:10975`) reads the **logical key** and truncates via -`chars().next()`; **`KeyEvent.text` is never read.** -`WindowEvent::Ime::Commit(String)` is a **separate event, ignored -entirely** (`set_ime_allowed`/`WindowEvent::Ime`: zero occurrences). -**Today's two failures are therefore: multi-scalar keyboard input is -truncated to its first scalar, and an IME commit produces nothing.** +### Still true, re-checked -**`FrontendEvent`: sixteen variants, none carrying an open path or -command invocation.** `PROTOCOL_VERSION = 23`. +- **`FrontendEvent`: sixteen variants**, none carrying an open path or + command invocation. **`PROTOCOL_VERSION = 23`.** +- **`WindowEvent::Ime` is ignored entirely** — + `set_ime_allowed`/`WindowEvent::Ime` still **zero occurrences**, so + `Ime::Commit(String)` produces nothing. (1d's D1.) +- **TUI wheel arms unmoved**: `EditorState::dispatch_mouse` + (`src/editor.rs:3052`), `ScrollUp`/`ScrollDown` at **`:3203`** — 1-pre + touched only `pmacs-gpu`. +- **The handshake precedes any window**: the client is constructed and + the handshake done before `run_app` (connect `main.rs:702`, `run_app` + `main.rs:733`; the old citation `:696` was the enclosing block). +- **1c is producer-side only for Focus/Detach** — those variants exist + on the wire. **Title, Bell and `Goodbye` are GPU consumer work.** +- **`Outbox::enqueue` returns `false` once closed** and **coalesces by + kind** (`attach.rs:415`; the old `:414` was off by one at both + anchors). -**The handshake precedes any window** — `EventLoop` built, client -constructed, handshake done **before `run_app`** (`main.rs:696`). +### Moved by 1-pre -**TUI wheel arms**: `EditorState::dispatch_mouse` (`src/editor.rs:3052`), -`ScrollUp`/`ScrollDown` at **`:3203`**. +- **`App::window_event` is `main.rs:4450` and is FOUR lines**, not 655 + at `:2734`. It calls `dispatch_window_event` and performs the exit; + **routing lives in `route_event`, and the bodies in seven `apply_*` + methods.** +- **"Eight arms handled, the rest fall to `_`" is now three family + decision functions** — `route_lifecycle`, `route_keyboard` (plus + `route_key_action`), `route_pointer` — over **nine** named + `WindowEvent` variants, with `Route::Unrouted` as the wildcard. **1a + edits `apply_keyboard` and `translate_key`, not `window_event`.** +- **`translate_key(logical: &Key, …)` is `main.rs:12053`**, not + `:10975`. It still reads the **logical key** and still truncates via + `chars().next()`, and `_ => return None` is still there — so **A1's + witness holds**. -**1c is producer-side only for Focus/Detach** — those variants exist on -the wire. **Title, Bell and `Goodbye` are GPU consumer work.** +### CORRECTION 1 — `KeyEvent.text` IS read, and always was -**`Outbox::enqueue` returns `false` once closed** (`attach.rs:414`) and -**coalesces by kind**. +The original section said *"`KeyEvent.text` is never read."* **That is +false, and was false at `a994f37` too** (`:2800` there, `main.rs:3251` +now): the AltGr rule reads it, as `is_layout_text(key.text.as_deref(), +pmods)`. + +The claim the section meant, and which is true: **`KeyEvent.text` is +never read as the text a keypress INSERTS.** It is consulted only as a +*discriminator* — Ctrl+Alt plus printable text means AltGr rather than a +command chord — and the inserted character always comes from +`translate_key`'s logical key, truncated to one scalar. + +**This matters to A5, not just to accuracy.** §5's rule 2 exempts +"printable Ctrl+Alt recognized by the existing AltGr rule", so the +precedence table already depends on the code the section claimed did not +exist. **1a widens `text` from a discriminator to a payload**, and that +is the actual change of kind — stating it as "text is never read" hides +the one place the new payload must not disturb. + +### CORRECTION 2 — A4's exit site + +A4's witness cited *"exits (`main.rs:2771`)"*. 1-pre moved the mechanism +without changing the behaviour: an idle Escape still exits, but +`apply_keyboard` (`main.rs:3219`) now returns `EventOutcome::Exit` and +**`window_event` (`main.rs:4452`) performs the only executable +`event_loop.exit()` in the crate.** + +**A4 therefore deletes a branch in `apply_keyboard` and changes its +return type — it does not touch `window_event`.** And **`EventOutcome` +survives A4**: a native close still returns `Exit`, and +`dispatch_window_event` must still distinguish it from `Continue`. + +**Today's two failures are unchanged by any of this:** multi-scalar +keyboard input is truncated to its first scalar, and an IME commit +produces nothing. ## 3. PR topology @@ -229,7 +290,7 @@ The crate has **exactly one** executable `event_loop.exit()`, in | A1 | `F1`–`F35` → `F(1..=35)` | `_ => return None` | map `F13+` → `None` → F13–F35 rows | | A2 | Shift+Tab → `BackTab` with `Shift` set | produces `Tab` | drop `Shift` → A2 only | | A3 | `ContextMenu` → `Menu` | produces nothing | map to `Char('\0')` → A3 only | -| A4 | Idle Escape reaches the daemon, never exits | exits (`main.rs:2771`) | restore the quit branch → A4 only | +| A4 | Idle Escape reaches the daemon, never exits | exits — `apply_keyboard` (`main.rs:3219`) returns `EventOutcome::Exit`, performed at `main.rs:4452` | restore the quit branch → A4 only | | A5 | Precedence per §5 (1–8) | multi-scalar truncated; IME ignored | move rule 1 (control text → text) → the `Enter`-in-dired row | | A6 | One commit = one edit, undo unit, hook, eligible CRDT op | commit truncated to one scalar | one edit per scalar → undo-unit row (**and D3 surfaces here**) | | A7 | Prompts consume scalars **in order** | multi-scalar never arrives | reverse order → A7's prompt transcript | diff --git a/pmacs-gpu/src/attach.rs b/pmacs-gpu/src/attach.rs index 5a75a0a..94a3bd8 100644 --- a/pmacs-gpu/src/attach.rs +++ b/pmacs-gpu/src/attach.rs @@ -960,6 +960,36 @@ impl AttachClient { })) } + /// Send a `FrontendEvent::TextInput` (GUI arc 1a): text the user + /// committed, by keypress or IME. + /// + /// **The caller must gate on [`Self::session_protocol_version`] + /// `>= TEXT_INPUT_MIN_VERSION` and fall back to `Key`** — this + /// method does not check, because the fallback needs the untranslated + /// key and only the caller has it. Withholding is the whole + /// old-peer contract: a `< 24` daemon keeps the behaviour it has, + /// including today's first-scalar truncation. + /// + /// Rejects oversize here as well as at the daemon, so a payload + /// that could never be accepted is not written to the socket at + /// all; rejection rather than truncation, per `TEXT_INPUT_MAX_BYTES`. + pub fn send_text_input(&self, text: &str) -> Result<(), TransportError> { + if text.len() > pmacs_protocol::TEXT_INPUT_MAX_BYTES { + return Err(TransportError::Io(io::Error::new( + io::ErrorKind::InvalidInput, + format!( + "TextInput of {} bytes exceeds the {}-byte cap", + text.len(), + pmacs_protocol::TEXT_INPUT_MAX_BYTES + ), + ))); + } + self.send_event(FrontendEvent::TextInput { + frontend_id: self.frontend_id, + text: text.to_owned(), + }) + } + /// Send a `FrontendEvent::Pointer` (session M-2): a locally /// hit-tested gesture in source bytes. Callers gate on /// [`Self::session_protocol_version`] `>= 5`. diff --git a/pmacs-gpu/src/main.rs b/pmacs-gpu/src/main.rs index 3b16815..c42fd94 100644 --- a/pmacs-gpu/src/main.rs +++ b/pmacs-gpu/src/main.rs @@ -46,8 +46,8 @@ use pmacs_protocol::{ MAX_STATUSLINE_FACE_BYTES, MAX_STATUSLINE_PROVIDERS, MAX_STATUSLINE_SEGMENT_BYTES, MAX_STATUSLINE_TOTAL_TEXT_BYTES, MenuPromptRow, MinibufferRow, Modifiers, MouseButton as ProtocolMouseButton, MouseKind as ProtocolMouseKind, PointerKind, - SelectionSnapshot, StatuslineSegment, StyleSegment, StyleSpan, TAB_STOP_COLUMNS, TerminalFrame, - UnderlineStyle, + SelectionSnapshot, StatuslineSegment, StyleSegment, StyleSpan, TAB_STOP_COLUMNS, + TEXT_INPUT_MIN_VERSION, TerminalFrame, UnderlineStyle, cell::{Color as CellColor, Style as CellStyle}, is_builtin_pair_char, is_modeline_face_name, panel::{PANEL_MIN_VERSION, PanelFrame, PanelFramePayload}, @@ -3162,7 +3162,7 @@ impl App { Route::Keyboard { action: KeyAction::Press, key, - } => return self.apply_keyboard(key), + } => self.apply_keyboard(&key.logical_key, key.text.as_deref()), Route::Pointer(PointerRoute::Moved { x, y }) => self.apply_cursor_moved(x, y), Route::Pointer(PointerRoute::Left(button_state)) => { self.apply_left_button(button_state); @@ -3183,10 +3183,25 @@ impl App { EventOutcome::Continue } - /// Perform [`KeyAction::Press`]. The router has already - /// discarded key-ups, so `key` is always a press. + /// Perform [`KeyAction::Press`]. The router has already discarded + /// key-ups, so this is always a press. + /// + /// **Takes the two fields it reads rather than the whole + /// `KeyEvent`, deliberately.** `KeyEvent` carries a `pub(crate)` + /// field and cannot be constructed outside winit, so a body taking + /// one is undrivable by any test; `Key` and `&str` are ordinary + /// values. That distinction is not academic — 1a's first review + /// found the `TextInput` selection sited *below* the intercept + /// return, making A7 and A8 reachable only when no prompt and no + /// terminal were present. The classifier was correct throughout; + /// only the call site was wrong, so only a test that drives THIS + /// function could have caught it. + /// + /// The router arm remains unwitnessable — it still cannot be handed + /// a `WindowEvent::KeyboardInput` — so 1-pre's structural exception + /// narrows to that one pattern arm rather than disappearing. #[allow(clippy::too_many_lines)] // one linear key pipeline; splitting hides the order. - fn apply_keyboard(&mut self, key: &KeyEvent) -> EventOutcome { + fn apply_keyboard(&mut self, logical: &Key, text: Option<&str>) { // While the daemon is intercepting keystrokes — an active // incremental search (Q#SR5), or a minibuffer / pending // prefix — every key belongs to its handler, not the @@ -3213,29 +3228,33 @@ impl App { .as_ref() .is_some_and(State::completion_open_for_current_buffer); - // Escape cancels an active intercept (e.g. a running - // search) or dismisses the completion popup; otherwise it - // stays the local quit. - if matches!(key.logical_key, Key::Named(NamedKey::Escape)) { - if intercept || completion_open { - if let Some(client) = self.attach_client.as_ref() - && let Err(e) = client.send_key(ProtocolKey::Escape, Modifiers::NONE) - { - eprintln!("pmacs-gpu: send Escape (cancel) failed: {e}"); - } - } else { - // Q#S1-1 / A4 — the local quit, unchanged here and - // deleted by Stage 1a: an idle Escape must reach the - // daemon. `window_event` performs the exit. This is the - // only reason a BODY needs an outcome; `EventOutcome` - // itself outlives A4, since a native close still exits. - return EventOutcome::Exit; + // A4 / Q#S1-1 — **every** Escape reaches the daemon, and none + // exits. + // + // It used to quit the frontend when nothing was intercepting, + // which made the GUI's most common "get me out of this" key + // destroy the window instead of cancelling. Q#S1-1 settles the + // exits and Escape is not among them: a native close detaches + // this frontend, `editor.quit` shuts the daemon and its + // attachments down, and **Escape only cancels or round-trips**. + // + // The `intercept || completion_open` test went with the quit + // branch. It never decided what to SEND — both arms sent the + // same `Escape` — only whether to send at all, so with one + // behaviour left there is nothing for it to choose. (Both flags + // remain live below, for the OS-paste, round-trip and + // completion-accept paths.) + if matches!(*logical, Key::Named(NamedKey::Escape)) { + if let Some(client) = self.attach_client.as_ref() + && let Err(e) = client.send_key(ProtocolKey::Escape, Modifiers::NONE) + { + eprintln!("pmacs-gpu: send Escape failed: {e}"); } - return EventOutcome::Continue; + return; } - let Some((pkey, mut pmods)) = translate_key(&key.logical_key, self.modifiers) else { - return EventOutcome::Continue; + let Some((pkey, mut pmods)) = translate_key(logical, self.modifiers) else { + return; }; // AltGr / international text (audit F-004). winit reports @@ -3248,7 +3267,7 @@ impl App { // being routed to the keymap. Alt alone is left intact so // macOS Option-as-Meta still reaches the keymap; on layouts // where AltGr isn't Ctrl+Alt this is a no-op. - if matches!(pkey, ProtocolKey::Char(_)) && is_layout_text(key.text.as_deref(), pmods) { + if matches!(pkey, ProtocolKey::Char(_)) && is_layout_text(text, pmods) { pmods = if pmods.contains(Modifiers::SHIFT) { Modifiers::SHIFT } else { @@ -3271,13 +3290,57 @@ impl App { { eprintln!("pmacs-gpu: send_paste failed: {e}"); } - return EventOutcome::Continue; + return; } let Some(client) = self.attach_client.as_ref() else { - return EventOutcome::Continue; + return; }; + // A5 — multi-scalar text travels as ONE `TextInput`, if the + // session can carry it. + // + // **This MUST precede the intercept branch below, and that + // placement is the contract rather than a preference.** A + // modal prompt or a focused terminal is exactly what makes + // `daemon_intercepts_keys` true, so classifying after it would + // leave A7 (prompts consume scalars in order) and A8 (terminals + // take raw UTF-8) reachable only when neither a prompt nor a + // terminal is present — which is to say, never. Sited here, the + // producer sends the same `TextInput` in every state and the + // daemon's `dispatch_text_input` applies §5's modal precedence, + // which is where that decision belongs: the frontend cannot see + // which shadow is up. + // + // Ordering against the branches below is safe by construction, + // not by luck: `text_input_payload` returns `None` whenever a + // command modifier is held, so the Ctrl-V paste and + // command-chord paths can never be shadowed by it. + // + // **The version gate WITHHOLDS rather than degrades.** A `< 24` + // daemon keeps exactly the behaviour it has — including today's + // truncation to the first scalar — because the fallback is the + // unchanged `Key` path below. No regression, not retroactive + // correctness. + if let Some(text) = text_input_payload(logical, text, pmods) + && client.session_protocol_version() >= TEXT_INPUT_MIN_VERSION + { + if let Some(state) = self.state.as_mut() { + state.mark_cursor_stale_after_round_trip(); + } + if debug_input() { + eprintln!( + "pmacs-gpu send_text_input: {} scalars", + text.chars().count() + ); + } + let client = self.attach_client.as_ref().expect("client checked above"); + if let Err(e) = client.send_text_input(text) { + eprintln!("pmacs-gpu: send_text_input failed: {e}"); + } + return; + } + // Intercept path: round-trip every key into the daemon's // active handler (search query / step / accept / cancel). if intercept { @@ -3290,7 +3353,7 @@ impl App { if let Err(e) = client.send_key(pkey, pmods) { eprintln!("pmacs-gpu: send_key (intercepted) failed: {e}"); } - return EventOutcome::Continue; + return; } // Idle: forward any command chord (Char/Enter/Tab with @@ -3311,7 +3374,7 @@ impl App { if let Err(e) = client.send_key(pkey, pmods) { eprintln!("pmacs-gpu: send_key (command chord) failed: {e}"); } - return EventOutcome::Continue; + return; } // Session B2 forwards cursor motion + plain text editing @@ -3320,7 +3383,7 @@ impl App { // here and are withheld, leaving OS/WM shortcuts (Cmd-Q, // Cmd-C) to the platform. if !should_forward_key(pkey, pmods) { - return EventOutcome::Continue; + return; } // Arc 1a Q#C6 — with the popup open, RET and TAB mean @@ -3357,7 +3420,7 @@ impl App { { eprintln!("pmacs-gpu: send Viewport failed: {e}"); } - return EventOutcome::Continue; + return; } if let Some(state) = self.state.as_mut() { if state.defer_round_trip_key_if_needed(pkey, pmods) { @@ -3367,7 +3430,7 @@ impl App { pending optimistic cursor" ); } - return EventOutcome::Continue; + return; } state.mark_cursor_stale_after_round_trip(); } @@ -3377,7 +3440,6 @@ impl App { if let Err(e) = client.send_key(pkey, pmods) { eprintln!("pmacs-gpu: send_key failed: {e}"); } - EventOutcome::Continue } } @@ -3418,23 +3480,19 @@ enum Route<'a> { /// What the event loop must do once a family's body has run. /// -/// **Two producers, and they are not the same kind of thing.** -/// `LifecycleRoute::Exit` is a native window close, which must always -/// exit; `apply_keyboard` returns `Exit` for an idle Escape, which is a -/// local quit. Returning the decision rather than taking an -/// `&ActiveEventLoop` is what keeps every body reachable from a test: -/// the crate has **exactly one** executable `event_loop.exit()`, in -/// `window_event`. +/// **Since A4, `LifecycleRoute::Exit` — a native window close — is the +/// SOLE producer.** `apply_keyboard` used to be the second, for the +/// idle-Escape local quit; A4 deleted that branch and with it the +/// keyboard body's need to return anything, so it returns `()` and the +/// obsolete channel is gone rather than merely unused. /// -/// **Stage 1a's A4 removes the KEYBOARD producer only** — an idle -/// Escape must reach the daemon and never exit — leaving **one** `Exit` -/// producer, the native close. -/// -/// **One producer is not one variant.** This type survives A4 because +/// **One producer is not one variant.** The type stays because /// `dispatch_window_event` must still distinguish `Continue` from -/// `Exit` on every event it handles: nearly all of them must not exit, -/// and the close must. What A4 changes is `apply_keyboard`'s signature, -/// not this type. +/// `Exit` on every event it handles: nearly all must not exit, and the +/// close must. Returning the decision rather than taking an +/// `&ActiveEventLoop` is also what keeps the bodies reachable from a +/// test — the crate has exactly one executable `event_loop.exit()`, in +/// `window_event`. #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum EventOutcome { Continue, @@ -3792,12 +3850,42 @@ impl EffectHarness { harness } + /// Drive `apply_keyboard` directly and report what it produced. + /// + /// Bypasses `route_event` because a `WindowEvent::KeyboardInput` + /// cannot be constructed outside winit — 1-pre's recorded + /// structural exception. What that exception covers is the router's + /// pattern arm; the BODY is reachable, and the body is where 1a's + /// placement defect lived. + fn feed_keyboard(&mut self, logical: &Key, text: Option<&str>) -> Step { + let before = self.snapshot(); + self.app.apply_keyboard(logical, text); + let after = self.snapshot(); + Step { + local: Self::diff(&before, &after, EventOutcome::Continue), + outbound: self.read_until_sentinel(), + } + } + /// Dispatch one event and report everything it did. fn feed(&mut self, event: &WindowEvent) -> Step { let before = self.snapshot(); let outcome = self.app.dispatch_window_event(event); let after = self.snapshot(); + Step { + local: Self::diff(&before, &after, outcome), + outbound: self.read_until_sentinel(), + } + } + + /// The local effects between two snapshots. Shared by every entry + /// point so a new one cannot observe a different set by accident. + fn diff( + before: &EffectSnapshot, + after: &EffectSnapshot, + outcome: EventOutcome, + ) -> Vec { let mut local = Vec::new(); if outcome == EventOutcome::Exit { local.push(LocalEffect::Exit); @@ -3819,11 +3907,7 @@ impl EffectHarness { top: after.scroll_top, }); } - - Step { - local, - outbound: self.read_until_sentinel(), - } + local } /// Observable state the local effects are derived from. @@ -4350,6 +4434,118 @@ mod input_routing_tests { ); } + // --------------------------------------------------------------- + // GUI arc 1a — PRODUCER REACHABILITY. + // + // These drive `App::apply_keyboard`, the real call site, not + // `text_input_payload`. 1a's first review found the classifier + // correct and the call site wrong: `TextInput` selection sat below + // the intercept return, and a modal prompt or a focused terminal is + // exactly what makes intercept true, so A7 and A8 were reachable + // only when neither was present. A classifier test stays green + // through that defect; these rows do not. + // --------------------------------------------------------------- + + /// Multi-scalar text reaches the wire as `TextInput` **while the + /// daemon is intercepting** — the state a modal prompt puts the + /// session in. + #[test] + fn multi_scalar_text_is_sent_while_the_daemon_intercepts() { + let mut h = EffectHarness::new(); + // A minibuffer is up: `daemon_intercepts_keys` is true. + h.app.state.as_mut().expect("harness state").dispatch_idle = false; + + let step = h.feed_keyboard(&Key::Character("e\u{301}".into()), Some("e\u{301}")); + + assert!( + step.outbound.iter().any(|e| matches!( + e, + pmacs_protocol::FrontendEvent::TextInput { text, .. } if text == "e\u{301}" + )), + "an intercepting session must still get the whole commit, \ + not a truncated Key; got {:?}", + step.outbound + ); + assert!( + !step + .outbound + .iter() + .any(|e| matches!(e, pmacs_protocol::FrontendEvent::Key(_))), + "and must NOT also get the first-scalar Key: {:?}", + step.outbound + ); + } + + /// A4 — an IDLE Escape reaches the daemon and does not exit. + /// + /// "Idle" is the case that used to quit: with nothing intercepting, + /// Escape destroyed the window instead of cancelling. Both halves + /// are asserted, because either alone would pass a wrong + /// implementation — sending Escape while also exiting, or not + /// exiting while also sending nothing. + #[test] + fn a4_an_idle_escape_reaches_the_daemon_and_does_not_exit() { + let mut h = EffectHarness::new(); + // Establish idle explicitly. A fresh `State` starts with + // `dispatch_idle` false — the daemon has not said otherwise yet + // — so ASSERTING the precondition rather than setting it would + // have tested the intercepting case under an idle name. It + // failed exactly that way first. + h.app.state.as_mut().expect("harness state").dispatch_idle = true; + assert!( + !h.app + .state + .as_ref() + .expect("harness state") + .daemon_intercepts_keys(), + "precondition: nothing intercepts, which is the case that quit" + ); + + let step = h.feed_keyboard(&Key::Named(NamedKey::Escape), None); + + assert!( + step.outbound.iter().any(|e| matches!( + e, + pmacs_protocol::FrontendEvent::Key(k) if k.key == ProtocolKey::Escape + )), + "an idle Escape must reach the daemon: {:?}", + step.outbound + ); + assert!( + !step.local.contains(&LocalEffect::Exit), + "and must NOT exit: {:?}", + step.local + ); + } + + /// The complement, so the row above cannot pass by sending + /// `TextInput` for everything: a SINGLE scalar while intercepting + /// still travels as `Key`, which is §5 rule 4 and preserves mode + /// keymaps and typed provenance. + #[test] + fn single_scalar_text_still_travels_as_key_while_intercepting() { + let mut h = EffectHarness::new(); + h.app.state.as_mut().expect("harness state").dispatch_idle = false; + + let step = h.feed_keyboard(&Key::Character("a".into()), Some("a")); + + assert!( + step.outbound + .iter() + .any(|e| matches!(e, pmacs_protocol::FrontendEvent::Key(_))), + "a single scalar stays a Key: {:?}", + step.outbound + ); + assert!( + !step + .outbound + .iter() + .any(|e| matches!(e, pmacs_protocol::FrontendEvent::TextInput { .. })), + "and must not become TextInput: {:?}", + step.outbound + ); + } + /// P2 — the harness records a transcript, and the transcript /// distinguishes every routed effect from the others and from an /// unclaimed event. **Two of these rows produce no outbound traffic @@ -12070,9 +12266,26 @@ fn translate_key( NamedKey::Enter => ProtocolKey::Enter, NamedKey::Delete => ProtocolKey::Delete, NamedKey::Insert => ProtocolKey::Insert, + // A2 — Shift+Tab is `BackTab`, a key of its own, and the + // Shift stays set. The daemon's keymap binds the two + // differently (indent versus outdent), and a `Tab` that + // merely carries Shift is indistinguishable from a Tab the + // user shifted by accident. The TUI has always sent + // `BackTab`; this closes the divergence rather than + // inventing a convention. + NamedKey::Tab if mods.shift_key() => ProtocolKey::BackTab, NamedKey::Tab => ProtocolKey::Tab, NamedKey::Space => ProtocolKey::Char(' '), - _ => return None, + // A3 — the menu key. `ProtocolKey::Menu` already exists and + // the TUI already sends it; only the GPU translation was + // missing, so the key did nothing in the GUI. + NamedKey::ContextMenu => ProtocolKey::Menu, + // A1 — F1..=F35. winit names each as its own variant, so + // there is no arithmetic to do and no range to trust: the + // mapping is total over the variants winit defines, and + // `named_function_key` is exhaustive rather than a + // computed offset. + named => named_function_key(*named).map(ProtocolKey::F)?, }, Key::Character(s) => ProtocolKey::Char(s.chars().next()?), _ => return None, @@ -12080,6 +12293,112 @@ fn translate_key( Some((pkey, pmods)) } +/// A5 / Q#S1-9 — whether a keypress should travel as `TextInput` +/// rather than `Key`, and with what payload. +/// +/// **A keypress stays `Key` unless a rule below moves it.** That +/// default is the contract, not an implementation convenience: every +/// mode keymap, every command chord and today's typed provenance are +/// built on `Key`, so widening the exception is how a working binding +/// silently becomes an insert. +/// +/// The rules, in the order they are checked: +/// +/// 1. **Named keys and control text stay `Key`**, whatever +/// `KeyEvent.text` says — `Enter` reports `"\r"`, and an `Enter` +/// that arrived as text would insert a newline in dired instead of +/// opening a file. +/// 2. **Ctrl/Alt chords stay `Key`**, except printable Ctrl+Alt that +/// the existing `AltGr` rule already recognizes — the caller strips +/// those modifiers before this is reached, so they arrive here as +/// plain text. +/// 3. **Meta/Super-only text stays reserved to the OS** (Q#S1-7 moves +/// the whole question to Stage 2). +/// 4. **Plain printable SINGLE-scalar stays `Key`.** This is the +/// conservative half of the ruling: it preserves mode keymaps and +/// the one-codepoint typed provenance that already exists. +/// 5. **Printable MULTI-scalar becomes one `TextInput`** — the case +/// that is broken today, truncated to its first scalar. +/// +/// `Key::Dead` is 1d's, and 1a buffers nothing (rule 7); `Shift` is +/// already baked into the resolved text and is not carried (rule 8). +fn text_input_payload<'a>( + logical: &Key, + text: Option<&'a str>, + mods: Modifiers, +) -> Option<&'a str> { + // Rule 1 — a named key is never text, regardless of what winit + // reports as its text. + if matches!(logical, Key::Named(_)) { + return None; + } + // Rules 2 and 3 — any command modifier still held at this point is + // a chord. The AltGr case has already had its modifiers stripped by + // the caller, so reaching here with Ctrl/Alt means a genuine chord. + if !is_plain_text_modifiers(mods) { + return None; + } + let text = text?; + // Rule 1, second half — control text is not text. + if text.is_empty() || text.chars().any(char::is_control) { + return None; + } + // Rules 4 and 5 — the single/multi split. + if text.chars().count() < 2 { + return None; + } + Some(text) +} + +/// A1 — winit's function-key variants to the protocol's 1-based `F(n)`. +/// +/// Written as an exhaustive match rather than parsed from the variant +/// name or computed as an offset from `F1`: winit's `NamedKey` is +/// `#[non_exhaustive]` and its ordering is not a contract, so arithmetic +/// over it would be a silent-corruption bug the day a variant is +/// inserted. `None` for anything that is not a function key, which is +/// what makes the caller's `?` fall through to "unmapped". +fn named_function_key(named: NamedKey) -> Option { + Some(match named { + NamedKey::F1 => 1, + NamedKey::F2 => 2, + NamedKey::F3 => 3, + NamedKey::F4 => 4, + NamedKey::F5 => 5, + NamedKey::F6 => 6, + NamedKey::F7 => 7, + NamedKey::F8 => 8, + NamedKey::F9 => 9, + NamedKey::F10 => 10, + NamedKey::F11 => 11, + NamedKey::F12 => 12, + NamedKey::F13 => 13, + NamedKey::F14 => 14, + NamedKey::F15 => 15, + NamedKey::F16 => 16, + NamedKey::F17 => 17, + NamedKey::F18 => 18, + NamedKey::F19 => 19, + NamedKey::F20 => 20, + NamedKey::F21 => 21, + NamedKey::F22 => 22, + NamedKey::F23 => 23, + NamedKey::F24 => 24, + NamedKey::F25 => 25, + NamedKey::F26 => 26, + NamedKey::F27 => 27, + NamedKey::F28 => 28, + NamedKey::F29 => 29, + NamedKey::F30 => 30, + NamedKey::F31 => 31, + NamedKey::F32 => 32, + NamedKey::F33 => 33, + NamedKey::F34 => 34, + NamedKey::F35 => 35, + _ => return None, + }) +} + /// Cursor-motion keys — forwarded with any modifier set (e.g. `C-Left` /// is word-motion, `S-Down` extends a selection; the daemon's keymap /// decides). @@ -12117,6 +12436,20 @@ fn should_forward_key(key: ProtocolKey, mods: Modifiers) -> bool { if matches!(key, ProtocolKey::Backspace | ProtocolKey::Delete) { return true; } + // A1–A3 — function keys, `BackTab` and `Menu` forward with ANY + // modifier set, for the same reason motion keys do: they are + // command keys that never insert text, so the chord-withholding + // rule below has nothing to protect them from. Translating them + // without forwarding them would have been the more expensive + // mistake — the key would map correctly and still do nothing, + // which reads as a daemon-side keymap gap rather than a frontend + // one. + if matches!( + key, + ProtocolKey::F(_) | ProtocolKey::BackTab | ProtocolKey::Menu + ) { + return true; + } if !is_plain_text_modifiers(mods) { return false; } @@ -13418,6 +13751,105 @@ mod tests { assert_eq!(source_line_range(text, 99), (7, 10)); } + /// A1 — every function key winit names maps to the protocol's + /// 1-based `F(n)`, and forwards. + /// + /// Written as an exhaustive loop over all 35 rather than spot + /// checks: the old code stopped at `_ => return None`, so F13+ + /// silently did nothing, and a test covering only F1–F12 would have + /// passed against exactly that. + #[test] + fn a1_function_keys_f1_to_f35_map_and_forward() { + use winit::keyboard::{Key as WKey, ModifiersState, NamedKey}; + + let named = [ + NamedKey::F1, + NamedKey::F2, + NamedKey::F3, + NamedKey::F4, + NamedKey::F5, + NamedKey::F6, + NamedKey::F7, + NamedKey::F8, + NamedKey::F9, + NamedKey::F10, + NamedKey::F11, + NamedKey::F12, + NamedKey::F13, + NamedKey::F14, + NamedKey::F15, + NamedKey::F16, + NamedKey::F17, + NamedKey::F18, + NamedKey::F19, + NamedKey::F20, + NamedKey::F21, + NamedKey::F22, + NamedKey::F23, + NamedKey::F24, + NamedKey::F25, + NamedKey::F26, + NamedKey::F27, + NamedKey::F28, + NamedKey::F29, + NamedKey::F30, + NamedKey::F31, + NamedKey::F32, + NamedKey::F33, + NamedKey::F34, + NamedKey::F35, + ]; + for (index, key) in named.into_iter().enumerate() { + let n = u8::try_from(index + 1).expect("1..=35 fits"); + let (k, m) = translate_key(&WKey::Named(key), ModifiersState::empty()) + .unwrap_or_else(|| panic!("F{n} must map")); + assert_eq!(k, ProtocolKey::F(n), "F{n} maps to F({n})"); + assert!( + should_forward_key(k, m), + "F{n} must FORWARD; translating without forwarding leaves \ + the key mapped and inert" + ); + } + } + + /// A2 — Shift+Tab is `BackTab`, and the `Shift` stays set. + /// + /// Both halves matter: the daemon binds `Tab` and `BackTab` + /// differently, and a `BackTab` that lost its modifier would be + /// indistinguishable from one the user did not shift. + #[test] + fn a2_shift_tab_is_backtab_with_shift_retained() { + use winit::keyboard::{Key as WKey, ModifiersState, NamedKey}; + + let (plain, plain_mods) = + translate_key(&WKey::Named(NamedKey::Tab), ModifiersState::empty()).expect("Tab maps"); + assert_eq!(plain, ProtocolKey::Tab); + assert!(plain_mods.is_empty()); + + let (shifted, shifted_mods) = + translate_key(&WKey::Named(NamedKey::Tab), ModifiersState::SHIFT) + .expect("Shift+Tab maps"); + assert_eq!(shifted, ProtocolKey::BackTab); + assert!( + shifted_mods.contains(Modifiers::SHIFT), + "Shift is retained on BackTab" + ); + assert!(should_forward_key(shifted, shifted_mods)); + } + + /// A3 — the menu key reaches the daemon. `ProtocolKey::Menu` and the + /// TUI's mapping both already existed; only the GPU translation was + /// missing, so the key did nothing in the GUI. + #[test] + fn a3_context_menu_maps_to_menu_and_forwards() { + use winit::keyboard::{Key as WKey, ModifiersState, NamedKey}; + + let (k, m) = translate_key(&WKey::Named(NamedKey::ContextMenu), ModifiersState::empty()) + .expect("ContextMenu maps"); + assert_eq!(k, ProtocolKey::Menu); + assert!(should_forward_key(k, m), "and forwards, or it is inert"); + } + #[test] fn translate_key_maps_motion_named_keys_and_chars() { use winit::keyboard::{Key as WKey, ModifiersState, NamedKey, SmolStr}; diff --git a/pmacs-protocol/src/lib.rs b/pmacs-protocol/src/lib.rs index 6976a8c..3a857b7 100644 --- a/pmacs-protocol/src/lib.rs +++ b/pmacs-protocol/src/lib.rs @@ -68,9 +68,9 @@ pub use message::{ 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, + StyleSegment, StyleSpan, TEXT_INPUT_MAX_BYTES, TEXT_INPUT_MIN_VERSION, 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 8516d4d..75340d4 100644 --- a/pmacs-protocol/src/message.rs +++ b/pmacs-protocol/src/message.rs @@ -513,8 +513,62 @@ pub enum FrontendEvent { /// Modifiers held during the gesture. mods: Modifiers, }, + /// Committed text from a keypress or an IME composition — GUI arc + /// Stage 1a, **protocol v24** ([`TEXT_INPUT_MIN_VERSION`]). + /// + /// **APPENDED, never widened.** postcard encodes an enum variant by + /// its positional index, so adding a field to any variant above + /// would silently re-interpret every older peer's bytes. A new + /// variant at the end is the only backward-compatible shape, which + /// is also why the frozen-byte pin in the tests sits on + /// [`FrontendEvent::PanelPointer`] — the *previous* final variant — + /// rather than on this one: an appended variant's own round-trip + /// cannot detect a discriminant shift beneath it. + /// + /// **This is not [`FrontendEvent::Paste`], and the difference is + /// behavioural rather than cosmetic.** A paste is bulk data from + /// elsewhere; this is what the user *typed*, so a terminal receives + /// it as **raw UTF-8 and never inside bracketed-paste markers** — a + /// shell that sees `ESC[200~` around typed input treats it as + /// pasted, which changes how it handles newlines and completion. + /// + /// **One `TextInput` is ONE edit**: one undo unit, one + /// `buffer.after-edit`, one eligible CRDT op. The multi-scalar case + /// is the whole reason the variant exists — a two-scalar grapheme + /// arriving as two keypresses is two undo units and, worse, can be + /// split by an intervening remote edit. + /// + /// `text` is capped at [`TEXT_INPUT_MAX_BYTES`]; an oversize payload + /// is **rejected, never truncated**, because truncating a UTF-8 + /// sequence at a byte boundary silently corrupts the last character + /// and a silently-shortened insert is worse than a refused one. + TextInput { + /// Which frontend produced the text. **Untrusted**, like every + /// other `frontend_id` on this enum — the daemon uses the + /// authenticated source, not this field. + frontend_id: FrontendId, + /// The committed text. Non-empty; see [`TEXT_INPUT_MAX_BYTES`]. + text: String, + }, } +/// First protocol version carrying [`FrontendEvent::TextInput`]. +/// +/// A frontend older than this **retains its existing `Key` behaviour and +/// its existing limitations** — it truncates multi-scalar input to the +/// first scalar today and ignores IME, and it continues to. The promise +/// is **no regression, not retroactive correctness**: nothing a `< 24` +/// peer already had degrades, and the daemon simply never receives a +/// variant such a peer cannot encode. +pub const TEXT_INPUT_MIN_VERSION: u32 = 24; + +/// Cap on [`FrontendEvent::TextInput::text`], in bytes of UTF-8. +/// +/// 64 KiB is far above any keystroke or IME commit and far below a +/// pathological paste, which has its own event. **Oversize is rejected +/// rather than truncated** — see the variant's own documentation. +pub const TEXT_INPUT_MAX_BYTES: usize = 64 * 1024; + /// Gesture step for [`FrontendEvent::Pointer`]. Double-click /// detection is frontend-side (`DoubleDown` instead of a second /// `Down`): only the frontend knows pixel proximity and its own @@ -562,7 +616,8 @@ impl FrontendEvent { | Self::TerminalPointer { frontend_id, .. } | Self::FrontendCellGeometry { frontend_id, .. } | Self::PanelResizeRows { frontend_id, .. } - | Self::PanelPointer { frontend_id, .. } => *frontend_id, + | Self::PanelPointer { frontend_id, .. } + | Self::TextInput { frontend_id, .. } => *frontend_id, } } } @@ -1840,7 +1895,32 @@ pub enum ResourceBody { /// 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; +/// +/// GUI arc Stage 1a: bumped 23 → 24 for +/// [`FrontendEvent::TextInput`] — committed text from a keypress or an +/// IME composition, carried as one event so that a multi-scalar +/// grapheme is one edit, one undo unit and one eligible CRDT op instead +/// of being truncated to its first scalar. Appended after +/// `PanelPointer`, the final v23 `FrontendEvent` variant, so no +/// existing discriminant moves. +/// +/// **The gate is producer-side AND receiver-side**, and for an inbound +/// variant the receiving half is the load-bearing one. An +/// instance→frontend variant is gated by the daemon simply not sending +/// it, which is entirely within the daemon's control; an inbound variant +/// cannot be, because the withholding would be the *peer's* job and a +/// client built from this same crate can encode the discriminant +/// whatever it negotiated. So the daemon refuses `TextInput` from a +/// session below [`TEXT_INPUT_MIN_VERSION`] rather than trusting the +/// producer to withhold — otherwise a v6–v23 session could drive an edit +/// through a variant its own session never declared. +/// +/// This is not a new shape: the v19 terminal and v21 panel families +/// already gate their own inbound events on the authenticated session's +/// negotiated version. What is unusual here is only that the extension +/// is **inbound-only** — there is no outbound counterpart to withhold, +/// so the receiver check is the whole of the daemon's half. +pub const PROTOCOL_VERSION: u32 = 24; /// Protocol version placed in the daemon's server-first [`Hello`]. /// @@ -2021,8 +2101,15 @@ pub fn negotiated_session_version(frontend_offer: u32) -> u32 { /// 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. +/// +/// GUI arc Stage 1a: extended to `[6, ..., 24]` for +/// [`FrontendEvent::TextInput`]. Additive, and gated producer-side AND +/// receiver-side — see [`PROTOCOL_VERSION`] for why an inbound variant +/// cannot rely on the producer withholding. [`ADVERTISED_PROTOCOL_VERSION`] does not move: a v23 +/// frontend negotiates v23, never sends the variant, and keeps today's +/// first-scalar behaviour. pub const SUPPORTED_PROTOCOL_VERSIONS: &[u32] = &[ - 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, + 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, ]; /// T M10.5: predicate for the handshake check. Returns `true` if diff --git a/src/daemon.rs b/src/daemon.rs index 121b469..c9d5973 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -71,6 +71,7 @@ use crate::protocol::{ InitialTarget, InitialTargetResult, InstanceCapabilities, InstanceIdentity, InstanceMessage, InstanceSignal, MAX_INITIAL_TARGET_ERROR_BYTES, MAX_INITIAL_TARGET_PATH_BYTES, PANEL_MIN_VERSION, PointerKind, SelectionSnapshot, SessionBootstrapRequest, + TEXT_INPUT_MAX_BYTES, TEXT_INPUT_MIN_VERSION, }; use crate::socket_path::{SocketPathError, ensure_runtime_subdir}; use crate::transport::{read_message, write_message}; @@ -2518,6 +2519,55 @@ fn handle_dispatcher_event( handle_inbound_paste(editor, source, claimed_fid, &data); } } + FrontendEvent::TextInput { text, .. } => { + // GUI arc 1a / A5 — committed text from a keypress + // or an IME composition. Handled here, beside + // `Paste`, for the same two reasons: the + // authenticated `source` is in scope (the event's + // own `frontend_id` is client-supplied and not + // trusted), and the semantic input dispatcher would + // otherwise drop it, which is exactly how GPU + // Ctrl-V was a no-op before Q#KR10a. + // + // A9 — the cap is enforced at the BOUNDARY, before + // anything is inserted. Rejected, never truncated: + // cutting UTF-8 at a byte offset corrupts the last + // character, and a silently-shortened insert is + // worse than a refused one. An empty payload is + // dropped too — it would be an edit that edits + // nothing, and would still cost an undo unit. + // **The producer gate is only half the contract.** + // A frontend that negotiated v6–v23 can still encode + // this variant — it is compiled from the same crate, + // and postcard will happily write the discriminant — + // so a peer that never declared v24 could otherwise + // mutate the buffer through a variant its own + // session does not include. Gate on the + // AUTHENTICATED session's negotiated version, not on + // the payload and not on what the daemon supports. + let peer_declared_text_input = session_registry + .session_state(source) + .is_some_and(|s| s.negotiated_protocol_version >= TEXT_INPUT_MIN_VERSION); + if !peer_declared_text_input { + eprintln!( + "pmacs: dropping TextInput from {source:?}, which negotiated \ + below v{TEXT_INPUT_MIN_VERSION}" + ); + return; + } + if text.is_empty() { + return; + } + if text.len() > TEXT_INPUT_MAX_BYTES { + eprintln!( + "pmacs: rejecting oversize TextInput from {source:?} \ + ({} bytes > {TEXT_INPUT_MAX_BYTES})", + text.len() + ); + return; + } + editor.dispatch_text_input(source, &text); + } _ => { let Some(&term_size) = term_sizes.get(&source) else { eprintln!( @@ -3588,12 +3638,15 @@ fn apply_event( render_state.resize(size); *term_size = size; } - // Q#KR10a — Paste is handled in the dispatcher's own - // `FrontendEvent::Paste` arm (unified for grid and semantic - // sessions, keyed by the authenticated source), and never - // reaches here. Listed explicitly so a future reshuffle can't - // silently re-route it through this payload-trusting path. + // Q#KR10a (Paste) and GUI arc 1a (TextInput) — both are handled + // in the dispatcher's own arms, unified for grid and semantic + // sessions and keyed by the AUTHENTICATED source, and neither + // reaches here. Listed explicitly rather than left to a + // wildcard so a future reshuffle cannot silently re-route + // either through this payload-trusting path: both carry a + // client-supplied `frontend_id` this function would believe. FrontendEvent::Paste { .. } + | FrontendEvent::TextInput { .. } | FrontendEvent::FocusGained(_) | FrontendEvent::FocusLost(_) // T M11.1: the semantic-frontend viewport declaration. Its diff --git a/src/editor.rs b/src/editor.rs index 458751b..b08c26a 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -1881,6 +1881,161 @@ impl EditorState { true } + /// GUI arc Stage 1a / A5 — apply committed text from a keypress or + /// an IME composition. + /// + /// **This is typed text, not a paste**, and the two differ in three + /// observable ways: + /// + /// * a terminal gets **raw UTF-8, never bracketed paste** (A8) — a + /// shell that sees `ESC[200~` treats the input as pasted and + /// changes how it handles newlines and completion; + /// * the clipboard slot is **not** touched, because nothing was + /// copied; + /// * the document path performs **one atomic edit** (A6) — one undo + /// unit, one `buffer.after-edit`, one eligible CRDT op — which is + /// the entire reason the wire variant exists. Delivering a + /// two-scalar grapheme as two keypresses makes two undo units and + /// lets a remote edit interleave between them. + /// + /// The modal precedence is `dispatch_key`'s, deliberately: menu, + /// search, query-replace and minibuffer are full keymap shadows, so + /// text that arrives while one is up belongs to it and not to the + /// buffer underneath. Those surfaces have no notion of a + /// multi-scalar commit, so the text is fed to them **one scalar at + /// a time, in order** (A7) — order is the contract, since a prompt + /// accumulates a query. + /// + /// Returns nothing, unlike [`Self::dispatch_paste`], and the + /// difference is real rather than stylistic: §5's precedence is + /// **total** — shadow, terminal, or document, every state routes + /// somewhere — so there is no "unhandled" case for a caller to fall + /// back on. + pub fn dispatch_text_input(&mut self, frontend_id: FrontendId, text: &str) { + self.core.borrow_mut().active_frontend = frontend_id; + + // Shadows first, one scalar at a time and in order (A7). + if self.feed_shadow_scalars(frontend_id, text) { + return; + } + + // A8 — a terminal takes the bytes exactly as typed. + if let Some(key) = self.active_terminal_key(frontend_id) { + self.claim_terminal_controller(key); + self.send_terminal_bytes(key.buffer_id, text.as_bytes()); + return; + } + + // §5's provenance split. A SINGLE-scalar commit is + // indistinguishable from a keypress, so it must be + // indistinguishable downstream too: it rotates to + // `buffer.self-insert` and produces the same one-codepoint + // typed-edit record a keystroke does. Without that, auto-pairing + // (Q#AP9) and every other typed-edit consumer silently stop + // recognizing GUI input, and `this_command` goes stale — a + // regression that shows up as "auto-pair stopped working in the + // GUI", far from its cause. + // + // A MULTI-scalar commit is not a keystroke: it breaks the + // command chain and creates no typed provenance, exactly as a + // paste does. + let single = { + let mut chars = text.chars(); + match (chars.next(), chars.next()) { + (Some(ch), None) => Some(ch), + _ => None, + } + }; + + let pre_revision = self.active_buffer_revision(); + { + let mut core = self.core.borrow_mut(); + if let Some(ch) = single { + core.rotate_command(frontend_id, "buffer.self-insert"); + core.typed_edit_arm(frontend_id, ch); + // **Must go through `insert_char_over_region`, not + // the generic byte insert.** Arming provenance is + // only half of it: `typed_edit_complete` is called + // from `insert_char` / `insert_char_over_region` and + // nowhere else, so a generic insert leaves the arm + // holding `None` and `buffer.after-edit` sees no + // record — `this_command` rotates correctly and + // auto-pairing still fails, which is a failure mode + // that looks like success from the command side. + // It handles the no-region case itself, by + // delegating to `insert_char`. + core.insert_char_over_region(ch); + } else { + core.break_command_chain(frontend_id); + // A6 — multi-scalar is ONE generic edit, and + // deliberately creates no typed provenance: it is not a + // keystroke. + if let Err(e) = core.insert_text_input(text) { + eprintln!("pmacs: text input failed: {e}"); + } + } + } + + // The dispatch tail `dispatch_key` runs, for the same reason: a + // typed-edit record is only meaningful to a hook that can see + // it, so it is armed across the fan-out and cleared after. + let typed_edit = self.core.borrow_mut().typed_edit_finish(frontend_id); + if pre_revision != self.active_buffer_revision() { + if let Some(record) = typed_edit { + self.core + .borrow_mut() + .typed_edit_set_armed(frontend_id, record); + } + self.lua_host + .run_hook("buffer.after-edit", mlua::MultiValue::new()); + self.core.borrow_mut().typed_edit_clear_armed(); + } + } + + /// Feed `text` to whichever modal shadow owns input, one scalar at a + /// time and in order. Returns `true` when a shadow consumed it. + /// + /// Each scalar becomes a plain `Char` chord, which is what these + /// handlers already take from `dispatch_key`; routing through them + /// rather than reaching into their state is what keeps a prompt's + /// own editing rules — history, completion, acceptance — in one + /// place. + fn feed_shadow_scalars(&mut self, frontend_id: FrontendId, text: &str) -> bool { + enum Shadow { + Menu, + Search, + QueryReplace, + Minibuffer, + } + let shadow = { + let core = self.core.borrow(); + if core.menu_is_open() { + Some(Shadow::Menu) + } else if core.search_active() { + Some(Shadow::Search) + } else if core.query_replace_active() { + Some(Shadow::QueryReplace) + } else if core.minibuffer.is_active() { + Some(Shadow::Minibuffer) + } else { + None + } + }; + let Some(shadow) = shadow else { + return false; + }; + for ch in text.chars() { + let chord = Chord::plain(KeyCode::Char(ch)); + match shadow { + Shadow::Menu => self.dispatch_menu_key(frontend_id, chord), + Shadow::Search => self.dispatch_search_key(chord), + Shadow::QueryReplace => self.dispatch_query_replace_key(chord), + Shadow::Minibuffer => self.dispatch_minibuffer_key(frontend_id, chord), + } + } + true + } + /// Apply authenticated frontend focus to terminal control/reporting. pub fn dispatch_focus(&mut self, frontend_id: FrontendId, gained: bool) { self.core.borrow_mut().active_frontend = frontend_id; diff --git a/src/editor_core.rs b/src/editor_core.rs index 517989c..01e2fe6 100644 --- a/src/editor_core.rs +++ b/src/editor_core.rs @@ -478,8 +478,9 @@ pub struct CommandBoundary { /// the exact facts for the one consumer contract that needs them (the /// pairing hook): the decoded codepoint, the requested and effective /// ranges, and the post-edit cursor, plus a `clean` verdict (effective -/// triple equals the request). It is ephemeral — armed by the two -/// self-insert producers (dispatch fallback, optimistic CRDT arm) for +/// triple equals the request). It is ephemeral — armed by the three +/// self-insert producers (dispatch fallback, optimistic CRDT arm, and +/// the single-scalar `TextInput` path of GUI arc Stage 1a) for /// exactly one `buffer.after-edit` fan-out, consumable once via /// `pmacs.editor.take_typed_edit()`, and cleared when the fan-out /// returns. Paste, programmatic mutation, manual hook runs, and a @@ -687,8 +688,10 @@ pub struct EditorCore { /// query-replace twin of `search`; drives the fifth dispatcher /// shadow. query_replace: Option, - /// In-flight typed-edit arm (auto-pairing Q#AP9): set by the - /// dispatch fallback just before it invokes `buffer.self-insert`, + /// In-flight typed-edit arm (auto-pairing Q#AP9): set by a + /// self-insert producer just before the edit — the dispatch + /// fallback invoking `buffer.self-insert`, or 1a's single-scalar + /// `TextInput` path — /// completed by the insert primitives, taken back by the /// dispatcher via [`Self::typed_edit_finish`] in the same /// dispatch. Never survives a dispatch cycle. @@ -4808,9 +4811,19 @@ impl EditorCore { /// Declare that `fid`'s dispatch is about to invoke /// `buffer.self-insert` for `codepoint`: the next insert primitive /// whose character matches completes the [`TypedEditRecord`]. - /// Called by the dispatch fallback only — programmatic + /// Called by the self-insert producers: the dispatch fallback, and + /// the **single-scalar** `TextInput` path (GUI arc 1a), which must + /// be indistinguishable from a keypress downstream. Programmatic /// `pmacs.command.invoke("buffer.self-insert")` deliberately never - /// arms, so a hook run after it observes no record. + /// arms, so a hook run after it observes no record; nor does a + /// MULTI-scalar `TextInput`, which is not a keystroke. + /// + /// **Arming is only half.** Completion happens in the insert + /// primitives ([`Self::insert_char`] / + /// [`Self::insert_char_over_region`]) and nowhere else, so a caller + /// that arms and then performs a generic byte insert leaves the arm + /// holding `None` — `this_command` looks right and auto-pairing + /// silently stops working. pub fn typed_edit_arm(&mut self, fid: FrontendId, codepoint: char) { self.typed_edit_pending = Some(TypedEditPending { fid, @@ -5009,6 +5022,25 @@ impl EditorCore { self.insert_bytes_over_region(data) } + /// GUI arc Stage 1a / A6 — insert **multi-scalar** committed text as + /// one edit. + /// + /// **Single-scalar text does NOT come here**; it goes through + /// [`Self::insert_char_over_region`], which is the only path (with + /// [`Self::insert_char`]) that completes a [`TypedEditRecord`]. + /// Routing a single scalar here would arm provenance and never + /// complete it — see `EditorState::dispatch_text_input`. + /// + /// Shares [`Self::insert_bytes_over_region`] with paste, which is + /// what makes it a single `EditOp` and therefore a single undo + /// unit, a single `buffer.after-edit`, and a single eligible CRDT + /// op. **It deliberately does NOT touch `clipboard_slot`**: typed + /// text was never copied, and recording it would let the next yank + /// resurrect something the user merely typed. + pub fn insert_text_input(&mut self, text: &str) -> Result<(), String> { + self.insert_bytes_over_region(text.as_bytes()) + } + /// Shared insert/replace for paste: `Replace` over the active /// region, else `Insert` at the cursor. The cursor lands just past /// the inserted bytes and any selection is cleared. No-op insert for diff --git a/src/protocol.rs b/src/protocol.rs index 33beee0..474473b 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_three_for_minibuffer_prompt_rows() { + fn protocol_version_is_twenty_four_for_text_input() { // 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 @@ -1740,7 +1740,11 @@ mod tests { // 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); + // + // v24 is `FrontendEvent::TextInput` (GUI arc Stage 1a) — an + // APPENDED variant, which is why the freeze above survives it + // untouched: nothing in `MinibufferPrompt`'s encoding moved. + assert_eq!(PROTOCOL_VERSION, 24); } #[test] @@ -1817,18 +1821,19 @@ mod tests { // (`CompletionPopup`), v16 (`ThemeFacts`), v17 (`FontFacts`), // v18 (`StatuslineSegments`), v19 (the vterm terminal family), // v20 (semantic initial-target bootstrap), v21 (the bottom - // panel band), v22 (`LineWrapFacts`), and v23 - // (`MinibufferPromptRows`) all interoperate. - for accepted in 6..=23 { + // panel band), v22 (`LineWrapFacts`), v23 + // (`MinibufferPromptRows`), and v24 (`TextInput`, GUI arc Stage + // 1a) all interoperate. + for accepted in 6..=24 { assert!( is_supported_protocol_version(accepted), "v{accepted} must be accepted" ); } - for rejected in [0, 1, 2, 3, 4, 5, 24, u32::MAX] { + for rejected in [0, 1, 2, 3, 4, 5, 25, u32::MAX] { assert!( !is_supported_protocol_version(rejected), - "v{rejected} must be rejected by a v23 binary" + "v{rejected} must be rejected by a v24 binary" ); } } @@ -1966,6 +1971,43 @@ mod tests { ); } + #[test] + fn panel_pointer_encoding_is_unchanged_by_the_v24_build() { + // GUI arc Stage 1a — the placement pin for `TextInput`, and it + // sits on `PanelPointer` rather than on `TextInput` itself for + // a structural reason: `TextInput` is APPENDED, so its own + // round-trip is identical whether or not a variant was inserted + // beneath it. Only the PREVIOUS final variant's bytes move, so + // only they can witness the shift. + // + // `PanelPointer` was the last `FrontendEvent` variant at v23. + // Inserting anything before it shifts its discriminant and + // breaks **v21–v23 panel traffic** — `PanelPointer` arrived at + // v21 and is gated, so that is its whole population, not every + // peer and not every session. Stated exactly rather than + // dramatically: an earlier version of this comment claimed + // every v6–v23 daemon on every session, which is false and + // would send the next reader hunting a larger blast radius than + // exists. + let ev = FrontendEvent::PanelPointer { + frontend_id: FrontendId(2), + geometry_epoch: 1, + panel_epoch: 1, + buffer_id: pmacs_protocol::BufferId::from_raw(4), + coord: CellCoord { row: 0, col: 0 }, + kind: pmacs_protocol::MouseKind::Move, + mods: Modifiers::NONE, + }; + let bytes = postcard::to_allocvec(&ev).expect("encode"); + assert_eq!( + bytes, + [15, 2, 1, 1, 4, 0, 0, 3, 0], + "PanelPointer's v23 wire bytes changed — a variant was \ + inserted before it; append new FrontendEvent variants at \ + the end" + ); + } + #[test] fn terminal_family_round_trips_and_pins_its_discriminants() { let bid = pmacs_protocol::BufferId::from_raw(9); @@ -2738,9 +2780,20 @@ mod tests { #[test] fn m4_6_handshake_accepts_v6_peer() { + // ANCHORED ON THE LITERAL 6, deliberately. The body used to + // assert `is_supported_protocol_version(PROTOCOL_VERSION)` — + // "the current wire accepts itself" — which is a different and + // much weaker claim than the name and the M4.6 contract make: + // **v6 is the FLOOR**, the oldest peer the handshake still + // admits, and it must keep being accepted no matter how far the + // ceiling moves. Written against the moving constant, the test + // would have gone on passing after v6 was dropped from the + // supported set, which is the only regression it exists to + // catch. Found when the v24 bump made it fail for the unrelated + // reason that `SUPPORTED_PROTOCOL_VERSIONS` had not been widened. assert!( - is_supported_protocol_version(PROTOCOL_VERSION), - "the current wire version must accept itself" + is_supported_protocol_version(6), + "v6 is the floor and must stay accepted" ); } diff --git a/tests/bottom_panel_stage2b_gpu_acceptance.rs b/tests/bottom_panel_stage2b_gpu_acceptance.rs index 32c197d..7da08ea 100644 --- a/tests/bottom_panel_stage2b_gpu_acceptance.rs +++ b/tests/bottom_panel_stage2b_gpu_acceptance.rs @@ -337,9 +337,10 @@ 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. v23 is `MinibufferPromptRows` (Discovery - // Stage 2); v22 was `LineWrapFacts` (long-lines Stage 3). - assert_eq!(PROTOCOL_VERSION, 23); + // here, not a silent one. v24 is `TextInput` (GUI arc Stage 1a); + // v23 was `MinibufferPromptRows` (Discovery Stage 2); v22 was + // `LineWrapFacts` (long-lines Stage 3). + assert_eq!(PROTOCOL_VERSION, 24); assert_eq!( ADVERTISED_PROTOCOL_VERSION, 20, "moving this is the incompatible act the mechanism exists to avoid" diff --git a/tests/discovery_stage2_acceptance.rs b/tests/discovery_stage2_acceptance.rs index 571c605..db5f8cc 100644 --- a/tests/discovery_stage2_acceptance.rs +++ b/tests/discovery_stage2_acceptance.rs @@ -67,25 +67,31 @@ use common::daemon::{TestDaemon, build_default_caps}; /// 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() { +fn the_wire_is_v24_and_the_advertised_baseline_is_unmoved() { assert_eq!( - PROTOCOL_VERSION, 23, - "v23 is MinibufferPromptRows (Discovery Stage 2)" + PROTOCOL_VERSION, 24, + "v24 is TextInput (GUI arc Stage 1a); v23 was MinibufferPromptRows" ); 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 { + // still supported, AND the current wire is in the set. The loop + // must run to `PROTOCOL_VERSION`, not to a literal: stopping at 23 + // let a supported range that ended at 23 pass this test while + // `PROTOCOL_VERSION` was already 24 — the accepted half proved + // nothing about the version the constant names. + for version in 6..=PROTOCOL_VERSION { assert!( is_supported_protocol_version(version), "v{version} must still be supported" ); } - assert!(!is_supported_protocol_version(24)); + // The ceiling: the supported set ENDS at the current wire, which + // is what makes an accidentally-widened set a failure rather than + // a silent pass. Probes one PAST the top, so it moves with it. + assert!(!is_supported_protocol_version(PROTOCOL_VERSION + 1)); } // --------------------------------------------------------------------------- @@ -612,7 +618,7 @@ fn one_daemon_serves_a_v23_rows_session_and_a_frozen_v22_session() { // 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); + assert_eq!(PROTOCOL_VERSION, 24); // 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 diff --git a/tests/gui_stage1a_acceptance.rs b/tests/gui_stage1a_acceptance.rs new file mode 100644 index 0000000..dad89e5 --- /dev/null +++ b/tests/gui_stage1a_acceptance.rs @@ -0,0 +1,423 @@ +//! GUI arc Stage 1a acceptance — `TextInput` at protocol v24. +//! +//! Framing: `docs/gui-stage1-input-framing.md` §5 (Q#S1-9 precedence) +//! and §6's A1–A9. +//! +//! **These rows drive the real dispatch, not the classifier.** 1a's +//! first review found A7 and A8 unreachable from the production +//! producer while `text_input_payload` was perfectly correct: the +//! intercept branch returned before classification, and a modal prompt +//! or a focused terminal is exactly what makes intercept true. A test +//! that exercises the pure function would have stayed green through +//! that, so the rows here go through `dispatch_text_input` and, for the +//! producer-side ones, through the real classifier at the real call +//! site. + +use pmacs::editor::EditorState; +use pmacs::protocol::FrontendId; + +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() +} + +fn editor_with(body: &str) -> EditorState { + let s = EditorState::new_with_roots(&crate::iso::roots()); + if !body.is_empty() { + exec(&s, &format!("pmacs.window.buffer():insert(0, {body:?})")); + } + exec(&s, "pmacs.editor.goto_byte(0)"); + s +} + +fn buffer_text(s: &EditorState) -> String { + let b: mlua::String = eval( + s, + "local b = pmacs.window.buffer(); return b:slice(0, b:len())", + ); + String::from_utf8_lossy(&b.as_bytes()).into_owned() +} + +const FID: FrontendId = FrontendId::LOCAL; + +// --------------------------------------------------------------------- +// A6 — one commit is one edit, one undo unit, one hook fan-out +// --------------------------------------------------------------------- + +/// A6 — a multi-scalar commit is **one** edit and **one** undo unit. +/// +/// This is the failure 1a exists to fix: as separate keypresses the same +/// grapheme is two edits, so one undo leaves half of it behind. +#[test] +fn a6_a_multi_scalar_commit_is_one_edit_and_one_undo_unit() { + let mut s = editor_with(""); + exec( + &s, + "_G.edits = 0 + pmacs.hook.add('buffer.after-edit', function() _G.edits = _G.edits + 1 end)", + ); + + // A composed grapheme: base plus combining acute. Two scalars, one + // thing the user meant to type. + s.dispatch_text_input(FID, "e\u{301}"); + assert_eq!(buffer_text(&s), "e\u{301}"); + + let edits: i64 = eval(&s, "return _G.edits"); + assert_eq!(edits, 1, "one commit must fire ONE buffer.after-edit"); + + exec(&s, "pmacs.command.invoke('buffer.undo')"); + assert_eq!( + buffer_text(&s), + "", + "one undo must remove the whole commit, not its last scalar" + ); +} + +// --------------------------------------------------------------------- +// §5 provenance — the single/multi split +// --------------------------------------------------------------------- + +/// §5 — a SINGLE-scalar commit is indistinguishable from a keypress, so +/// it must produce a real, consumable `TypedEditRecord`. +/// +/// **Asserting `this_command` is not enough**, and that is the whole +/// point of this row: review round 2 found the code rotating the command +/// correctly while never completing the record, because arming and +/// completing are different steps and only the insert primitives +/// complete. `this_command` looked right and auto-pairing was broken. +/// So this consumes the record through the same seam `pair.lua` uses. +#[test] +fn single_scalar_text_input_produces_a_consumable_typed_edit_record() { + let mut s = editor_with(""); + exec(&s, "pmacs.pair._capture_records = true"); + + s.dispatch_text_input(FID, "("); + + let (cp, ch, clean, il): (i64, String, bool, i64) = eval( + &s, + "local r = pmacs.pair._last_record + return r.codepoint, r.char, r.clean, r.inserted_len", + ); + assert_eq!(cp, 40, "exact codepoint for '('"); + assert_eq!(ch, "("); + assert!(clean, "no intercept ran, so the effective triple is clean"); + assert_eq!(il, 1); + + let this_command: String = eval(&s, "return pmacs.editor.this_command() or ''"); + assert_eq!( + this_command, "buffer.self-insert", + "and the command rotates, which is the half that already worked" + ); +} + +/// §5 — a MULTI-scalar commit is **not** a keystroke: it creates no +/// typed provenance and breaks the command chain. +#[test] +fn multi_scalar_text_input_creates_no_typed_provenance() { + let mut s = editor_with(""); + exec(&s, "pmacs.pair._capture_records = true"); + + s.dispatch_text_input(FID, "e\u{301}"); + + let no_record: bool = eval(&s, "return pmacs.pair._last_record == nil"); + assert!( + no_record, + "a multi-scalar commit must not forge a typed-edit record" + ); +} + +/// §5 — a MULTI-scalar commit **breaks the command chain**, as a paste +/// does. +/// +/// **The chain is PRIMED first, and that is what makes the row +/// discriminating.** Starting from a fresh editor the chain is already +/// empty, so an assertion that it is empty afterwards passes whether or +/// not `break_command_chain` is called — the first version of this row +/// did exactly that and would have survived deleting the call. +#[test] +fn multi_scalar_text_input_breaks_a_live_command_chain() { + let mut s = editor_with(""); + + // Prime it with 1a's OWN single-scalar path, which rotates to + // `buffer.self-insert`. A programmatic + // `pmacs.command.invoke('buffer.self-insert')` cannot prime it: + // rotation belongs to the dispatcher, and invoking the command + // directly deliberately never rotates or arms. + s.dispatch_text_input(FID, "x"); + let primed: String = eval(&s, "return pmacs.editor.this_command() or ''"); + assert_eq!( + primed, "buffer.self-insert", + "precondition: the chain is live before the commit" + ); + + s.dispatch_text_input(FID, "e\u{301}"); + + let after: Option = eval(&s, "return pmacs.editor.this_command()"); + assert_eq!( + after, None, + "a multi-scalar commit is not a command and must clear the chain" + ); +} + +/// The pairing consumer, end to end: a single-scalar `(` must auto-pair +/// exactly as a typed `(` does. This is the behaviour the missing record +/// silently disabled, stated in the terms a user would notice. +#[test] +fn single_scalar_text_input_auto_pairs_like_a_keypress() { + let mut s = editor_with(""); + s.dispatch_text_input(FID, "("); + assert_eq!( + buffer_text(&s), + "()", + "auto-pairing consumes the typed-edit record; without one the \ + closer is never inserted" + ); +} + +// --------------------------------------------------------------------- +// A7 — prompts consume scalars IN ORDER +// --------------------------------------------------------------------- + +/// A7 — a prompt accumulates the scalars in order. +/// +/// Order is the contract: a reversed or set-wise delivery would still +/// "consume" the text and would produce a different query. +#[test] +fn a7_a_prompt_consumes_scalars_in_order() { + let mut s = editor_with(""); + exec( + &s, + "pmacs.minibuffer.read({ prompt = 'x: ', on_accept = function() end })", + ); + assert!(s.core.borrow().minibuffer.is_active(), "prompt is up"); + + s.dispatch_text_input(FID, "abc"); + + let content: String = eval(&s, "return pmacs.minibuffer.contents() or ''"); + assert_eq!(content, "abc", "in order, not reversed or reordered"); + assert_eq!( + buffer_text(&s), + "", + "and the buffer underneath is untouched" + ); +} + +// --------------------------------------------------------------------- +// A9 — the cap rejects rather than truncates +// --------------------------------------------------------------------- + +/// A9 — a payload at the cap is accepted whole. The complement of the +/// rejection row: a cap that refused its own boundary value would be +/// off by one in the direction nobody notices until a long IME commit +/// vanishes. +/// +/// **The rejection half is witnessed where it is enforced** — at the +/// daemon boundary (`daemon.rs`, gated before any insert) and at the +/// producer (`AttachClient::send_text_input`, whose unit test lives +/// beside it in `pmacs-gpu`). Neither is reachable from an +/// `EditorState`, so asserting the constant here instead would be a row +/// that cannot fail for the right reason. +#[test] +fn a9_a_payload_at_the_cap_is_inserted_whole() { + let mut s = editor_with(""); + let at_cap = "a".repeat(pmacs_protocol::TEXT_INPUT_MAX_BYTES); + s.dispatch_text_input(FID, &at_cap); + assert_eq!( + buffer_text(&s).len(), + pmacs_protocol::TEXT_INPUT_MAX_BYTES, + "the boundary value is legal and must land intact" + ); +} + +// --------------------------------------------------------------------- +// A8 — a terminal receives RAW UTF-8, never bracketed paste +// --------------------------------------------------------------------- + +/// A8, delivered rather than merely routed: the child process receives +/// the exact UTF-8 bytes, **while bracketed-paste mode is ENABLED**, and +/// no `ESC[200~` / `ESC[201~` markers. +/// +/// **The enabled mode is the whole precondition.** With bracketed paste +/// off, "no markers" is true of every code path including a paste, so +/// the assertion would pass against the behaviour it exists to forbid. +/// The row therefore waits for the child's own `ESC[?2004h` to be +/// parsed, asserts the mode really is on, and only then types. +/// +/// The contrast at the end is what makes it a discriminator: through the +/// same terminal in the same mode, a PASTE does get the markers. One +/// path bracketed and the other not, observed at the PTY. +#[test] +fn a8_a_terminal_receives_raw_utf8_with_bracketed_paste_enabled() { + use pmacs::terminal::TerminalSpec; + use std::time::{Duration, Instant}; + + let dir = tempfile::tempdir().expect("tempdir"); + let sink = dir.path().join("received"); + let sink_disp = sink.display().to_string(); + + let mut s = EditorState::new_with_roots(&crate::iso::roots()); + + // The child turns bracketed paste ON, then copies its stdin to a + // file so the test can read exactly what arrived on the PTY. + let script = format!("printf '\\033[?2004h'; exec cat > {sink_disp}"); + let mut spec = TerminalSpec::new("/bin/sh"); + spec.args = vec!["-c".into(), script]; + spec.rows = 24; + spec.cols = 80; + let buffer_id = s + .terminal_manager + .borrow_mut() + .open( + spec, + &mut s.core.borrow_mut(), + &mut s.process_supervisor.borrow_mut(), + ) + .expect("open terminal"); + + // Point this frontend's view at the terminal buffer, the way a + // daemon-side buffer switch does. Without it `active_terminal_key` + // returns `None`, the terminal branch is never taken, and the row + // fails for a setup reason rather than a behavioural one — which is + // exactly how it first failed. + let window_id = attach_terminal_view(&s, FID, buffer_id); + let key = pmacs::terminal::TerminalViewKey::new(FID, window_id, buffer_id); + // Precondition, asserted rather than assumed: this frontend's + // ACTIVE window shows the terminal buffer. A row that silently + // failed this would be testing the document path and reporting it + // as a terminal result. + { + let core = s.core.borrow(); + let view = core.views.get(&FID).expect("view registered"); + let active = core.windows.get(&view.active).expect("active window"); + assert_eq!(active.buffer_id, buffer_id); + assert!( + s.terminal_manager.borrow().is_terminal(buffer_id), + "and the manager agrees it is a terminal" + ); + } + + // Wait for the child's mode-set to be parsed — a condition, not a + // sleep, so a slow machine waits longer rather than failing. + let deadline = Instant::now() + Duration::from_secs(10); + loop { + s.tick_processes(); + let on = s + .terminal_manager + .borrow() + .modes_for_view(key) + .is_some_and(|m| m.bracketed_paste); + if on { + break; + } + assert!( + Instant::now() < deadline, + "child never enabled bracketed paste; the precondition this \ + row depends on was never established" + ); + std::thread::sleep(Duration::from_millis(10)); + } + + // Multi-byte and multi-scalar, so a byte-level mistake shows up. + let typed = "h\u{e9}llo\u{301}"; + s.dispatch_text_input(FID, typed); + + // Waits for AT LEAST the full payload, then asserts exact equality. + // Sound against a split write for a reason the contrast below does + // not share: the gate is a lower bound on length, so a partial + // delivery keeps waiting rather than being mistaken for a wrong + // answer — and the equality can still fail for the real reason, + // which a wait-for-exact-content loop could not. + let deadline = Instant::now() + Duration::from_secs(10); + let got = loop { + s.tick_processes(); + let got = std::fs::read(&sink).unwrap_or_default(); + if got.len() >= typed.len() { + break got; + } + assert!( + Instant::now() < deadline, + "the child never received the typed text; got {got:?}" + ); + std::thread::sleep(Duration::from_millis(10)); + }; + + assert_eq!( + String::from_utf8_lossy(&got), + typed, + "the PTY must receive the exact UTF-8 that was typed" + ); + let text = String::from_utf8_lossy(&got).into_owned(); + assert!( + !text.contains("\u{1b}[200~") && !text.contains("\u{1b}[201~"), + "typed text must NOT be bracketed: {text:?}" + ); + + // The contrast, through the same terminal in the same mode: a paste + // IS bracketed. Without this the row above could pass because the + // mode was somehow inert rather than because the code is right. + assert!( + s.dispatch_paste(FID, b"pasted"), + "the terminal claims the paste" + ); + // + // **Wait for the COMPLETE sequence, not the opening marker.** PTY + // delivery and the child's writes can split anywhere, so breaking + // as soon as `ESC[200~` appears and then requiring the payload and + // the closer is a race that fails on correct code — the closer may + // simply not have arrived yet. Polling for the whole string makes a + // partial write indistinguishable from "not yet", which is what it + // is. Same rule the vterm suite follows when it waits for `row19` + // rather than for a prefix of it. + let want = "\u{1b}[200~pasted\u{1b}[201~"; + let deadline = Instant::now() + Duration::from_secs(10); + loop { + s.tick_processes(); + let all = String::from_utf8_lossy(&std::fs::read(&sink).unwrap_or_default()).into_owned(); + if all.contains(want) { + break; + } + assert!( + Instant::now() < deadline, + "a paste through the same terminal must be bracketed on both \ + sides; waited for {want:?}, saw {all:?}" + ); + std::thread::sleep(Duration::from_millis(10)); + } +} + +/// Register a frontend view whose active window shows `buffer_id`. +fn attach_terminal_view( + state: &EditorState, + frontend_id: FrontendId, + buffer_id: pmacs::buffer::BufferId, +) -> pmacs::window::WindowId { + use pmacs::window::{FrontendView, Layout, Window, WindowId}; + let mut core = state.core.borrow_mut(); + let text_view = { + let registry = core.registry.clone(); + let registry = registry.borrow(); + let buffer = registry.get(buffer_id).expect("buffer present"); + pmacs::text_view::TextView::new(buffer) + }; + let window_id = WindowId::next(); + core.windows + .insert(window_id, Window::new(window_id, buffer_id, text_view)); + core.register_frontend_view( + frontend_id, + FrontendView { + layout: Layout::single(window_id), + active: window_id, + fold_projection: true, + panel_capable: true, + frame_geometry: None, + panel_hidden: false, + }, + ); + window_id +} + +#[path = "common/iso.rs"] +mod iso; diff --git a/tests/gui_stage1a_wire_acceptance.rs b/tests/gui_stage1a_wire_acceptance.rs new file mode 100644 index 0000000..a365ac0 --- /dev/null +++ b/tests/gui_stage1a_wire_acceptance.rs @@ -0,0 +1,186 @@ +//! GUI arc Stage 1a — the `TextInput` wire gate, against a real daemon. +//! +//! Separate from `gui_stage1a_acceptance` because these rows need a +//! live daemon and a negotiated session; that suite is in-process. +//! +//! **The claim under test is a REFUSAL**, which is the hardest kind to +//! witness honestly: "nothing happened" is also what a broken test, +//! a dead daemon or a dropped connection look like. Every row here +//! therefore pairs the refusal with a positive control on the same +//! session — something that *does* take effect — so silence can only +//! mean the gate fired. + +#![cfg(feature = "crdt")] + +use std::os::unix::net::UnixStream; +use std::time::{Duration, Instant}; + +use pmacs_protocol::{ + ADVERTISED_PROTOCOL_VERSION, AttachRequest, CellSize, FrontendCapabilities, FrontendEvent, + Hello, InstanceMessage, Key, KeyEvent, Modifiers, PROTOCOL_VERSION, SessionBootstrapRequest, + TEXT_INPUT_MIN_VERSION, read_message, write_message, +}; + +#[path = "common/mod.rs"] +mod common; + +fn semantic_caps() -> FrontendCapabilities { + FrontendCapabilities { + synchronized_output: false, + unicode_smp: true, + true_color: true, + mouse: false, + bracketed_paste: false, + terminal_kind: Some("stage1a".into()), + multi_frontend: true, + crdt_replica: true, + semantic_render: true, + } +} + +/// Attach a semantic session that counter-offers `offer`. +fn attach_semantic( + daemon: &common::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 stays 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"); + write_message(&mut stream, &SessionBootstrapRequest::default()).expect("write bootstrap"); + (stream, fid) +} + +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}"); +} + +fn send_text_input(stream: &mut UnixStream, fid: pmacs_protocol::FrontendId, text: &str) { + write_message( + stream, + &FrontendEvent::TextInput { + frontend_id: fid, + text: text.to_owned(), + }, + ) + .expect("write TextInput"); +} + +fn send_key(stream: &mut UnixStream, fid: pmacs_protocol::FrontendId, key: Key) { + write_message( + stream, + &FrontendEvent::Key(KeyEvent { + frontend_id: fid, + key, + mods: Modifiers::NONE, + timestamp_ns: 0, + }), + ) + .expect("write key"); +} + +/// **The discriminating witness for the inbound gate.** A session that +/// negotiated v23 can still ENCODE `TextInput` — it is built from this +/// same crate — so the daemon must refuse it on the authenticated +/// session's negotiated version rather than trusting the producer to +/// withhold. +/// +/// The positive control is what makes the refusal legible: the SAME +/// session then sends an ordinary `Key`, and that must take effect. So +/// the session is alive, the stream is synchronized and the daemon is +/// listening — silence about the `TextInput` is the gate, not the +/// plumbing. +#[test] +fn a_v23_session_cannot_drive_an_edit_through_text_input() { + assert_eq!( + TEXT_INPUT_MIN_VERSION, 24, + "this row is written against the v24 floor" + ); + let daemon = common::daemon::TestDaemon::spawn(); + let (mut stream, fid) = attach_semantic(&daemon, TEXT_INPUT_MIN_VERSION - 1); + + let buffer_id = pump(&mut stream, "first BufferSnapshot", |msg| match msg { + InstanceMessage::BufferSnapshot { buffer_id, .. } => Some(*buffer_id), + _ => None, + }); + + // Refused: encoded by a peer that never declared v24. + send_text_input(&mut stream, fid, "REFUSED"); + + // The positive control, on the same session and after it. + send_key(&mut stream, fid, Key::Char('k')); + + // The first edit that reaches this session must be the CONTROL's, + // never the refused text. Ordering carries the proof: the daemon + // processes a session's events in order, so the control's edit + // arriving with no preceding `REFUSED` edit means the TextInput was + // dropped rather than merely slow. + let op = pump(&mut stream, "the control's edit", |msg| match msg { + InstanceMessage::CrdtOp { + buffer_id: b, op, .. + } if *b == buffer_id => Some(op.bytes.clone()), + _ => None, + }); + let text = String::from_utf8_lossy(&op).into_owned(); + assert!( + !text.contains("REFUSED"), + "a v23 session must not be able to insert through TextInput; got {text:?}" + ); +} + +/// The complement, so the row above cannot pass because `TextInput` is +/// broken outright: the SAME traffic on a v24 session **does** edit. +#[test] +fn a_v24_session_can_drive_an_edit_through_text_input() { + let daemon = common::daemon::TestDaemon::spawn(); + let (mut stream, fid) = attach_semantic(&daemon, PROTOCOL_VERSION); + + let buffer_id = pump(&mut stream, "first BufferSnapshot", |msg| match msg { + InstanceMessage::BufferSnapshot { buffer_id, .. } => Some(*buffer_id), + _ => None, + }); + + send_text_input(&mut stream, fid, "ACCEPTED"); + + let op = pump(&mut stream, "the TextInput edit", |msg| match msg { + InstanceMessage::CrdtOp { + buffer_id: b, op, .. + } if *b == buffer_id => Some(op.bytes.clone()), + _ => None, + }); + let text = String::from_utf8_lossy(&op).into_owned(); + assert!( + text.contains("ACCEPTED"), + "a v24 session must be able to insert through TextInput; got {text:?}" + ); +} diff --git a/tests/statusline_segments_acceptance.rs b/tests/statusline_segments_acceptance.rs index c577af4..c927956 100644 --- a/tests/statusline_segments_acceptance.rs +++ b/tests/statusline_segments_acceptance.rs @@ -804,11 +804,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, 23); - for version in 6..=23 { + assert_eq!(PROTOCOL_VERSION, 24); + for version in 6..=24 { assert!(is_supported_protocol_version(version)); } - assert!(!is_supported_protocol_version(24)); + assert!(!is_supported_protocol_version(PROTOCOL_VERSION + 1)); 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 92c287b..837d242 100644 --- a/tests/vterm_stage3_acceptance.rs +++ b/tests/vterm_stage3_acceptance.rs @@ -888,10 +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. v23 is - // `MinibufferPromptRows` (Discovery Stage 2); v22 was - // `LineWrapFacts` (long-lines Stage 3). - assert_eq!(PROTOCOL_VERSION, 23); + // Tripwire: a wire bump must be a conscious edit here. v24 is + // `TextInput` (GUI arc Stage 1a); v23 was `MinibufferPromptRows` + // (Discovery Stage 2); v22 was `LineWrapFacts` (long-lines Stage 3). + assert_eq!(PROTOCOL_VERSION, 24); let daemon = common::daemon::TestDaemon::spawn_with_env_and_init( &[ ("PMACS_INSTANCE_SEMANTIC_RENDER", "1"),