[v0.1/Input Review] Move mode-dependent key encoding to daemon authority #309

Closed
opened 2026-09-21 12:09:06 +09:00 by natsukium · 1 comment
Owner

Parent: #12

Problem

Keyboard bytes are encoded in the client from mode state mirrored by the daemon:

  • felis-client/src/event_handler.rs calls input::encode with the shadow's Kitty flags, DECCKM, DECKPAM, modifyOtherKeys, and win32-input-mode.
  • InputMsg::KeyBytes then carries only the resulting PTY bytes.
  • GridMsg::ModeFlags and KittyKbdFlags are grid updates, and on a pull-paced window those updates may not have reached the client when the next physical key is pressed.

This creates a state-authority race. A producer can enable a keyboard mode and wait for input after the daemon has parsed the mode change but before the client has received the corresponding grid frame. The client then encodes the key under stale state. An occluded window that has stopped pulling makes the gap especially clear.

Mouse input already uses the safer boundary: the client sends a structured MouseEvent, and the daemon encodes it against the authoritative mouse mode. Keyboard input is the exception even though its encoding depends on daemon-owned terminal state in the same way.

Decision to make before the v0.1 contract freezes

Prefer a structured keyboard input arm carrying the platform-independent facts needed by the existing encoder, for example:

  • logical key / named key;
  • composed text when present;
  • modifiers;
  • press, repeat, or release kind;
  • key location.

The daemon should encode that event against its current grid mode snapshot before writing to the PTY. The client still owns OS event capture, IME, and keymap dispatch; only terminal-state-dependent byte encoding moves to the side that owns that state.

Literal byte injection remains necessary for sessions send --raw, send_string, and other explicitly byte-oriented paths. Paste and committed IME text also remain distinct from a physical key event.

If structured input is rejected, record how the existing byte form prevents or accepts the stale-mode race. Merely pushing mode frames eagerly does not establish a cross-direction ordering barrier before a user key.

Acceptance criteria

  • The wire distinguishes a semantic key event from literal byte injection.
  • GUI keys and sessions send --key use one encoding policy and cannot drift.
  • The daemon encodes semantic keys from authoritative Kitty/DECCKM/DECKPAM/modifyOtherKeys/win32 state.
  • A test changes a mode daemon-side, withholds the corresponding client grid update, sends a key, and observes the new-mode PTY bytes.
  • Press/repeat/release and location behavior remain covered by the key-encoding tests.
  • --raw, paste, IME commit, and send_string retain their documented byte semantics.
  • IPC reference/explanation, input docs, minor or pre-release breaking metadata, CLI help/schema where applicable, skills/felis, and CHANGELOG.md agree.

Principle check

  1. Add only what earns its place — Pass. This corrects an existing input path with GUI and CLI consumers and uses a typed IPC variant.
  2. Render everything, fast — Pass. No rendering feature or unmeasured optimization is added.
  3. The daemon owns state, the client owns pixels — Pass. Terminal-mode-dependent encoding is evaluated where raw terminal state is authoritative; client-side OS input ownership is unchanged.
  4. Explicit over heuristic — Pass. The message carries explicit key facts and inspects no shell content.

Evidence

  • crates/felis-client/src/event_handler.rs:301
  • crates/felis-client/src/input.rs
  • crates/felis-client-core/src/key_encode.rs
  • crates/felis-protocol/proto/felis.proto:727
  • crates/felis-daemon/src/serve/session_task.rs (InputMsg::Mouse vs InputMsg::KeyBytes)
  • docs/reference/ipc.md "Input (kind = 1)"
  • docs/explanation/input.md "Keyboard"
  • docs/explanation/architecture/control-surfaces.md "send --key"
Parent: #12 ## Problem Keyboard bytes are encoded in the client from mode state mirrored by the daemon: - `felis-client/src/event_handler.rs` calls `input::encode` with the shadow's Kitty flags, DECCKM, DECKPAM, modifyOtherKeys, and win32-input-mode. - `InputMsg::KeyBytes` then carries only the resulting PTY bytes. - `GridMsg::ModeFlags` and `KittyKbdFlags` are grid updates, and on a pull-paced window those updates may not have reached the client when the next physical key is pressed. This creates a state-authority race. A producer can enable a keyboard mode and wait for input after the daemon has parsed the mode change but before the client has received the corresponding grid frame. The client then encodes the key under stale state. An occluded window that has stopped pulling makes the gap especially clear. Mouse input already uses the safer boundary: the client sends a structured `MouseEvent`, and the daemon encodes it against the authoritative mouse mode. Keyboard input is the exception even though its encoding depends on daemon-owned terminal state in the same way. ## Decision to make before the v0.1 contract freezes Prefer a structured keyboard input arm carrying the platform-independent facts needed by the existing encoder, for example: - logical key / named key; - composed text when present; - modifiers; - press, repeat, or release kind; - key location. The daemon should encode that event against its current grid mode snapshot before writing to the PTY. The client still owns OS event capture, IME, and keymap dispatch; only terminal-state-dependent byte encoding moves to the side that owns that state. Literal byte injection remains necessary for `sessions send --raw`, `send_string`, and other explicitly byte-oriented paths. `Paste` and committed IME text also remain distinct from a physical key event. If structured input is rejected, record how the existing byte form prevents or accepts the stale-mode race. Merely pushing mode frames eagerly does not establish a cross-direction ordering barrier before a user key. ## Acceptance criteria - [ ] The wire distinguishes a semantic key event from literal byte injection. - [ ] GUI keys and `sessions send --key` use one encoding policy and cannot drift. - [ ] The daemon encodes semantic keys from authoritative Kitty/DECCKM/DECKPAM/modifyOtherKeys/win32 state. - [ ] A test changes a mode daemon-side, withholds the corresponding client grid update, sends a key, and observes the new-mode PTY bytes. - [ ] Press/repeat/release and location behavior remain covered by the key-encoding tests. - [ ] `--raw`, paste, IME commit, and `send_string` retain their documented byte semantics. - [ ] IPC reference/explanation, input docs, minor or pre-release breaking metadata, CLI help/schema where applicable, `skills/felis`, and `CHANGELOG.md` agree. ## Principle check 1. **Add only what earns its place — Pass.** This corrects an existing input path with GUI and CLI consumers and uses a typed IPC variant. 2. **Render everything, fast — Pass.** No rendering feature or unmeasured optimization is added. 3. **The daemon owns state, the client owns pixels — Pass.** Terminal-mode-dependent encoding is evaluated where raw terminal state is authoritative; client-side OS input ownership is unchanged. 4. **Explicit over heuristic — Pass.** The message carries explicit key facts and inspects no shell content. ## Evidence - `crates/felis-client/src/event_handler.rs:301` - `crates/felis-client/src/input.rs` - `crates/felis-client-core/src/key_encode.rs` - `crates/felis-protocol/proto/felis.proto:727` - `crates/felis-daemon/src/serve/session_task.rs` (`InputMsg::Mouse` vs `InputMsg::KeyBytes`) - `docs/reference/ipc.md` "Input (kind = 1)" - `docs/explanation/input.md` "Keyboard" - `docs/explanation/architecture/control-surfaces.md` "send --key"
Author
Owner

Direction agreed 2026-09-21 after three sol design-review rounds (final verdict PASS). Full plan and batch order: see the comment on #12.

  • #309 — add a structured InputMsg::Key (named key or character, text,
    a 4-bit modifier set including Super, press/repeat/release, location).
    Wire vocabulary (NamedKey, KeyLocation, key modifiers) moves to
    felis-protocol; the encoder (key_encode.rs with its 102 unit tests and
    11 proptests) moves to felis-daemon, its only remaining runtime consumer,
    and encodes against Grid::mode_snapshot() + kitty_kbd_flags() under the
    same lock InputMsg::Mouse uses. Input admission: the wire caps Key.character at
    MAX_KEY_CHARACTER_BYTES and Key.text at MAX_KEY_TEXT_BYTES (both in
    felis_protocol::limits, UTF-8 bytes, sized for one composed key, which is
    a few codepoints at most; longer text is Paste/KeyBytes territory). A
    frame over either cap is refused at admission with InputOverLimit before
    it reaches the session channel, like an oversized Paste.
    input_reservation_bytes reserves MAX_KEY_REPORT_BYTES, a constant
    derived from those caps as the mode-independent worst case (Kitty CSI u with
    alternate keys and associated text as decimal codepoints is the widest
    form: per codepoint at most 7 digits plus a separator, plus the fixed
    header, modifiers and event fields). A proptest over every mode combination
    (Kitty flag subsets × modifyOtherKeys levels × DECCKM × DECKPAM × win32),
    event kinds and locations, and capped inputs asserts
    encode(...).len() <= MAX_KEY_REPORT_BYTES. The permit is carried through
    encode and write_pty_reserved, so the roster-query barrier sessions send
    relies on still covers structured keys; a saturated-budget test proves Key
    then Ops::List cannot acknowledge dropped input. KeyBytes stays for --raw,
    paste, IME commit (already raw today), send_string. Keymap dispatch is
    unaffected: it resolves on the logical chord before any encoding. sessions send --key drops its rehydrate-drain of mode frames and sends Key. Repeat
    is new behavior (Kitty event type 2), not a move. The standing rejection in
    docs/explanation/architecture/control-surfaces.md ("no daemon Key message")
    is rewritten in place with the stale-mode race as the reason; input.md and
    key-encoding.md follow.
Direction agreed 2026-09-21 after three `sol` design-review rounds (final verdict PASS). Full plan and batch order: see the comment on #12. - **#309** — add a structured `InputMsg::Key` (named key or character, text, a 4-bit modifier set including Super, press/repeat/release, location). Wire vocabulary (`NamedKey`, `KeyLocation`, key modifiers) moves to `felis-protocol`; the encoder (`key_encode.rs` with its 102 unit tests and 11 proptests) moves to `felis-daemon`, its only remaining runtime consumer, and encodes against `Grid::mode_snapshot()` + `kitty_kbd_flags()` under the same lock `InputMsg::Mouse` uses. Input admission: the wire caps `Key.character` at `MAX_KEY_CHARACTER_BYTES` and `Key.text` at `MAX_KEY_TEXT_BYTES` (both in `felis_protocol::limits`, UTF-8 bytes, sized for one composed key, which is a few codepoints at most; longer text is `Paste`/`KeyBytes` territory). A frame over either cap is refused at admission with `InputOverLimit` before it reaches the session channel, like an oversized `Paste`. `input_reservation_bytes` reserves `MAX_KEY_REPORT_BYTES`, a constant derived from those caps as the mode-independent worst case (Kitty CSI u with alternate keys and associated text as decimal codepoints is the widest form: per codepoint at most 7 digits plus a separator, plus the fixed header, modifiers and event fields). A proptest over every mode combination (Kitty flag subsets × modifyOtherKeys levels × DECCKM × DECKPAM × win32), event kinds and locations, and capped inputs asserts `encode(...).len() <= MAX_KEY_REPORT_BYTES`. The permit is carried through encode and `write_pty_reserved`, so the roster-query barrier `sessions send` relies on still covers structured keys; a saturated-budget test proves `Key` then `Ops::List` cannot acknowledge dropped input. `KeyBytes` stays for `--raw`, paste, IME commit (already raw today), `send_string`. Keymap dispatch is unaffected: it resolves on the logical chord before any encoding. `sessions send --key` drops its rehydrate-drain of mode frames and sends `Key`. Repeat is new behavior (Kitty event type 2), not a move. The standing rejection in `docs/explanation/architecture/control-surfaces.md` ("no daemon Key message") is rewritten in place with the stale-mode race as the reason; `input.md` and `key-encoding.md` follow.
Commenting is not possible because the repository is archived.
No description provided.