Skip to content

feat(terminal): newline chord as registry data, plus a Key tester in Settings - #522

Open
opticon454 wants to merge 2 commits into
Ark0N:masterfrom
opticon454:fix/newline-sequence-capability
Open

opticon454 wants to merge 2 commits into
Ark0N:masterfrom
opticon454:fix/newline-sequence-capability

Conversation

@opticon454

@opticon454 opticon454 commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

What

Moves "which bytes does Shift+Enter type into this CLI's pane" out of the send-key route and into the CLI registry, and adds a Key tester to Settings.

  • capabilities.newline ('line-feed' | 'esc-enter', absent = line feed). POST /api/sessions/:id/send-key reads it for S-Enter instead of hardcoding 0x0a; C-Enter is always a line feed. No stock CLI declares it, so behaviour is unchanged for every CLI today: it is the place to put a CLI whose composer ignores a bare line feed (or a user clis.json override) instead of a mode === 'x' check in the route. It is an enum, not a byte string, so config never carries bytes that get typed into a pane.
  • Key tester (Settings → Terminal & Input): click the box, press keys, see what this browser reports for keydown/keypress/keyup (key, code, modifiers, charCode). Read-only, sends nothing to a session. It does not preventDefault on keydown (that would suppress the keypress that mattered in fix(terminal): Shift+Enter no longer submits after inserting a newline #520), and keys pressed in it do not trigger app shortcuts: the global shortcut dispatcher skips events aimed at a [data-raw-keys] element.

Why

Shift+Enter handling has three layers (browser handler, send-key bytes, the CLI's own composer). Making the middle one data keeps the registry's no-id-branching rule true, and the Key tester makes the next "Shift+Enter does X on my device" report diagnosable in seconds. Independent of #520 (the keypress fix); they touch different files apart from the test list.

Tests

  • test/cli-newline-capability.test.ts: no stock CLI declares a chord; schema accepts the two values and rejects free-form byte strings; optional.
  • test/routes/session-routes.test.ts: bytes sent to tmux per mode (line feed by default, including codex; ESC CR only for a CLI that declares it, exercised via a clis.json override; Ctrl+Enter always a line feed; unknown mode falls back to a line feed).
  • test/key-tester.browser.test.ts (real Chromium): Shift+Enter shows the keypress with charCode=13, Ctrl+Enter shows none, the field never types, and Ctrl+W / Ctrl+L / Escape / Alt+1 / Ctrl+K pressed in it fire no app shortcut and leave Settings open (verified to fail without the dispatcher guard), while Escape elsewhere still reaches closeAllPanels.
  • Full CI gate: typecheck, lint, format, public assets, catalogue and 8525 tests pass.

🤖 Generated with Claude Code

…Settings

capabilities.newline replaces choosing the Shift+Enter bytes in the send-key
route. Key tester shows the keydown/keypress/keyup a browser reports.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@Ark0N

Ark0N commented Oct 2, 2026

Copy link
Copy Markdown
Owner

Thanks @opticon454, this moves the Shift+Enter byte choice of the send-key route into the CLI registry as an enum and adds a Key tester to Settings. The registry half is clean: enum-only config, resolved at call time, no id branching, and the route tests pin the bytes per mode. The full gate is green here too (8523 tests).

A few things before it goes in:

  1. Key tester lets app shortcuts fire (src/web/public/settings-ui.js:1122, root cause src/web/public/app.js:1244). The shortcut dispatcher is a document capture-phase keydown listener with no focus-target check, so it runs before the tester's handler. With focus in #keyTesterInput in Chromium, Ctrl+W called killActiveSession() (in the app that closes the active session and kills its tmux, no confirm), Ctrl+L cleared the terminal, and Escape closed Settings. Alt+1..9, Ctrl+Tab and the other registry chords go through the same path. On macOS and in an installed PWA window Ctrl+W reaches the page, so probing "what does Ctrl+W send" loses the session, while the row says nothing is sent to a session. Please make the dispatcher return early for events aimed at the tester, as its first statement (for example if (e.target && e.target.id === 'keyTesterInput') return;, or a data-raw-keys marker checked with closest() so the shortcut-rebind input can opt in later). Keep not calling preventDefault, so keypress still fires. Then extend test/key-tester.browser.test.ts: stub app.killActiveSession and app.clearTerminal, press Ctrl+W, Ctrl+L and Escape in the field, and assert nothing was called and Settings is still open.

  2. Codex switch to ESC CR (src/config/cli-registry/stock.ts:623). The bytes are typed by tmux on the server, so the browser's OS cannot change what Codex reads. I tested codex-cli 0.147.0 in a tmux 3.4 pane: send-keys -H 0a inserts a newline, and 1b 0d does too. fix(codex): make Shift+Enter insert a newline in Windows terminals #495's symptom matches the keypress leak you fixed in fix(terminal): Shift+Enter no longer submits after inserting a newline #520. Unless you have a Codex version or setup where the line feed really fails, please leave Codex on the line feed (drop the stock.ts line and have test/cli-newline-capability.test.ts assert that no stock CLI declares a chord). If you do have one, keep esc-enter and change the comment, docs/cli-registry.md:123 and the changeset to describe that case.

  3. Port (test/key-tester.browser.test.ts:6): 3197 is already used by test/base-path-server.test.ts:11. Please pick an unused one.

  4. Ctrl+Enter test (test/key-tester.browser.test.ts:40): the name says "without a keypress" but the test does not check it. Chromium emits none, so expect(text).not.toMatch(/keypress/) passes.

Smaller things, fine in the same push:

  • src/web/routes/session-routes.ts:2096 and docs/architecture-invariants.md (lines 194 and 783) still say send-key always injects 0x0a. Point them at capabilities.newline if Codex keeps esc-enter.
  • src/web/public/index.html:1834 uses class="set-select" where the other settings text inputs use set-input, and line 1825 has trailing whitespace.
  • config/test-suites.ts:34 conflicts with fix(terminal): Shift+Enter no longer submits after inserting a newline #520 (both PRs insert at the same line). I plan to merge fix(terminal): Shift+Enter no longer submits after inserting a newline #520 first, so this one will need a rebase that keeps both lines. After that, the "used to submit" wording in settings-ui.js:1120 and the test comment at line 35 will be accurate.

Once 1 to 4 are addressed I'll merge it.

…tays on line feed

- app.js: the shortcut dispatcher returns early for events aimed at a data-raw-keys
  field, so Ctrl+W / Ctrl+L / Escape / Alt+1 / Ctrl+K pressed in the Key tester no
  longer kill the session, clear the terminal or close Settings
- stock.ts: drop Codex's esc-enter (a line feed works); no stock CLI declares a chord.
  The esc-enter path is tested through a clis.json override
- tests: unused port (3194), Ctrl+Enter asserts no keypress, shortcut-isolation test
  (verified to fail without the guard)
- docs/comments point at capabilities.newline; set-input class, trailing whitespace

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@opticon454

Copy link
Copy Markdown
Contributor Author

Thanks, all four are fixed in the push above, plus the smaller items. Full gate on the branch: typecheck, lint, format, public assets, catalogue and 8525 tests pass.

  1. Key tester vs app shortcuts. The dispatcher in app.js now returns early, as its first statement, for events aimed at a [data-raw-keys] element (matched with closest()), and the tester input carries that attribute. keydown is still not preventDefaulted, so keypress still fires. test/key-tester.browser.test.ts now stubs killActiveSession, clearTerminal, openCommandPalette and closeAllPanels, presses Ctrl+W, Ctrl+L, Escape, Alt+1 and Ctrl+K in the field, asserts none of them was called, that each chord still shows up in the tester, and that Settings is still open. I also ran it with the guard removed to check the test means something: without the guard killActiveSession and two others fire and the test fails. A second test pins that the guard is scoped, so Escape outside the field still reaches closeAllPanels.
  2. Codex. Dropped. stock.ts no longer declares a chord and test/cli-newline-capability.test.ts asserts no stock CLI does. You are right that tmux types the bytes on the server; I had attributed fix(codex): make Shift+Enter insert a newline in Windows terminals #495 to a byte problem when it matches the keypress leak from fix(terminal): Shift+Enter no longer submits after inserting a newline #520. The esc-enter path is still covered, through a clis.json override of codex in test/routes/session-routes.test.ts (Esc+Enter for Shift+Enter on that CLI only, Ctrl+Enter always a line feed). docs/cli-registry.md and the changeset now describe the field as available for a CLI that needs it, not as something Codex uses.
  3. Port. 3194 (3197 is base-path-server.test.ts; I also checked against the other ports in the tree).
  4. Ctrl+Enter test. Now asserts not.toMatch(/keypress/).

Smaller items: the send-key route comment and the two architecture-invariants.md mentions point at capabilities.newline; the input uses set-input; the trailing whitespace is gone.

On the config/test-suites.ts conflict with #520: agreed that #520 goes first. I have not rebased yet since #520 is not merged; once it is, I will rebase onto master keeping both lines and push.

@opticon454

Copy link
Copy Markdown
Contributor Author

Adding to the config/test-suites.ts note above: I checked my open PRs against each other with trial merges, and #522 overlaps the others too. Textual conflicts only (adjacent insertions), no behavioural interaction:

Nothing to do on this PR now. Once #520 merges (and for whichever of #521/#523 lands before this one) I will rebase onto master keeping both sides, re-run the full gate and push.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants