From cd888bed5ee1e67958b89665785afbe65009b78e Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 12 Aug 2026 14:29:04 +0200 Subject: [PATCH 01/12] docs: Stage 1a --- re-measure the framing's ground truth, and open the lane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Docs only, and the first commit on the 1a branch. **No ruling changes.** **1a needs no framing of its own** --- `docs/gui-stage1-input-framing.md` already governs every Stage 1 slice, and A1-A9, the eight Q#S1-9 precedence rules, §8's wire contract and §11's gates are ruled there. What 1a does need is §2 to be TRUE, and it was measured at `a994f37`, before 1-pre moved almost every GPU-side coordinate in it. A framing whose ground truth points at the wrong lines is how an implementation ends up arguing with the tree. Re-measured at `4f77491`, and split by kind rather than renumbered wholesale, because the two are not the same claim: STILL TRUE `FrontendEvent`'s sixteen variants, `PROTOCOL_VERSION = 23`, IME's zero occurrences, the TUI wheel arms (1-pre touched only `pmacs-gpu`), the handshake preceding the window, and `Outbox::enqueue`'s closed-return. MOVED `window_event` `:2734`/655 lines -> `:4450`/FOUR lines; `translate_key` `:10975` -> `:12053`; "eight arms, rest fall to `_`" -> three family decision functions over nine variants. **1a edits `apply_keyboard` and `translate_key`, not `window_event`.** **TWO CLAIMS WERE WRONG AT BOTH ANCHORS, so they are corrections rather than drift.** **"`KeyEvent.text` is never read" is false, and was false at `a994f37` too** (`:2800` there, `main.rs:3251` now): the AltGr rule reads it via `is_layout_text(key.text.as_deref(), pmods)`. The true claim is narrower and is the one that matters --- **`text` is never read as the text a keypress INSERTS**, only as a discriminator separating AltGr from a command chord. That is load-bearing for A5 rather than cosmetic. §5's rule 2 already exempts "printable Ctrl+Alt recognized by the existing AltGr rule", so the precedence table depends on code the section claimed did not exist. **1a widens `text` from discriminator to payload, and that is the actual change of kind** --- which "text is never read" hid, along with the one place the new payload must leave undisturbed. **A4's exit site moved without its behaviour changing.** 1-pre routed the idle-Escape quit through `EventOutcome`, so **A4 edits a branch and a return type in `apply_keyboard` (`main.rs:3219`) and does not touch `window_event`**, which holds the crate's only executable `event_loop.exit()` (`main.rs:4452`). And **`EventOutcome` survives A4**: a native close still returns `Exit`. The lane records the v24 serialization constraint and the bump's blast radius up front --- eight version-sensitive failures across six suites on the last bump, of which CI showed one, because cargo stops at the first failing target --- together with the rule for sorting them and the pin that must never fire, `ADVERTISED_PROTOCOL_VERSION == 20`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- docs/active-work.md | 50 ++++++++++++++ docs/gui-stage1-input-framing.md | 109 ++++++++++++++++++++++++------- 2 files changed, 135 insertions(+), 24 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index cbed93f..1394cea 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -270,6 +270,56 @@ 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 — branch OPEN, no PR yet + +**Written with the branch's first commit**, per the standing correction +from #171 and #215. + +- **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.** +- **Gates:** `./scripts/gate --protocol` plus the touched input suites. + **`--protocol` is required here** — 1a changes the wire. + ## 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 | From 211241a43744bca3005a3da588c51ad17dc560f8 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 12 Aug 2026 16:35:21 +0200 Subject: [PATCH 02/12] feat(gpu,protocol): 1a --- A1-A5 and the v24 TextInput variant The mechanism, without its witnesses yet; tests follow in the next commits. **A1-A3 were mapping gaps, and forwarding was half the fix.** `translate_key` gained F1-F35, Shift+Tab -> `BackTab`, and `ContextMenu` -> `Menu`. All three already existed in the protocol `Key` enum and the TUI already sent them, so this closes a divergence rather than inventing a convention. **`should_forward_key` had to learn them too** --- translated but unforwarded, they would have mapped correctly and still done nothing, which reads as a daemon keymap gap rather than a frontend one. They forward with ANY modifier, like motion keys: they are command keys that never insert text, so the chord-withholding rule has nothing to protect them from. F-keys are an exhaustive match, not arithmetic off `F1`: winit's `NamedKey` is `#[non_exhaustive]` and its ordering is not a contract, so an offset would corrupt silently the day a variant is inserted. **A4 --- every Escape now reaches the daemon and none exits.** 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, and with one behaviour left there is nothing to choose. Both flags remain live for the OS-paste, round-trip and completion-accept paths. **The v24 wire variant is APPENDED and the reason is postcard.** It encodes a variant by positional index, so widening any variant above would re-interpret every older peer's bytes. `TextInput` carries an untrusted `frontend_id` like its neighbours --- the daemon uses the authenticated source --- plus the text. **It is not `Paste`, and the difference is behavioural.** A terminal receives it as RAW UTF-8, never bracketed (A8): a shell that sees `ESC[200~` treats input as pasted and changes how it handles newlines and completion. The clipboard slot is untouched, because nothing was copied. And the document path is ONE edit (A6) --- one undo unit, one `buffer.after-edit`, one eligible CRDT op --- which is the entire reason the variant exists, since a two-scalar grapheme sent as two keypresses is two undo units that a remote edit can interleave. **A5's precedence is a pure function** (`text_input_payload`) so the eight rules are testable without a window. A keypress stays `Key` unless a rule moves it, and only printable MULTI-scalar moves; the version gate WITHHOLDS rather than degrades, so a `< 24` daemon keeps exactly the behaviour it has, truncation included. **A7's ordering falls out of routing through the existing shadow handlers** one scalar at a time, rather than reaching into prompt state: history, completion and acceptance stay in one place. THE 1-PRE EFFECT HARNESS CAUGHT A REAL DEFECT IN THIS COMMIT. Bumping `PROTOCOL_VERSION` to 24 while leaving `SUPPORTED_PROTOCOL_VERSIONS` at `..=23` made the handshake reject its own version. All NINE effect rows failed while the thirteen routing rows passed --- the M21 signature, meaning `EffectHarness::new` could not attach at all. A pure-routing harness would have stayed green. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- pmacs-gpu/src/attach.rs | 30 +++++ pmacs-gpu/src/main.rs | 205 ++++++++++++++++++++++++++++++---- pmacs-protocol/src/lib.rs | 6 +- pmacs-protocol/src/message.rs | 61 +++++++++- src/daemon.rs | 44 +++++++- src/editor.rs | 104 +++++++++++++++++ src/editor_core.rs | 12 ++ 7 files changed, 432 insertions(+), 30 deletions(-) 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..ab898a8 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}, @@ -3213,23 +3213,27 @@ 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. + // 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!(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; + 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; } @@ -3314,6 +3318,32 @@ impl App { return EventOutcome::Continue; } + // A5 — multi-scalar text travels as one `TextInput`, if the + // session can carry it. + // + // Sited AFTER the command-chord and OS-paste branches and + // BEFORE the optimistic path, which is the order §5's rules + // describe: a chord is never text, and text must not be + // optimistically applied one scalar at a time when the whole + // point is that it is one edit. + // + // **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 below + // is the unchanged `Key` path. That is the no-regression + // promise, not retroactive correctness. + if let Some(text) = text_input_payload(&key.logical_key, key.text.as_deref(), 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 let Err(e) = client.send_text_input(text) { + eprintln!("pmacs-gpu: send_text_input failed: {e}"); + } + return EventOutcome::Continue; + } + // Session B2 forwards cursor motion + plain text editing // (Char / Backspace / Enter / Delete / Tab). Command chords // are handled above; Meta/Super-only chords fall through @@ -12070,9 +12100,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 +12127,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 +12270,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; } 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..712eff5 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,7 @@ 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; +pub const PROTOCOL_VERSION: u32 = 24; /// Protocol version placed in the daemon's server-first [`Hello`]. /// @@ -2022,7 +2077,7 @@ pub fn negotiated_session_version(frontend_offer: u32) -> u32 { /// `>= 23` peer receives only the rows form, and no peer ever receives /// both. [`ADVERTISED_PROTOCOL_VERSION`] does not move. pub const SUPPORTED_PROTOCOL_VERSIONS: &[u32] = &[ - 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 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..ed67c3e 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, }; use crate::socket_path::{SocketPathError, ensure_runtime_subdir}; use crate::transport::{read_message, write_message}; @@ -2518,6 +2519,36 @@ 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. + 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 +3619,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..3860258 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -1881,6 +1881,110 @@ 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 `true` when the text was consumed by a shadow or a + /// terminal, `false` when the caller should treat it as an ordinary + /// document edit — the same convention as [`Self::dispatch_paste`]. + 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; + } + + // A6 — the ordinary document path: ONE edit. + self.with_after_edit_check(|state| { + let mut core = state.core.borrow_mut(); + // Provenance follows §5. A single-scalar commit is + // indistinguishable from a keypress and keeps today's + // one-codepoint typed provenance; a multi-scalar commit is + // not a command and breaks the chain, exactly as a paste + // does. + if text.chars().count() > 1 { + core.break_command_chain(frontend_id); + } + if let Err(e) = core.insert_text_input(text) { + eprintln!("pmacs: text input failed: {e}"); + } + }); + } + + /// 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..51f71b7 100644 --- a/src/editor_core.rs +++ b/src/editor_core.rs @@ -5009,6 +5009,18 @@ impl EditorCore { self.insert_bytes_over_region(data) } + /// GUI arc Stage 1a / A6 — insert committed text as **one edit**. + /// + /// 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 From 266bc6e4f3e1c1f08e5585f8fffab733973a00c4 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 12 Aug 2026 17:05:55 +0200 Subject: [PATCH 03/12] fix(gpu,daemon): 1a review round 1 --- three P1s, and the version fallout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **A7 AND A8 WERE UNREACHABLE FROM THE REAL PRODUCER.** The intercept branch sends a truncated `Key` and returns, and TextInput classification sat below it --- but a modal prompt or a focused terminal is exactly what makes `daemon_intercepts_keys` true, so the two contracts about prompts and terminals were reachable only when neither was present. The selection moves ABOVE the intercept return, where the producer sends the same `TextInput` in every state and the daemon applies the modal precedence, which is where it belongs: the frontend cannot see which shadow is up. Ordering against the branches below is safe by construction rather than by luck --- `text_input_payload` returns `None` whenever a command modifier is held, so Ctrl-V and command chords can never be shadowed. **A pure `text_input_payload` test cannot catch this**, which is the lesson worth keeping: the classifier was right the whole time and the call site was wrong. The witness has to drive `intercept = true` and a terminal. **SINGLE-SCALAR PROVENANCE WAS PROMISED IN A COMMENT AND NOT IMPLEMENTED.** §5 rules that a single-scalar commit is indistinguishable from a keypress; the code only broke the chain for multi-scalar and called a generic insert, so `this_command` went stale and no `TypedEditRecord` was produced. Auto-pairing (Q#AP9) and every other typed-edit consumer would have silently stopped recognizing GUI input --- surfacing as "auto-pair stopped working in the GUI", far from its cause. Now runs the real machinery: `rotate_command("buffer.self-insert")` -> `typed_edit_arm(ch)` -> the one edit -> `typed_edit_finish` -> `typed_edit_set_armed` -> `buffer.after-edit` -> clear, which is the tail `dispatch_key` already runs. **THE PRODUCER GATE WAS ONLY HALF THE WIRE CONTRACT.** The daemon accepted `TextInput` from every installed session, so a peer negotiated at v6-v23 --- compiled from this same crate, and postcard will happily write the discriminant --- could mutate a buffer through a variant its own session never declared. Now gated on the AUTHENTICATED session's negotiated version. **A4's structural half is implemented, not just its behaviour.** `apply_keyboard` returns `()`, so `LifecycleRoute::Exit` is the sole `EventOutcome::Exit` producer and the obsolete keyboard-exit channel is gone rather than merely unused. The type survives, as ruled: one producer is not one variant. Also: `dispatch_text_input`'s rustdoc claimed a boolean return that its signature does not have. VERSION FALLOUT, SORTED RATHER THAN RENUMBERED. Six deliberate tripwires took the conscious edit they exist to force (protocol.rs, bottom-panel, discovery x2, statusline, and the vterm one that was missing from my inventory). Two carried the version in their NAME, so the name moved with the number rather than being left to lie. Two ceiling assertions --- `!is_supported_protocol_version(24)` --- now probe `PROTOCOL_VERSION + 1`, so they keep meaning "the set ends at the current wire" instead of needing a hand-edit every bump. `m4_6_handshake_accepts_v6_peer` was GENUINELY DEFECTIVE and is the one real find: its name and the M4.6 contract say **v6 is the floor**, but its body asserted `is_supported_protocol_version(PROTOCOL_VERSION)` --- "the current wire accepts itself", a different and far weaker claim that would have kept passing after v6 was dropped from the supported set, which is the only regression it exists to catch. Anchored on literal 6. The M10 pair needed no edit: they already use `PROTOCOL_VERSION`, and they failed in the first sweep only because it predated the `SUPPORTED_PROTOCOL_VERSIONS` fix. `ADVERTISED_PROTOCOL_VERSION == 20` did not fire, as it must not. Full `--workspace --no-fail-fast` sweep clean under an isolated TMPDIR. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- pmacs-gpu/src/main.rs | 119 ++++++++++--------- src/daemon.rs | 21 +++- src/editor.rs | 66 +++++++--- src/protocol.rs | 34 ++++-- tests/bottom_panel_stage2b_gpu_acceptance.rs | 7 +- tests/discovery_stage2_acceptance.rs | 13 +- tests/statusline_segments_acceptance.rs | 6 +- tests/vterm_stage3_acceptance.rs | 8 +- 8 files changed, 182 insertions(+), 92 deletions(-) diff --git a/pmacs-gpu/src/main.rs b/pmacs-gpu/src/main.rs index ab898a8..5584d44 100644 --- a/pmacs-gpu/src/main.rs +++ b/pmacs-gpu/src/main.rs @@ -3162,7 +3162,7 @@ impl App { Route::Keyboard { action: KeyAction::Press, key, - } => return self.apply_keyboard(key), + } => self.apply_keyboard(key), Route::Pointer(PointerRoute::Moved { x, y }) => self.apply_cursor_moved(x, y), Route::Pointer(PointerRoute::Left(button_state)) => { self.apply_left_button(button_state); @@ -3186,7 +3186,7 @@ impl App { /// Perform [`KeyAction::Press`]. The router has already /// discarded key-ups, so `key` is always a press. #[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, key: &KeyEvent) { // 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 @@ -3235,11 +3235,11 @@ impl App { { 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; + return; }; // AltGr / international text (audit F-004). winit reports @@ -3275,13 +3275,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(&key.logical_key, key.text.as_deref(), 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 { @@ -3294,7 +3338,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 @@ -3315,33 +3359,7 @@ impl App { if let Err(e) = client.send_key(pkey, pmods) { eprintln!("pmacs-gpu: send_key (command chord) failed: {e}"); } - return EventOutcome::Continue; - } - - // A5 — multi-scalar text travels as one `TextInput`, if the - // session can carry it. - // - // Sited AFTER the command-chord and OS-paste branches and - // BEFORE the optimistic path, which is the order §5's rules - // describe: a chord is never text, and text must not be - // optimistically applied one scalar at a time when the whole - // point is that it is one edit. - // - // **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 below - // is the unchanged `Key` path. That is the no-regression - // promise, not retroactive correctness. - if let Some(text) = text_input_payload(&key.logical_key, key.text.as_deref(), 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 let Err(e) = client.send_text_input(text) { - eprintln!("pmacs-gpu: send_text_input failed: {e}"); - } - return EventOutcome::Continue; + return; } // Session B2 forwards cursor motion + plain text editing @@ -3350,7 +3368,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 @@ -3387,7 +3405,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) { @@ -3397,7 +3415,7 @@ impl App { pending optimistic cursor" ); } - return EventOutcome::Continue; + return; } state.mark_cursor_stale_after_round_trip(); } @@ -3407,7 +3425,6 @@ impl App { if let Err(e) = client.send_key(pkey, pmods) { eprintln!("pmacs-gpu: send_key failed: {e}"); } - EventOutcome::Continue } } @@ -3448,23 +3465,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, diff --git a/src/daemon.rs b/src/daemon.rs index ed67c3e..c9d5973 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -71,7 +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_MAX_BYTES, TEXT_INPUT_MIN_VERSION, }; use crate::socket_path::{SocketPathError, ensure_runtime_subdir}; use crate::transport::{read_message, write_message}; @@ -2536,6 +2536,25 @@ fn handle_dispatcher_event( // 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; } diff --git a/src/editor.rs b/src/editor.rs index 3860258..52bc0f7 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -1906,9 +1906,11 @@ impl EditorState { /// a time, in order** (A7) — order is the contract, since a prompt /// accumulates a query. /// - /// Returns `true` when the text was consumed by a shadow or a - /// terminal, `false` when the caller should treat it as an ordinary - /// document edit — the same convention as [`Self::dispatch_paste`]. + /// 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; @@ -1924,21 +1926,57 @@ impl EditorState { return; } - // A6 — the ordinary document path: ONE edit. - self.with_after_edit_check(|state| { - let mut core = state.core.borrow_mut(); - // Provenance follows §5. A single-scalar commit is - // indistinguishable from a keypress and keeps today's - // one-codepoint typed provenance; a multi-scalar commit is - // not a command and breaks the chain, exactly as a paste - // does. - if text.chars().count() > 1 { - core.break_command_chain(frontend_id); + // §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(); + match single { + Some(ch) => { + core.rotate_command(frontend_id, "buffer.self-insert"); + core.typed_edit_arm(frontend_id, ch); + } + None => core.break_command_chain(frontend_id), + } + // A6 — ONE edit, whichever branch armed it. 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 diff --git a/src/protocol.rs b/src/protocol.rs index 33beee0..483ed59 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" ); } } @@ -2738,9 +2743,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..8afa29a 100644 --- a/tests/discovery_stage2_acceptance.rs +++ b/tests/discovery_stage2_acceptance.rs @@ -67,10 +67,10 @@ 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, @@ -85,7 +85,10 @@ fn the_wire_is_v23_and_the_advertised_baseline_is_unmoved() { "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 +615,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/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"), From 97da79f136d6d3eda863629758d50d1cff8cd9d1 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 12 Aug 2026 17:20:15 +0200 Subject: [PATCH 04/12] fix: 1a review round 2 --- provenance was armed but never completed **The P1 was real and my previous fix only did half of it.** Arming and completing a `TypedEditRecord` are separate steps: `typed_edit_complete` is called from `insert_char` / `insert_char_over_region` and NOWHERE else, so routing the single-scalar branch through the generic `insert_text_input` left the arm holding `None`. `this_command` rotated correctly and `buffer.after-edit` saw no record --- auto-pairing stayed broken while the command side looked right, which is the failure mode that hides longest. The single-scalar branch now goes through `insert_char_over_region(ch)`, which handles the no-region case itself by delegating to `insert_char`. `insert_text_input` is documented as the MULTI-scalar path only, with the trap named at the definition rather than left for the next caller to rediscover. **The witness consumes the record rather than inspecting `this_command`**, per the review. `single_scalar_text_input_produces_a_consumable_typed_edit_record` takes it through the same `pmacs.pair._last_record` seam `pair.lua` uses, and `single_scalar_text_input_auto_pairs_like_a_keypress` states the same fact in the terms a user would notice: typing `(` must produce `()`. **Mutation M-1a-1 reverts the fix and both rows fail**; the four others stay green, so they are discriminating rather than duplicated. **A ceiling tripwire proved less than it claimed.** The discovery acceptance looped `6..=23` and then rejected `PROTOCOL_VERSION + 1`, so a supported set that ENDED at 23 would have passed while `PROTOCOL_VERSION` was 24 --- the accepted half said nothing about the version the constant names. It runs to `PROTOCOL_VERSION` now. **The public protocol history stopped at v23 while both constants already included 24.** The rustdoc above `PROTOCOL_VERSION` and `SUPPORTED_PROTOCOL_VERSIONS` now carries the v24 bump, and states the thing that makes it unlike its predecessors: **it is the first FRONTEND->INSTANCE extension needing a gate in BOTH directions**, because the producer withholding is not enough when a peer compiled from this same crate can encode the variant whatever it negotiated. Three typed-edit doc sites said the arm is set by "the dispatch fallback only" or named two producers; there are three now, and the single-scalar `TextInput` path is one. `typed_edit_arm`'s own doc gains the warning that arming is only half. Also 1a's suite: A6 (one commit, one edit, one undo unit), A7 (a prompt accumulates scalars in order), and A9's boundary row --- a payload exactly at the cap lands intact, the complement of the rejection that is enforced where a test can reach it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- pmacs-protocol/src/message.rs | 26 ++++ src/editor.rs | 31 ++-- src/editor_core.rs | 34 ++++- tests/discovery_stage2_acceptance.rs | 9 +- tests/gui_stage1a_acceptance.rs | 208 +++++++++++++++++++++++++++ 5 files changed, 289 insertions(+), 19 deletions(-) create mode 100644 tests/gui_stage1a_acceptance.rs diff --git a/pmacs-protocol/src/message.rs b/pmacs-protocol/src/message.rs index 712eff5..517fe34 100644 --- a/pmacs-protocol/src/message.rs +++ b/pmacs-protocol/src/message.rs @@ -1895,6 +1895,24 @@ 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. +/// +/// 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. +/// +/// **This is the first FRONTEND→INSTANCE extension to need a gate in +/// both directions**, and the reason is that the direction of travel is +/// reversed: for an instance→frontend variant the daemon simply +/// withholds, but here the *frontend* must withhold below +/// [`TEXT_INPUT_MIN_VERSION`] **and** the daemon must refuse what a peer +/// below it nonetheless sends. A client built from this crate can encode +/// the variant whatever it negotiated, so the producer gate alone would +/// leave a v6–v23 session able to drive an edit through a variant its +/// own session never declared. pub const PROTOCOL_VERSION: u32 = 24; /// Protocol version placed in the daemon's server-first [`Hello`]. @@ -2076,6 +2094,14 @@ 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 in **both** +/// directions rather than only daemon-side — see [`PROTOCOL_VERSION`] +/// for why an inbound frontend→instance variant needs the receiving +/// check too. [`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, 24, ]; diff --git a/src/editor.rs b/src/editor.rs index 52bc0f7..b08c26a 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -1950,16 +1950,29 @@ impl EditorState { let pre_revision = self.active_buffer_revision(); { let mut core = self.core.borrow_mut(); - match single { - Some(ch) => { - core.rotate_command(frontend_id, "buffer.self-insert"); - core.typed_edit_arm(frontend_id, ch); + 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}"); } - None => core.break_command_chain(frontend_id), - } - // A6 — ONE edit, whichever branch armed it. - if let Err(e) = core.insert_text_input(text) { - eprintln!("pmacs: text input failed: {e}"); } } diff --git a/src/editor_core.rs b/src/editor_core.rs index 51f71b7..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,7 +5022,14 @@ impl EditorCore { self.insert_bytes_over_region(data) } - /// GUI arc Stage 1a / A6 — insert committed text as **one edit**. + /// 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 diff --git a/tests/discovery_stage2_acceptance.rs b/tests/discovery_stage2_acceptance.rs index 8afa29a..db5f8cc 100644 --- a/tests/discovery_stage2_acceptance.rs +++ b/tests/discovery_stage2_acceptance.rs @@ -77,9 +77,12 @@ fn the_wire_is_v24_and_the_advertised_baseline_is_unmoved() { "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" diff --git a/tests/gui_stage1a_acceptance.rs b/tests/gui_stage1a_acceptance.rs new file mode 100644 index 0000000..369e653 --- /dev/null +++ b/tests/gui_stage1a_acceptance.rs @@ -0,0 +1,208 @@ +//! 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" + ); + let this_command: String = eval(&s, "return pmacs.editor.this_command() or ''"); + assert_ne!( + this_command, "buffer.self-insert", + "a multi-scalar commit is not a self-insert" + ); +} + +/// 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" + ); +} + +#[path = "common/iso.rs"] +mod iso; From b2a273b1fd958b4c6af22afcd70a2582ce551817 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 12 Aug 2026 17:29:21 +0200 Subject: [PATCH 05/12] docs,test: 1a review round 3 --- an overclaimed first, and a vacuous row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **"The first frontend→instance extension needing a gate in both directions" was false**, and checking it took one grep: the v19 terminal family and the v21 panel family each have an inbound variant AND an outbound one, each gated. "First inbound-only extension" does not hold either --- v7's `TripleDown` and v11's `Context` are inbound-only `PointerKind` variants. So the superlative is gone rather than weakened. The rustdoc now states the PROPERTY and its consequence: the gate is producer-side and receiver-side, and for an inbound variant the receiving half is the load-bearing one, because withholding would otherwise be the peer's job and a client built from this same crate can encode the discriminant whatever it negotiated. The v19/v21 precedent is named so the shape does not read as novel; what is unusual is only that this extension has no outbound counterpart, so the receiver check is the whole of the daemon's half. **`multi_scalar_text_input_creates_no_typed_provenance` proved half of what its name claimed.** It started from a fresh editor, where the command chain is ALREADY empty, so asserting emptiness afterwards passed whether or not `break_command_chain` ran --- the assertion could not fail for the reason it existed. Split in two. The record half keeps its name and its claim; the chain half is a new row that PRIMES the chain first and then asserts it cleared. Priming goes through 1a's own single-scalar path, because `pmacs.command.invoke('buffer.self-insert')` cannot prime it: rotation belongs to the dispatcher and invoking the command directly deliberately never rotates --- the first attempt at this row failed on exactly that precondition, which is a better outcome than passing on a chain that was never live. **M-1a-2 deletes `break_command_chain` from the multi-scalar branch and fails the new row alone**; the other six stay green. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- pmacs-protocol/src/message.rs | 32 +++++++++++++++++------------ tests/gui_stage1a_acceptance.rs | 36 +++++++++++++++++++++++++++++---- 2 files changed, 51 insertions(+), 17 deletions(-) diff --git a/pmacs-protocol/src/message.rs b/pmacs-protocol/src/message.rs index 517fe34..75340d4 100644 --- a/pmacs-protocol/src/message.rs +++ b/pmacs-protocol/src/message.rs @@ -1904,15 +1904,22 @@ pub enum ResourceBody { /// `PanelPointer`, the final v23 `FrontendEvent` variant, so no /// existing discriminant moves. /// -/// **This is the first FRONTEND→INSTANCE extension to need a gate in -/// both directions**, and the reason is that the direction of travel is -/// reversed: for an instance→frontend variant the daemon simply -/// withholds, but here the *frontend* must withhold below -/// [`TEXT_INPUT_MIN_VERSION`] **and** the daemon must refuse what a peer -/// below it nonetheless sends. A client built from this crate can encode -/// the variant whatever it negotiated, so the producer gate alone would -/// leave a v6–v23 session able to drive an edit through a variant its -/// own session never declared. +/// **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`]. @@ -2096,10 +2103,9 @@ pub fn negotiated_session_version(frontend_offer: u32) -> u32 { /// both. [`ADVERTISED_PROTOCOL_VERSION`] does not move. /// /// GUI arc Stage 1a: extended to `[6, ..., 24]` for -/// [`FrontendEvent::TextInput`]. Additive, and gated in **both** -/// directions rather than only daemon-side — see [`PROTOCOL_VERSION`] -/// for why an inbound frontend→instance variant needs the receiving -/// check too. [`ADVERTISED_PROTOCOL_VERSION`] does not move: a v23 +/// [`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] = &[ diff --git a/tests/gui_stage1a_acceptance.rs b/tests/gui_stage1a_acceptance.rs index 369e653..e322694 100644 --- a/tests/gui_stage1a_acceptance.rs +++ b/tests/gui_stage1a_acceptance.rs @@ -127,10 +127,38 @@ fn multi_scalar_text_input_creates_no_typed_provenance() { no_record, "a multi-scalar commit must not forge a typed-edit record" ); - let this_command: String = eval(&s, "return pmacs.editor.this_command() or ''"); - assert_ne!( - this_command, "buffer.self-insert", - "a multi-scalar commit is not a self-insert" +} + +/// §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" ); } From 3ed37f77862e467fe307c1326980600d071a5710 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 12 Aug 2026 17:34:56 +0200 Subject: [PATCH 06/12] test(gpu): 1a --- producer reachability, and a narrowed structural exception MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first of the three remaining discriminators: **multi-scalar text reaches the wire as `TextInput` while the daemon is INTERCEPTING** --- the state a modal prompt or a focused terminal puts the session in, and the state under which A7 and A8 were unreachable before review round 1. **Getting there required narrowing the 1-pre exception, which is the substantive part.** `apply_keyboard` took a `&KeyEvent`; `KeyEvent` carries a `pub(crate)` field and cannot be constructed outside winit, so the body was undrivable and only the pure classifier could be tested --- which is precisely why the defect survived: the classifier was correct throughout and the CALL SITE was wrong. It now takes the two fields it actually reads, `&Key` and `Option<&str>`, both ordinary constructible values. The exception does not disappear, it shrinks: the router arm still cannot be handed a `WindowEvent::KeyboardInput`, so what remains unwitnessed is one pattern arm containing a match and a call. That is recorded on `apply_keyboard` itself, where the next reader meets it. **M-1a-3 reinstates the original defect** --- the selection moved back below the intercept return --- **and fails the new row alone**, 23 of 24 still green. That is the shape the review asked for: a witness that fails for the reason the defect existed. Its complement is included so the pair cannot be satisfied by sending `TextInput` for everything: a SINGLE scalar while intercepting still travels as `Key`, which is §5 rule 4 preserving mode keymaps and typed provenance. The harness gains `feed_keyboard`, and the local-effect diffing it shares with `feed` is extracted rather than copied --- two entry points observing different effect sets by accident is the kind of divergence that makes a transcript lie. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- pmacs-gpu/src/main.rs | 137 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 124 insertions(+), 13 deletions(-) diff --git a/pmacs-gpu/src/main.rs b/pmacs-gpu/src/main.rs index 5584d44..eeca80f 100644 --- a/pmacs-gpu/src/main.rs +++ b/pmacs-gpu/src/main.rs @@ -3162,7 +3162,7 @@ impl App { Route::Keyboard { action: KeyAction::Press, key, - } => 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) { + 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 @@ -3229,7 +3244,7 @@ impl App { // 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!(key.logical_key, Key::Named(NamedKey::Escape)) { + 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) { @@ -3238,7 +3253,7 @@ impl App { return; } - let Some((pkey, mut pmods)) = translate_key(&key.logical_key, self.modifiers) else { + let Some((pkey, mut pmods)) = translate_key(logical, self.modifiers) else { return; }; @@ -3252,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 { @@ -3307,7 +3322,7 @@ impl App { // 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(&key.logical_key, key.text.as_deref(), pmods) + 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() { @@ -3835,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); @@ -3862,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. @@ -4393,6 +4434,76 @@ 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 + ); + } + + /// 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 From e3a19a4e6386f04b5923d9c6c8f7433f247ff613 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 12 Aug 2026 21:29:48 +0200 Subject: [PATCH 07/12] test: 1a --- A8 raw PTY delivery, and the negotiated-v23 refusal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last two of the three discriminators. Each is proven by a mutation that reinstates the defect it exists to catch. **A8, delivered rather than routed.** The producer row proved a terminal-focused session reaches the `TextInput` branch; this one observes the CHILD PROCESS. A real PTY runs `printf '\033[?2004h'; exec cat > FILE`, so the terminal turns bracketed paste ON and then records exactly what arrives on its stdin. **The enabled mode is the entire precondition**, and the row waits for the child's own mode-set to be parsed before typing rather than assuming it: with bracketed paste OFF, "no markers" is true of every code path including a paste, so the assertion would pass against the behaviour it forbids. The contrast closes it from the other side — through the SAME terminal in the SAME mode, a paste IS bracketed. One path marked and the other not, both observed at the PTY. `M-1a-4` routes typed text through `encode_paste` and the row fails with the forbidden bytes in hand: `"\u{1b}[200~héllo\u{301}\u{1b}[201~"`. **The negotiated-v23 refusal** gets its own suite, because it needs a live daemon. A refusal is the hardest claim to witness honestly — "nothing happened" is also what a dead daemon, a desynchronized stream or a broken test look like — so the row pairs it with a POSITIVE CONTROL on the same session: after the refused `TextInput`, an ordinary `Key` that must take effect. Events from one session are processed in order, so the control's edit arriving with no preceding `REFUSED` edit means the gate fired rather than that the daemon was asleep. Its complement runs the same traffic on a v24 session and requires the edit to land, so the pair cannot be satisfied by `TextInput` being broken outright. `M-1a-5` disables the inbound gate and the v23 row fails with `REFUSED` visible inside the CRDT op — a v23 peer editing a buffer through a variant its session never declared, which is precisely the hole review round 1 identified. One setup lesson, recorded because it cost a red: the A8 row first failed with an empty file, and the cause was that the frontend's view was never pointed at the terminal buffer, so `active_terminal_key` returned `None` and the DOCUMENT path ran. It now asserts that precondition through public state before typing — a row that quietly tests the document path and reports a terminal result is worse than one that fails. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- tests/gui_stage1a_acceptance.rs | 174 +++++++++++++++++++++++++ tests/gui_stage1a_wire_acceptance.rs | 186 +++++++++++++++++++++++++++ 2 files changed, 360 insertions(+) create mode 100644 tests/gui_stage1a_wire_acceptance.rs diff --git a/tests/gui_stage1a_acceptance.rs b/tests/gui_stage1a_acceptance.rs index e322694..b0ca30b 100644 --- a/tests/gui_stage1a_acceptance.rs +++ b/tests/gui_stage1a_acceptance.rs @@ -232,5 +232,179 @@ fn a9_a_payload_at_the_cap_is_inserted_whole() { ); } +// --------------------------------------------------------------------- +// 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); + + 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" + ); + 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("\u{1b}[200~") { + assert!( + all.contains("pasted") && all.contains("\u{1b}[201~"), + "a paste is bracketed on both sides: {all:?}" + ); + break; + } + assert!( + Instant::now() < deadline, + "a paste through the same terminal must be bracketed; got {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:?}" + ); +} From 42a0a91f5886abf3b3c7ddf82bb05ee6efd949d3 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 12 Aug 2026 22:38:45 +0200 Subject: [PATCH 08/12] test: A8's contrast waited for a prefix, not the sequence **A partial write could have failed correct code.** The contrast broke out of its poll as soon as `ESC[200~` appeared and then required `pasted` and `ESC[201~` in the same breath --- but PTY delivery and the child's writes split anywhere, so the closer may simply not have arrived. A flake that only ever fires on a loaded machine, and one that would have been read as an A8 regression. It now polls for the COMPLETE `ESC[200~pastedESC[201~`, which makes a partial write indistinguishable from "not yet" --- which is what it is. Same rule the vterm suite already follows when it waits for `row19` rather than for a prefix of it. **The typed-text read above it is sound and is now documented as such**, because the two loops look alike and are not. Its gate is a LOWER BOUND ON LENGTH: a split delivery keeps waiting rather than being mistaken for a wrong answer, and the exact-equality assertion afterwards can still fail for the real reason. A wait-for-exact-content loop there would have been tautological. Stating the difference beside the two loops is worth more than making them superficially uniform. M-1a-4 re-run after the change: still fails the row, so the fix removed a race without removing the discrimination. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- tests/gui_stage1a_acceptance.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/tests/gui_stage1a_acceptance.rs b/tests/gui_stage1a_acceptance.rs index b0ca30b..dad89e5 100644 --- a/tests/gui_stage1a_acceptance.rs +++ b/tests/gui_stage1a_acceptance.rs @@ -324,6 +324,12 @@ fn a8_a_terminal_receives_raw_utf8_with_bracketed_paste_enabled() { 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(); @@ -356,20 +362,27 @@ fn a8_a_terminal_receives_raw_utf8_with_bracketed_paste_enabled() { 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("\u{1b}[200~") { - assert!( - all.contains("pasted") && all.contains("\u{1b}[201~"), - "a paste is bracketed on both sides: {all:?}" - ); + if all.contains(want) { break; } assert!( Instant::now() < deadline, - "a paste through the same terminal must be bracketed; got {all:?}" + "a paste through the same terminal must be bracketed on both \ + sides; waited for {want:?}, saw {all:?}" ); std::thread::sleep(Duration::from_millis(10)); } From 76beb79a4de96f91f5a8a9ad2b55a9b684bed118 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 12 Aug 2026 22:46:00 +0200 Subject: [PATCH 09/12] test: 1a --- A1-A4 witnesses and the PanelPointer frozen-byte pin **A1 is an exhaustive loop over all 35 function keys, not spot checks.** The defect it closes was `_ => return None` swallowing F13 upward, and a test covering F1-F12 would have passed against exactly that. Each row also asserts `should_forward_key`, because translating without forwarding leaves a key mapped and inert --- which reads as a daemon keymap gap rather than a frontend one. **A2 asserts both halves**: `BackTab`, and `Shift` still set. A `BackTab` that lost its modifier is indistinguishable from one the user did not shift. **A3** likewise pairs the mapping with forwarding. **A4 establishes idle rather than asserting it.** A fresh `State` starts with `dispatch_idle` false --- the daemon has not spoken yet --- so the first version of the row asserted the precondition and failed. Had it been written the other way round it would have tested the INTERCEPTING case under an idle name, which is the state where Escape never quit anyway: the row would have passed while proving nothing about the behaviour A4 changes. It now sets idle, confirms nothing intercepts, and asserts both halves: the Escape reaches the daemon AND no exit occurs. **The frozen-byte pin sits on `PanelPointer`, not on `TextInput`, and the placement is the point.** `TextInput` is appended, so its own round-trip is byte-identical whether or not a variant was inserted beneath it; only the PREVIOUS final variant's bytes move. Every v6-v23 daemon decodes the variants below `PanelPointer` on every session, so an insertion anywhere earlier is a silent wire break for all of them. MY FIRST MUTATION OF THAT PIN WAS WRONG AND THE PIN WAS RIGHT. I inserted the wedge variant before `TextInput` --- which is to say AFTER `PanelPointer`, exactly where an append belongs --- and the pin passed, correctly, because nothing shifted. Re-run with the wedge BEFORE `PanelPointer`, it fails with the discriminant visibly moving 15 -> 16. Worth recording because a mutation that targets the wrong side of the boundary reports the pin as vacuous when it is sound. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- pmacs-gpu/src/main.rs | 141 ++++++++++++++++++++++++++++++++++++++++++ src/protocol.rs | 32 ++++++++++ 2 files changed, 173 insertions(+) diff --git a/pmacs-gpu/src/main.rs b/pmacs-gpu/src/main.rs index eeca80f..c42fd94 100644 --- a/pmacs-gpu/src/main.rs +++ b/pmacs-gpu/src/main.rs @@ -4476,6 +4476,48 @@ mod input_routing_tests { ); } + /// 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 @@ -13709,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/src/protocol.rs b/src/protocol.rs index 483ed59..ede6caf 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -1971,6 +1971,38 @@ 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. + // Every v6–v23 daemon decodes the variants below it on every + // session, so a variant inserted anywhere earlier is a silent + // wire break for all of them. + 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); From 69b9e6a41f8b55a171d8766f4014031e3a986ef0 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 12 Aug 2026 22:55:55 +0200 Subject: [PATCH 10/12] docs: record 1a's gate result and mutation inventory Sixteen gates green with --protocol, under an isolated TMPDIR. The crdt-gated wire suite is verified to have RUN --- 2 tests in the crdt sweep, 0 in the default one --- rather than assumed, because a suite that compiles to nothing reports ok and that is the shape this project has been caught by before. The mutation inventory names which row each one fails, including M-1a-6: a mutation that targeted the wrong side of the append boundary and so reported a sound pin as vacuous. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- docs/active-work.md | 39 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index 1394cea..9866ef8 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -317,8 +317,43 @@ from #171 and #215. 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.** -- **Gates:** `./scripts/gate --protocol` plus the touched input suites. - **`--protocol` is required here** — 1a changes the wire. +- **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 From 2d40aea50912f567fe18757df211ae20d5c0b8c2 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 12 Aug 2026 22:56:41 +0200 Subject: [PATCH 11/12] docs: state the PanelPointer pin's blast radius exactly The comment claimed every v6-v23 daemon decodes the variants before `PanelPointer` on every session. False on both counts: `PanelPointer` arrived at v21 and is gated, so a wedge before it shifts its discriminant and breaks v21-v23 PANEL traffic --- not v6-v20, and not every session. The pin itself is unchanged and still fires (15 -> 16). What was wrong was the justification, and an inflated one is not harmless: it sends the next reader hunting a larger blast radius than exists, and it teaches the wrong rule about which populations an insertion actually reaches. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- src/protocol.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/protocol.rs b/src/protocol.rs index ede6caf..474473b 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -1981,9 +1981,14 @@ mod tests { // only they can witness the shift. // // `PanelPointer` was the last `FrontendEvent` variant at v23. - // Every v6–v23 daemon decodes the variants below it on every - // session, so a variant inserted anywhere earlier is a silent - // wire break for all of them. + // 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, From 36a32961f28d50cd90475ad6a861c3d62d78ab0c Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Thu, 13 Aug 2026 08:44:42 +0200 Subject: [PATCH 12/12] docs: record PR #239 in the 1a lane The number goes in the moment the PR opens, per this file's own rule. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- docs/active-work.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/active-work.md b/docs/active-work.md index 9866ef8..fede39e 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -270,11 +270,12 @@ 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 — branch OPEN, no PR yet +## 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