Repository navigation
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 37.93% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 19 files. (7 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description check The description includes both required sections. It clearly explains the change, behavior, validation results, limitations, and remaining CI requirements. Linked Issues check The description references issue #798and clearly explains how the implementation follows its direction. It also identifies related independent work.Out of Scope Changes check The changes remain focused on shortcut recording, editing, dispatch, documentation, translations, and regression coverage. The description explicitly identifies excluded paging and input-focus work. Title check The title is concise and accurately identifies the main change: recording and previewing custom shortcut bindings.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 37.93% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 19 files. (7 skipped: 7 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
This was referenced Oct 11, 2026
Owner
|
Thanks for this. The maintainer's direction is in #798: the recorder (preview, Save/Cancel, manual editor) is welcome, and modifiers may be free as long as a binding never takes a key the terminal, the agent or the browser needs. Plain Ctrl+letter / Alt+letter and browser-reserved chords should be refused or warned about when recording; Mod+Shift+key stays the safe default, and chords like Cmd+PageUp are fine.\n\nPlease go ahead if you'd like to finish it that way. If there's no update in about two days, we'll carry it over the line from your commits, with credit to you. |
This was referenced Oct 13, 2026
kratam
marked this pull request as ready for review
October 13, 2026 15:11
devswha
added a commit
that referenced
this pull request
Oct 13, 2026
) ## Problem On a phone in input-line mode (the touch default), the whole key bar goes grey and ignores taps while a word is still composing in the input line. With Gboard that is most of the time, because every word composes until a space; with a Korean or Japanese IME it lasts until the syllable or phrase commits. Esc and ^C cannot stop an agent, an arrow cannot answer a menu, Ctrl/Alt/Shift cannot be armed, and the keyboard-mode toggle is dead. ## Cause `PaneTerminal` passed `onComposing={setComposing}` to `TerminalInput`, so the input line's `compositionstart`/`compositionend` wrote the same `composingRef` and `composing` state that guard xterm's deferred IME commit. That flag disables `<KeyBar>` and short-circuits `pressKey`, `toggleModifier` and `toggleDirect`. The guard exists so a key-bar key cannot overtake xterm's pending commit, but the input line's text never goes through xterm or `onData`: it is submitted separately with `socket.submit`. There was nothing to protect, only taps to drop. ## Fix - `PaneTerminal.tsx`: stop wiring `TerminalInput`'s composition into `setComposing`. The xterm host `compositionstart`/`compositionend` listeners (and the pane reset) stay the only writers of `composingRef`/`compositionCommitPendingRef`, so the xterm deferred-commit gate is unchanged. - `TerminalInput.tsx`: remove the now unused `onComposing` prop and its unmount cleanup. The line keeps its own local `composing` ref that guards its Send button and Enter. - `docs/terminal-input.md`: the key bar waits only for a direct-typing (xterm) composition; an input-line composition leaves it live. - Review follow-up (ca26dd6): the direct-typing toggle is the one key that still waits for an input-line composition. Switching mid-word would unmount the line with the IME composition open, and the word could be lost or typed into the pane. `TerminalInput` reports its composition through `onComposing` again. `PaneTerminal` tracks it apart from the xterm composition, `toggleDirect` returns early on it, and `KeyBar` gets `directDisabled`, which greys out only that key. ## Tests - New phone block in `scripts/terminal-input-regression.ts` (390x844, touch, line mode): fill the line, dispatch `compositionstart` on `.terminal-input-text`, assert Esc, ^C and Ctrl are enabled, tap Esc and ^C, expect exactly two `input` frames `\x1b` and `\x03`, and the line's value unchanged. - `bun run check run --build bun scripts/terminal-input-regression.ts`: PASS (all three blocks). With the two source files reverted the new block fails on "Esc stays live while the input line composes". - `bun run typecheck`: clean. `bun run test:unit`: 3069 pass, 0 fail, 8 skip. `scripts/changelog-fold.test.ts`, `scripts/pages-gate.test.ts`, `scripts/conventions.test.ts`: pass. - Not run: `scripts/ui-regression.ts` as a whole (it calls `checkTerminalInput`, so CI runs the new block there too), `check full`/`concurrent`. - The phone regression also checks that `[data-key="direct"]` is disabled during the composition and that a forced tap on it leaves the line, its text and the pane untouched. Before the follow-up fix, that check failed with "the direct-typing toggle waits for the line's composition". ## Notes - No open PR changes this gate (checked the diffs of #936, #946, #943, #822, #812 for `onComposing`/`setComposing`). #936 edits `TerminalInput.tsx` and #943/#946/#950/#951 edit `PaneTerminal.tsx`/`KeyBar.tsx`, so a textual conflict is possible depending on merge order, but none duplicates this fix. - Left out on purpose: no separate gate on the key-bar Enter while the line composes. It sends `\r` to the pane, not the line's text, just as it does after a commit. The direct-typing toggle is gated (see the follow-up in Fix). - The regression uses synthetic composition events, so it certifies the gate, not a physical IME. - Gate added in #334 (c50c881) for both inputs; this keeps it for xterm only. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
devswha
added a commit
that referenced
this pull request
Oct 13, 2026
…appers (#970) ## Problem `server/herdr/client.ts` exported an untyped `herdrRpc(method: string, params: Record<string, unknown>)`. Eleven server files called it directly for four methods (`pane.process_info` x10, `agent.get` x6, `pane.send_text` x2, `pane.send_keys` x1), each with its own inline result type. Nothing stopped `{ target }` reaching `pane.process_info`, which herdr then silently answers for the focused pane (the AGENTS.md rule was prose only). ## Change - `server/herdr/methods.ts`: `HerdrMethods`, a `{ [method]: { params; result } }` map for those four methods. Params and result bodies come from the generated schema types; `PaneProcessInfoParams`, `AgentTarget` and `PaneProcessInfo` are now generator roots (`scripts/generate-protocol-types.ts`, regenerated `shared/herdr-api.generated.ts`). The map itself is hand-written because herdr's schema links a method to its params (`request.oneOf`) but not to its answer: answers are one `ResponseResult.oneOf` told apart only by a `type` tag (`pane_process_info`, `agent_info`, `ok`). - `herdrRpc<M extends keyof HerdrMethods>(method, params: HerdrMethods[M]["params"], …): Promise<HerdrMethods[M]["result"]>` is the first overload. An untyped fallback overload stays for methods the map does not list yet. It refuses a typed method name (`UntypedHerdrMethod<M>` is `never` for one), so `herdrRpc("pane.process_info", { target })` is a compile error. `server/herdr/seam.test.ts` pins that with `@ts-expect-error` and checks the wire params of the new wrappers against a stand-in socket. - New wrappers: `paneProcessInfo(paneId)` returns one shared `ProcessInfo` (= generated `PaneProcessInfo`: shell_pid, foreground_processes with pid/name/argv0/argv) and `agentGet(paneId)` returns `AgentInfo`. The existing `paneSendText` / `paneSendKeys` now go through the typed overload. The wrappers call the exported `herdrRpc` binding, so tests that `spyOn(herdr, "herdrRpc")` keep working. - Migrated (no open PR touches them): `server/pi.ts`, `server/omp.ts`, `server/grok-store.ts`, `server/agy-store.ts`, `server/codex.ts` (x3). Behaviour is unchanged: the same params go on the wire, with the same default timeout and the same `.catch` / `try` at each call site. `codex.ts` keeps its old `argv?: string[]` reading through a commented cast, because the schema allows `argv: null` and that path has never handled null. Changing that would change behaviour, so it is left for a separate fix. - Guard: `server/conventions.test.ts` ("herdr socket") now fails when server code outside `server/herdr/` uses `herdrRpc`. The message names the fix ("add a typed wrapper in server/herdr/client.ts; for pane status use the server/collector.ts listeners") and the allowlist `HERDR_RPC_ALLOWED`. A stale allowlist entry also fails. The `events.subscribe` / `subscribeEvents` rules from #949 were already there; their message now also says where `events.subscribe` lives. - AGENTS.md: the `pane.process_info` line points at `paneProcessInfo` / `HerdrMethods` and names the enforcing test. Only that one line changes; #812, #888 and #894 also edit AGENTS.md. ## Allowlist (`HERDR_RPC_ALLOWED`, migrated in PR B) | File | Open PRs touching it | |------|----------------------| | `server/conversation.ts` | #869 #912 #923 | | `server/gjc-runtime.ts` | #912 | | `server/hermes.ts` | #910 | | `server/omo.ts` | #923 | | `server/shell-agent.ts` | #885 #923 | Built from `gh pr list --state open --limit 100 --json number,files` on 2026-10-14. ## Known limits (closed in PR B) - The export is still named `herdrRpc`. Renaming it to `herdrRpcUnsafe` would touch about 33 test files and 30 `scripts/` harnesses, several of them in open PRs (api.contract, gjc.contract, omo.contract, ui-regression, pending-input…). Tests and `scripts/` QA harnesses are outside this guard's scope: they drive herdr (`pane.report_agent`, `workspace.create`, …) and are not part of the bridge seam. - A call with an explicit result type, `herdrRpc<Inline>("pane.process_info", …)`, still reaches the untyped fallback. That is the form the allowlisted files use, and they may not change here. In server code the source guard closes this gap. The fallback goes away once the allowlist is empty. ## Validation - `bun run check fast`: green. A first run had one failure in `server/portal.test.ts` ("does not treat an arbitrary interpreter as a Portal script": the process argv read raced and returned undefined). This change does not touch that file, it passed alone, and the rerun was fully green. - `bun run test:integration`: 455 pass, 0 fail, 14 skip in 38 files (includes the omp, codex, codex-binding, pi, gjc, omo and hermes contract tests). - I checked by hand that the guard fails on a new `herdr.herdrRpc` use in `server/pi.ts`. ## Follow-up: PR B Once #869, #885, #910, #912 and #923 land: migrate the five allowlisted files to `paneProcessInfo` / `agentGet` / `paneSendText` / `paneSendKeys`, empty `HERDR_RPC_ALLOWED`, drop the untyped fallback overload (or rename it `herdrRpcUnsafe` for tests and scripts), and consider handling `argv: null` in `codex.ts`. ## Review Fixed in 290d5f1: - **Union of typed methods (medium, part of finding 1).** The typed overload takes `SingleHerdrMethod<M>`, so `herdrRpc(cond ? "pane.process_info" as const : "agent.get" as const, { target })` no longer compiles. `seam.test.ts` pins this with a `@ts-expect-error` line. Checked with tsc: the inferred `herdrRpc("pane.process_info", { pane_id })` still returns the typed result. - **Allowlist missed #928 and #859 (medium).** `conventions.test.ts` adds `HERDR_RPC_PENDING` with `server/gjc-status.ts` (#928) and `server/index.ts` (#859). Those PRs add a `pane.process_info` call, but neither file calls herdrRpc on main yet, so the "no longer calls" check skips them. It would be better for both PRs to switch to `paneProcessInfo(paneId)`, which is a one-line change in each. After that, these entries come out. - **Docs claimed more than the PR delivers (low).** AGENTS.md now says the guard enforces the rule, not the compiler, and that the allowlisted and pending files still call herdrRpc. The changelog fragment now says "no longer compiles without an explicit result type". "PR B" is replaced with "a follow-up PR that moves these files to the wrappers once their open PRs land". The fallback docblock in `client.ts` and the `UntypedHerdrMethod` comment in `methods.ts` list the holes. Deferred to the follow-up PR that drains `HERDR_RPC_ALLOWED`: - **Explicit result type and widened string (rest of finding 1).** `herdrRpc<T>("pane.process_info", { target })` and `herdrRpc(m as string, { target })` still compile. With an explicit T, the fallback's `M` stays at `string`, and that cannot be closed while the fallback shares the name. Closing it means splitting the fallback out as `herdrRpcUntyped` (or `herdrRpcUnsafe`) and moving about 400 test and script call sites onto it. It also means converting the `herdrRpc<T>("pane.process_info" | "agent.get", …)` calls in the five allowlisted files, which conflicts with their open PRs. `seam.test.ts` keeps both forms as compiling lines with a comment, so the follow-up can turn them into `@ts-expect-error`. - **Error message.** Two overloads are still present, so tsc reports a bad inferred call such as `{ target }` to pane.process_info as `not assignable to parameter of type 'never'` instead of naming the wrong param. That message only gets better with the split. - **Stale pending entries.** Nothing removes a `HERDR_RPC_PENDING` entry on its own. If #928 or #859 switches to `paneProcessInfo` or closes, its entry stays until someone takes it out by hand. - **Tell the open PRs.** #912, #923, #928, #859 and #910 call `herdrRpc<T>("pane.process_info", …)`. They should hear about the switch to `paneProcessInfo` before the split lands. I have not commented on those PRs. Checks: `bun run check fast` passed (typecheck, build, 3452 unit tests). `bun run test:integration` passed (455 pass, 0 fail). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
devswha
added a commit
that referenced
this pull request
Oct 13, 2026
## Problem
On a phone in input-line mode, tapping Send while the keyboard is still
composing the last word does nothing: no send, no note, and tapping
again does nothing either. It works only after typing a space or the
keyboard's own Return. Gboard composes every word (any language), and
Korean/Japanese/Chinese IMEs keep the last syllable open, so this hits
the default touch input path.
## Cause
`TerminalInput.send()` returned early on `composing.current` with no
feedback and no deferred send. The Send button's `onPointerDown`
`preventDefault` (which keeps the soft keyboard up) also keeps focus and
the selection in the textarea, so nothing in the tap ends the
composition and the guard never clears. Gboard commits on a space,
Return, or a focus/selection change, and this tap causes none of those.
The synthetic regression pinned the no-op ("unfinished composition must
not submit via button"), and the native Gboard regression always typed a
trailing space, so the real case was untested.
## Fix
- A Send during composition marks a pending send, shows "Finishing the
word…", and then **finishes the word itself**: `blur()` + `focus({
preventScroll: true })` on the line inside the click. Chrome ends the
composition on the focus change and fires `compositionend`
synchronously; the refocus inside the same gesture keeps the keyboard
up. Probed in headless Chrome with a real CDP composition
(`Input.imeSetComposition`): without the blur no `compositionend` ever
fires; with it, `compositionend` fires before `blur`, and focus is back
on the line afterwards.
- On `compositionend` with a pending send, `send()` runs on the next
tick (`setTimeout(0)`, so WebKit's committing input event has landed).
- `send()` reads the line from the draft store
(`readTerminalDraft(owner)`) instead of the last render's `text`, so a
commit that has not re-rendered yet (the forced commit, or WebKit's late
input) is not cut short.
- Fallback: if no commit arrives within 300 ms, the tap is withdrawn
with "Finish the word, then send.", so the tap is never silent and a
later commit never sends on its own. The timer is cleared on unmount.
- A deferred send that finds the terminal disconnected says "Not sent:
the terminal is disconnected." instead of leaving "Finishing the word…"
up; if another send is already in flight it clears the note.
- The Return key path is unchanged (IME Enter still never sends).
- New strings translated in ko/ja/zh (and tr); docs/terminal-input.md
updated; changelog fragment added.
## Tests
`scripts/terminal-input-regression.ts` (also run by
`scripts/ui-regression.ts`):
- Real composition: CDP `Input.imeSetComposition` on the line, click
Send, expect the committed line sent at once and focus still on the
line. Fails without the forced commit (`actual: 0, expected: 1`, checked
by running the case against the previous `TerminalInput.tsx`).
- Synthetic commit within the window: `compositionend` followed in the
same task by the committing input; the committed line is sent once.
- Disconnect during the wait: the socket is closed and refused, then
`compositionend`; the note reads "Not sent: the terminal is
disconnected.". Fails without the deferred-note change (the note stays
"Finishing the word…").
- Withdrawn tap: after "Finish the word, then send.", a `compositionend`
and then a 400 ms wait (past the deferred tick and a whole commit
window) before asserting nothing was sent; `compositionstart` is
dispatched only after that check.
Runs:
- `bun run typecheck`: pass. `bun run test:unit`: 3069 pass, 0 fail.
`bun run build`: pass.
- `bun run check run bun scripts/terminal-input-regression.ts`: pass 3
times in a row.
- `bun run check run bun scripts/ui-regression.ts`: pass.
- `scripts/android-ime-regression.ts` (native Gboard step: types
`gksrmf` with no trailing space, taps Send, expects `한글\r` in the PTY),
on an Android 16 emulator with Chrome 133.0.6943.137 and Gboard
15.1.08.726012951-preload-x86_64 (Korean 2-bulsik):
- PR head (21eb156): **pass 4/4**. After the send, focus is still on
the input line and the keyboard stays up (`lineFocused: true`, visual
viewport 471 px, screenshot taken).
- main (control): **fail 2/2** at the send-during-composition step (`PTY
did not receive expected input`: the tap sends nothing).
- Merged head after bringing main in (16bd27f): **pass 2/2**.
- Merged head: `bun run check fast` pass (3451 unit tests, 0 fail);
`scripts/terminal-input-regression.ts` and `scripts/ui-regression.ts`
pass.
## Notes
- **Device behaviour observed on an emulator.** The forced commit
(Chrome/Blink finishing the composition on a focus change) holds on
Chrome Android with native Gboard: see the emulator runs above. The
Android step still taps Send through Playwright `tap()`; the commit is
forced inside the click handler whatever started the tap, so a native
tap takes the same path. On iOS the blur/refocus inside the tap also
ends a WebKit composition, but it may make the keyboard flicker. That is
not observed yet.
- Caveat for future adb-coordinate taps: with a Chrome command-line flag
file on a non-rooted device, Chrome shows the `CommandLineOnNonRooted`
("unsupported command-line flag") banner at the top, which shifts the
page down. Screen coordinates mapped from CSS positions must account for
it (or dismiss it first). The script here does not tap by adb
coordinates, so these runs are unaffected.
- Overlap: #937 (chat Send keeps focus) adds the same `onPointerDown
preventDefault` to the chat Composer, and `Composer.send()` bails on
`composingRef` the same way, so chat Send would get the same silent
no-op on Gboard. Not changed here. It needs the same treatment as a
follow-up or within #937.
- Overlap: #936 (Speaches dictation) rewrites `TerminalInput.send()` and
adds its own `sendRef`. A textual merge conflict is likely, and
whichever merges second must keep both behaviours.
- Overlap: #822 and #812 edit `scripts/terminal-input-regression.ts`
near the changed block.
## Review
Five low findings from review, all fixed in 6e1b494 (test tightened in
the next commit):
- **Empty commit pressed a bare Enter.** A deferred text Send whose word
committed empty (preedit deleted or cancelled within the wait) took the
empty-line branch and sent a lone CR. The tap now records whether the
line held text; such a Send sends nothing. Emptying the line also
withdraws a waiting Send.
- **Transient `role="alert"` on every mid-word Send.** "Finishing the
word…" was set before the blur, so it mounted for one macrotask even
when Gboard committed inside the tap. The note and the timer are now
armed only if the composition is still open after the blur and refocus.
- **Docs contradiction.** `docs/terminal-input.md` said both Send
buttons preserve an active composition; now only the chat Send does, and
the input-line behaviour (fixed deadline, drop, empty commit) is
described.
- **Deadline was not absolute.** A deferred Send that found a new
composition armed a fresh 300 ms wait. The deadline is now fixed at the
tap; a later re-check waits only for what is left, and past it the tap
is withdrawn with "Finish the word, then send."
- **Pending Send survived a quick reconnect.** The first reconnect (250
ms) fits in the window. A drop now withdraws a waiting Send at once with
"Not sent: the terminal is disconnected.", and a deferred send runs only
while its attempt is still current.
Regression tests in `scripts/terminal-input-regression.ts`: no
`.terminal-input-note` node is ever added when a real CDP composition
commits inside the tap (MutationObserver; fails on 16bd27f), an empty
commit after a text Send sends nothing, a chain of compositions past the
deadline sends nothing and leaves "Finish the word, then send.", and a
drop plus reconnect withdraws the tap. Each of these fails when the
component is reverted to 16bd27f (checked one at a time, earlier
assertions neutralized).
Verified on 6e1b494 and its follow-up test commit: `bun run check fast`
pass (3451 unit, 0 fail); `scripts/ui-regression.ts` (includes the
terminal-input regression) pass; native
`scripts/android-ime-regression.ts` on the Android 16 emulator (Chrome
133.0.6943.137, Gboard 15.1.08.726012951) **pass 1/1**, send during
composition delivers `한글\r`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Owner
|
Thanks @kratam for this and for the iPad use case behind it. Closing as superseded: #951 brought a shortcut recorder to main (Settings → Shortcuts now has Record with a live preview, plus search), which covers the core of this PR. If exact Ctrl/Alt modifier chords or a manual key editor for touch-only devices still matter to you, please open an issue and we can scope it on top of what's on main. |
devswha
added a commit
that referenced
this pull request
Oct 15, 2026
> **Maintainer update (0be3413):** the Automatic chat focus setting was dropped; this PR now adds only the Mod+Shift+E "Focus pane input" shortcut. The description below is the original. ## Change Closes #818. On an iPad with an external keyboard, selecting a pane still requires a tap before typing into chat: the current coarse-pointer check suppresses composer autofocus. Add **Automatic chat focus — Auto / On / Off** to Settings → Chat. Auto preserves the existing behavior, On enables autofocus on touch devices too, and Off lets users keep keyboard focus where it is. Add **Mod+Shift+E — Focus pane input**, configurable or disableable through the existing Shortcuts settings. It focuses the selected pane's chat composer, terminal line input, direct xterm input, or an already-visible masked input. The focus happens synchronously in the keyboard event, preserves drafts and selections, respects modal/observe/presentation guards, and neither sends input nor takes over an attached terminal. This is an independent branch from `main`; it does not depend on the paging PR #811 or recorder proposal #812. It follows the current Mod+Shift policy and leaves the existing native text-selection shortcuts intact. Includes Korean, Japanese, Chinese and Turkish strings, guide/design documentation, and a changelog fragment without a version bump. Owned fictional demo screenshots (1024px touch viewport and 320px phone viewport):   ## Validation Updated against upstream `main` (`4ae6a25e1423b99417eb7f7bda24b76231b60455`) on 2026-10-13; head `9f77bd67a37c09df4a8e8e23c1c2d18accf8ee02`. Focus now uses the upstream pane controls for the selected pane of the active split half. The setting remains device-local and is not added to the telemetry schema. - `UNIT_TEST_WORKERS=4 bun run check fast` passed at code commit `de989f4e7463ad0a2755fc4d33d4944f0f3c1fa6`: generated types, typecheck, production build, 3310 unit tests passed, 0 failed, 50 skipped. Workflow syntax validation skips locally because actionlint is unavailable. - The final follow-up only restores upstream CHANGELOG.md and moves the PR note into `changelog.d/pane-input-focus.md`; all 12 changelog tests passed. - 107 focused settings/shortcuts/i18n/browser-runner tests passed. - `bun run check run --build bun scripts/pane-input-focus-demo-regression.ts` passed at the exact final head with unchanged-source verification: all six desktop/touch × Auto/On/Off configurations, draft/selection preservation, keyboard pane selection, modal and IME guards, line/direct focus, shortcut customization/disable and reload. Touch emulation does not establish actual iPad Safari keyboard behavior. - Independent read-only review found no blocking regression. - Full integration/browser lanes and native Windows checks were not run locally; maintainer CI and review remain required. --------- Co-authored-by: Hako <devswha@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
devswha
added a commit
that referenced
this pull request
Oct 15, 2026
## Change Follows the direction in #798 and the note on #812: scoped on top of the recorder from #951. - **Exact modifiers in Record.** A chord other than Mod+Shift is stored as pressed (`meta+PageUp`, `alt+meta+b`, `ctrl+k`) and shown that way in the row and the palette. A bare stored key still means Mod+Shift, so existing bindings are unchanged. - **Risky chords**, as discussed in #798: - refused: chords the browser, the OS or text editing keeps (Mod+W/T/N, Mod+A/C/V/X/Z, ⌘+Q/H/M and ⌘+\` on a Mac, Ctrl+PageUp/PageDown, macOS Ctrl+arrows), a non-Mac Meta chord, and a key without Ctrl, Alt or ⌘; - plain Ctrl/Alt (no ⌘): the row warns that the terminal uses it and saves only after **Save anyway** (Cancel keeps the old binding). This covers Ctrl+K for the palette; - a few ⌘ chords the browser commonly uses get a note, but are saved. - **More keys for Record:** Home, End and the punctuation keys. Punctuation is matched by key position (`KeyboardEvent.code`), so it works on any layout: on a Hungarian keyboard `É` is the `;` key. Letters still match the character typed, so AZERTY keeps its M. Where Chromium exposes the layout map, the row shows the user's own character. - **The row's dropdown is unchanged.** Home, End and punctuation are only available through Record, since a US character in a list would point to a different key on most layouts. A recorded one is listed as itself. Commits are in order and each builds on the one before; the PR can be squashed. ## Validation - `bun run check fast`: generated types, typecheck, build pass. Unit tests: 3748 pass; `server/portal.test.ts` failed 2 timing tests in the parallel run and passes alone (31/0). This PR does not touch `server/`. - New unit tests in `src/lib/shortcuts.test.ts` and `src/lib/shortcutRecorder.test.ts`: exact chord storage and matching, refusals and risks per platform, macOS Option chords, Home/End, position-based punctuation (Hungarian, German, Russian; AZERTY M stays M), the dropdown key list. - New browser regression `scripts/shortcut-exact-modifiers-demo-regression.ts` (added to `scripts/ci-browser.sh`), on the demo: a safe exact chord is saved and dispatched, Ctrl+K needs Save anyway and Cancel saves nothing, refusals, End via Record shown in the dropdown, a Hungarian É bound by position, the default released after rebinding, from the message box and after a reload. PASS. - Used daily on an iPad with a Hungarian hardware keyboard. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01VoME51Ex6avEzqEBmkYzGd --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Hako <devswha@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change
On an iPad with an external keyboard, selecting the final key from an alphabet dropdown misses useful characters and prevents choosing combinations such as Cmd+PageUp without Shift. Add a focusable recorder with a candidate preview and explicit Save/Cancel, plus a manual key/modifier editor for touch devices without a hardware keyboard.
Following the maintainer's direction in #798, allow exact Ctrl/Meta/Alt/Shift modifiers while retaining Mod+Shift defaults and existing string overrides. Terminal/text-editing combinations (including plain Ctrl+letter and Alt+letter) and known browser/OS reservations show warnings before the explicit Save; no binding is written merely by recording it. Cmd+PageUp is supported. Exact duplicate app bindings, including fixed hold-to-dictate, block Save.
An unchanged manual Save preserves legacy matching and default aliases; new captured bindings use exact modifiers. The updated branch preserves upstream shortcut search, paging controls, split-layout behavior and shared settings, with four translated dictionaries.
Recording handles printable Unicode and named keys, IME/modifier guards, conflicts (including the fixed dictation chord), disable/reset, focus return and reload. The editor owns its keys, and matched app shortcuts stop propagation before composer or terminal input. Unsafe text/terminal overrides and known browser reservations get warnings. The manual editor accepts unambiguous base Latin letters, digits, comma and named keys; physical-layout-dependent entries require hardware recording. System/browser-intercepted chords cannot be captured; the reported iPad Cmd+Shift+Ú/Ű combinations produced no letter events in either focused-button or input diagnostics.
Refs #798. This branch starts independently from main; paging and input-focus features are outside this contribution.
Validation
Updated against upstream
main(4ae6a25e1423b99417eb7f7bda24b76231b60455) on 2026-10-13; headf77d1f6d44d1f9a00c61c28fa87fca03610061ab. The changelog uses the current fragment convention. This PR remains independent of #822 and #917.UNIT_TEST_WORKERS=4 bun run check fastpassed on the committed head: generated types, typecheck, production build and unit suite (3320 pass, 0 fail, 50 skip). Workflow syntax validation skips on macOS because actionlint is unavailable.shortcut-recorder-demo-regression.tspassed on desktop and a 320px touch viewport: preview/Save/Cancel, Ctrl/Alt-letter and Cmd+W/T/N/Q warnings before Save, no automatic persistence, retained upstream shortcut search, legacy bindings, physical aliases, IME, exact dispatch, composer isolation and reload.bun run check run --build bun scripts/terminal-input-regression.tspassed on the committed head against two owned test panes: real xterm composition, direct/line input modes, drafts/reload, IME, late acknowledgements, shortcut reassignment and explicit sending. The run used the installed lockfile Chromium via CHROME_PATH.Owned fictional demo screenshot: