Repository navigation
feat(keyboard): focus pane input with Mod+Shift+E - #822
Merged
Merged
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 18 files. (4 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check The PR meets the coding requirements in directly linked issue [ #818].chatAutoFocusimplements Auto, On, and Off modes with device-local storage and sanitization, andPaneTerminalapplies the sett…Out of Scope Changes check The changes stay within [ #818]. Settings, shortcut handling, input-focus behavior, translations, documentation, changelog, and regression coverage directly support the linked issue. CI registration ch…Title check The title clearly describes the new Mod+Shift+E pane-input focus shortcut. It omits the automatic chat focus setting, but it still identifies a real and significant part of the changes. Description check The description includes the required Change and Validation sections, explains the intended behavior, lists test results, and notes remaining CI limitations. It contains a contradictory maintainer not…
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 18 files. (4 skipped: 4 unsupported.)
✨ 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 13, 2026
# Conflicts: # CHANGELOG.md # DESIGN.md # docs/guide.md # scripts/ci-browser.sh # scripts/ci-browser.test.ts # src/App.tsx # src/components/PaneTerminal.tsx # src/lib/actions.ts # src/lib/i18n.ja.ts # src/lib/i18n.ko.ts # src/lib/i18n.zh.ts # src/lib/settings.test.ts # src/lib/shortcutBindings.ts # src/lib/shortcuts.test.ts # src/lib/shortcuts.ts
devswha
added a commit
that referenced
this pull request
Oct 13, 2026
## Problem A render error outside the few places that catch one (chat turns, the chat lens, the file viewer, the release notes) unmounted the whole React tree and left an empty page. In an installed home-screen app on iOS or Android there is no browser reload button and no pull-to-refresh on the terminal, so the only way back was to force-quit the app. Nothing was logged under a name a bug report could point to. ## Cause `src/main.tsx` rendered `<SettingsProvider><App/></SettingsProvider>` straight into `createRoot` with no boundary. `RenderBoundary` was used only around chat turns, the chat lens, FileViewer and the release notes. App, the sidebar, the tab strip, the pane canvas, the terminal's own JSX, Settings and the dialogs had no boundary, and React 18 unmounts the root on an uncaught render error. ## Fix - `main.tsx` wraps the whole tree, `SettingsProvider` included, in `RenderBoundary`. Its fallback, `AppCrash`, uses the module-level `t` and no context, because a context may be what threw. It offers **Reload app**, **Copy details** (via `lib/clipboard.ts` `copyText`, which falls back to selecting the text on a plain-http LAN address) and the error text in a fold. - The sidebar (`MachineSidebar`), each pane slot (tab strip plus canvas, reset when another pane is picked) and the Settings dialog get boundaries of their own, so one broken region leaves the rest usable. Settings' fallback is a small dialog with Close that keeps what Settings gives a modal: the focus moves to Close and Tab stays inside (`useFocusTrap`), Escape closes it, and on a phone the sheet's swipe down closes it (`useSheetSwipe`). - When the active pane slot falls back to its notice, the notice reports the connection lost (`RegionCrash` `onShown`), since the unmounted terminal cannot; the header no longer claims live with no terminal attached. - `RenderBoundary` now passes the caught error to its fallback as a second argument. Existing callers are unchanged. - `window` `error` and `unhandledrejection` are logged to the console with a `herdr-web-ui:` prefix. Console only, no telemetry. - Test hook: `src/lib/crashProbe.ts` (`?crash=root|sidebar|pane|settings`) throws while rendering only when the build-time constant `__HERDR_TEST_HOOKS__` is true. `vite.config.ts` sets it from `HERDR_TEST_HOOKS=1`, which only `scripts/demo-build.ts` passes. `bun run build` and the published demo (`build-site.ts`) build with it off, so the check compiles out of production. - New strings have entries in ko, ja, zh and tr. Changelog fragment `changelog.d/render-error-boundaries.md`. `src/AGENTS.md` and `scripts/AGENTS.md` each get one line about the boundaries and the hook. ## Tests - New browser check `scripts/render-crash-demo-regression.ts`, registered in `ci-browser.sh` (lane count 45 to 46 in `ci-browser.test.ts`). On the demo build it checks five cases: - Without `crash`, no notice appears and there are no page errors. - `root`: AppCrash shows; Copy details puts the error on the clipboard; Reload app reloads the page. - `sidebar` and `pane`: only that region shows its notice, the rest is drawn, and Try again recovers once the region stops throwing. - `settings`: the dialog shows its notice, the app is drawn behind it, the focus is inside it and Tab/Shift+Tab stay there, Escape dismisses it, and in a second opening Close does. - A pane that throws after its terminal was live (switched to with `crash=pane` set): the header goes from live to reconnecting. - On a 390px touch phone (`isMobile`, `hasTouch`): `root` with the clipboard API absent and the copy command refused, where Copy details opens the details with the error text selected and Reload app is on screen; `pane`, where the workspace drawer still opens and Try again recovers. - `bun run check run bun scripts/render-crash-demo-regression.ts`: PASS. - Negative runs: root boundary removed from `main.tsx` FAILS (timeout waiting for the AppCrash heading); the `App.tsx` connection report reverted FAILS (timeout waiting for `.conn-reconnecting`); the Settings fallback's focus trap/Escape reverted FAILS (focus never enters the dialog). - `bun run typecheck`: pass. The new script, which is outside `tsconfig.tools.json`, was also typechecked with a temporary config: pass. - `bun run test:unit`: 3069 pass, 0 fail, 8 skip (rerun after the review fixes). - `bun run build`, then grep: `crash probe` is absent from `dist/`. - Not run: `check full` and `check concurrent`. The phone sheet swipe on the Settings fallback uses the same `useSheetSwipe` hook as Settings and is not exercised by the script. ## Notes - Overlaps with open PRs: - #951 touches App.tsx, SettingsDialog.tsx, src/AGENTS.md, scripts/AGENTS.md, ci-browser.sh, ci-browser.test.ts and demo-build.ts. It is not a duplicate: it adds `RenderBoundary` only inside chat and diff rendering. - #917 and #822 also add browser scripts, so the `toHaveLength` lane count in `ci-browser.test.ts` will conflict with whichever merges second. - #909, #890, #917 and #822 touch App.tsx near the sidebar and slot JSX. - The `crashProbe` check compiles out, but the `<CrashProbe>` elements stay in production as components that render null. - The pane boundary covers a split half (tab strip plus canvas), not each herdr pane, so a sibling in a herdr split inside the same half goes down with it. A per-pane boundary in `PaneCanvas.renderPane` would be the finer follow-up. - The browser console already prints uncaught errors. The listeners add a greppable `herdr-web-ui:` prefix. - The lane's shared demo build now has the inert probe compiled in, so every demo script runs on that build. - Left out: reporting crashes to telemetry, and other dialogs and the palette, which are still caught by the root boundary only. ## Merge-session review Fixed in 5ff418f: - The Settings crash fallback ignored `overPreview`: with a file preview open (Settings opened by its shortcut), the notice was drawn under the preview while its focus trap held the keyboard. `SettingsCrash` now takes `overPreview` and uses the `settings-over-preview` layer, as Settings does. - The fallback recorded no Settings entry in the history: if Settings threw on its first render, the system Back button left the page or the installed app; if it threw after a phone opened a page, the first Back did nothing visible. `SettingsCrash` now records one Settings level (`recordSettings([{ page: null, keyBar: false }])`), so one Back closes it. - `render-crash-demo-regression` adds three cases, each confirmed to fail without its fix: the hit test on Close over a file preview, Back on a desktop and a phone after a first-render crash, and one Back after a phone crash on an opened page. Deferred: none. Checks: `bun run check fast` passed; `render-crash-demo-regression` passed (run directly, since `check run` admission was busy). 🤖 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
…PCs (#958) ## Problem - On Android Chrome or a desktop browser, tapping an agent alert could bring forward a Markdown reader tab (`/?read=…`) or a raw file tab opened earlier from the file viewer. The pane named in the alert never opened. - An alert (still in the notification center) or a remembered selection (reload, new window) that names a PC removed since left an empty pane area with no explanation. An alert for a pane that has closed switched to herdr's focused pane without saying anything. - Review follow-ups (second commit): the worker's second delivery of one tap could strand a closed pane's lookup; a tab resumed with an old roster dropped a tap for a PC set up since; a just-added PC could be reported missing by a roster rebuilt from a pane status; shipped files such as `/social-preview.png` still counted as app windows; and a closed pane's tap always said its workspace had closed. ## Cause - `public/sw.js` `notificationclick` picked `windows.find(focused) || windows[0]` without checking `client.url`. `matchAll` lists the most recently focused window first, so the reader tab got the `select-pane` message. It renders `FileReader`, not `App`, so nothing consumed the message, and `focus()` succeeded, so `openWindow` never ran. - `src/App.tsx` handled "PC missing" and "pane closed" only for address links (`pendingLink`, `resolveLink`, `setHeaderNote`). Notification targets went straight to `selectTarget`, which also cleared `pendingLink`. That includes the worker's `selectLatest` repeat on a cold start, so a cold start's link could lose its machine-missing handling as well. A stored selection never set `pendingLink`. - Review follow-ups: the link resolver only re-ran on snapshot or selection changes, and dropped a lookup whose `pendingLink` object had been replaced, so the worker's second identical delivery stranded it. The tap handler decided "PC missing" from whatever roster the tab held, and the selection check ran on any `machines` change, including lists rebuilt from a pane status before `load()` brought a just-added PC. The alert carried no workspace, so `resolveLink` could only answer workspace-closed for a closed pane. ## Fix - `sw.js`: new `isAppWindow(url)`. Only the app shell's paths count: it rejects `?read=` readers, `/api/` responses (raw files and HTML pages), `/ws`, and any path whose last part has an extension other than `index.html` (icons, `/social-preview.png`, the manifest, `/sw.js`, notices). `notificationclick` only considers app windows, and opens the app when none is left. `CACHE_NAME` is unchanged because no icons changed. - Push payload: `PushPayload.workspace_id` (optional, additive). `server/push.ts` records each pane's workspace in `seed()` beside its title and sends it with status and ended alerts. `sw.js` keeps it in the notification data, posts it with `select-pane`, and puts it in the cold-start URL as `ws=`. - `src/lib/notificationTarget.ts`: `machineMissing`, `machineGone(roster, rosterSeq, since, id)` (a PC is gone only by a roster the server sent after it was named), `notificationLanding(roster, target)` (select, or a link carrying the alert's workspace, marked `stale` when the cached roster lacks the PC) and `sameLink`. - `App.tsx`, notification handler: - A pane the roster lists is selected as before. - Anything else is set as `pendingLink`, the same way popstate does, so it settles through `resolveLink` with its notes. With the alert's workspace, a closed pane opens its workspace's pane with the pane-closed note. - A second identical delivery keeps the pending link (and its lookup); a new link bumps `linkTick`, which the resolver effect watches, so it also settles when the pane is already selected. - A PC missing from the cached roster no longer settles at once: the link waits and `load()` asks for a fresh roster. - `App.tsx`, machine-missing effect: counts server rosters (`rosterSeq`, bumped in `load()` and the `machines` event, never for pane-status rebuilds) and decides only from one sent after the link or selection was made. A remembered selection (reload, new window) is checked against the first roster, and the selection keeps being checked against each newer one (a PC removed elsewhere while selected); "Open PC" before `load()` is checked against the roster `load()` brings. A `rosterTick` state re-runs the check when a roster comes in while a link waits, even if the list is unchanged (no re-render per poll otherwise). ## Tests - `src/pwa-notification.test.ts`: - `isAppWindow` cases, now including `/social-preview.png`, `/THIRD_PARTY_NOTICES.md`, the manifest, `/sw.js` and `/favicon.ico`. - The push handler keeps `workspace_id` in the notification data; a tap posts it and a cold start opens `/?machine=…&ws=…&pane=…&view=chat`. Both fail on the previous `sw.js`. - A reader tab used last never gets focused or messaged, and the app window takes the pane. - With only reader or raw tabs open, the app opens on the pane. - Existing tests now give clients a URL. All 17 fail without the `sw.js` change. - `src/lib/notificationTarget.test.ts`: `machineMissing`, `machineGone` (a roster no newer than the naming never settles; one after it does), `notificationLanding` (a PC the cached roster lacks waits as a stale link, known pane, unknown roster/panes/pane, the alert's workspace with `resolveLink` giving `pane-closed`), `sameLink`, and `workspace_id` parsing in `notificationTargets`. - `server/push.test.ts`: status and ended alerts carry the seeded `workspace_id`. - `scripts/notification-view-regression.ts`: an ended pane's alert delivered twice while the session lookup is held (`page.route` on `/api/session`, released after the second tap) lands on its workspace's pane with "That agent has closed: its workspace is open". Run alone through `bun run check run`: passes with the fix, times out on the note with the dedupe and `linkTick` taken out. - First commit: `bun run build` then `bun run check run bun scripts/ui-contexts-regression.ts`: pass. After the review follow-ups: `bun run build` then `bun run check run bun scripts/ui-contexts-regression.ts` (includes `checkNotificationStartup`, which pins the `select-pane` message shape, and `checkNotificationView`): pass. - `bun run check fast` (workflow syntax, generated types, typecheck, build, unit tests: 3080 pass, 0 fail): pass. ## Notes - #957 (service worker cache per build) also edits `public/sw.js`, in the cache and fetch code; this PR edits the push and notificationclick handlers and no longer calls `isCacheFirst`, so the two should merge with at most a textual conflict. Otherwise no open PR touches `public/sw.js`, `src/lib/notificationTarget.ts` or `src/lib/deepLink.ts`. Several open PRs edit `src/App.tsx` (#951, #917, #909, #890, #822), but none in the notification or machine-missing code. - Wording: the notes reuse the existing link strings, so no new i18n entries. A reload of a removed PC also shows "That link's PC is not set up here"; a separate non-link wording is left out. - An alert from a server before this change, or for a pane that alerts before the server's first `seed()` saw it, has no `workspace_id`: its closed pane still gets the workspace-closed note and herdr's fallback pane. - A tap for a PC the tab's roster lacks selects that PC while a fresh roster is fetched, so the pane area is empty until it arrives (the same state as a reload remembering a removed PC); a failed fetch is retried by the next poll. - Left out: the push handler's `watching` check still counts any visible window, a reader tab included; a split view's other half on a removed PC is not handled here. ## Merge-session review Fixed (2be0fd0, 3b87dfc): - **Ended pane still in the cached roster** (medium): a tap on a pane the resumed tab still listed selected it and dropped the alert's `workspace_id`; when the next roster removed the pane, the app fell back to herdr's focused pane with no note. The tap now keeps the alert's workspace as a hint with the selection (cleared by any other pick); once the selected pane is confirmed gone against `fetchSession`, the hint resolves through `resolveLink`: the workspace's pane opens with "That agent has closed: its workspace is open". Regression in `scripts/notification-view-regression.ts`: the pane is listed at the tap, closed through herdr afterwards, with herdr's focus in another workspace; it times out without the fix. - **Tab alerts without `workspace_id`** (medium): with push off and alerts on in this tab, `showPaneStatusNotification` / `showPaneEndedNotification` now put the pane's `workspace_id` in the notification data, and the worker-less fallback click goes through the same landing as a worker's tap. Unit test in `src/lib/notifications.test.ts` (fails without the fix). - **Remove PC on the selected PC** (low): the server tells the roster before it answers the DELETE, so the selection check showed "That link's PC is not set up here" for the user's own removal. `MachineSidebar` now selects this PC before the request and restores the selection if the request fails. `scripts/machine-conflict-regression.ts` gains an event stream and a DELETE that tells the roster first; it fails without the fix. - **Unbounded wait in the browser check** (low): the `/api/session` lookup wait races a 4 s timeout, and `release()` / `unroute` run in a `finally`. Deferred: - An old server's push (or a pane that alerts before the server's first `seed()`) with no `workspace_id`, whose pane alone is gone still shows "That link's workspace is closed"; settling such a tap silently is a wording/policy choice, left as in the Notes above. Checks: `bun run check fast` (3081 pass, 0 fail); `CI=1 bun run check run --build bun scripts/ui-contexts-regression.ts`: pass; `CI=1 bun run check run --build bun scripts/machine-conflict-regression.ts`: pass. 🤖 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 (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
…bar (#964) ## Problem A phone shows a pane in the terminal lens while a desktop tab of the same pane takes the pty back to its own size (a click or key in the desktop terminal, or the desktop window coming back to the front). The phone's xterm stays at phone width while herdr streams rows drawn for the desktop's width, so lines wrap in the wrong place and agent TUIs look scrambled. Typing in the input line, Send, Enter and key-bar taps did not fix it. Only a touch on the grid itself, a focus change or a reload did. ## Cause In the terminal lens, an interact client stores `pane-geometry` in `sharedGridRef` and then ignores it, because it drives the grid itself. The only thing that took the size back while the window kept its focus was `reclaim`, and that runs only for a pointerdown or keydown whose target is inside `term.element`. The phone's usual input paths are all outside that element: - `sendTerminalLine` submits from the `TerminalInput` textarea. - `pressEnter` sends `\r`/keys from the same line. - `pressKey` calls `term.input()` from the key bar, with no DOM event on the grid. ## Fix - The size check moves out of `reclaim` into `reassertIfStale()`. When the shared grid differs from this grid, the tab is in use and has not released its pane, it calls `refit()` (force resize). - `reclaim` keeps its event filters (target inside the grid, no IME/repeat key, primary button) and then calls the helper. - `sendTerminalLine`, `pressEnter` and `pressKey` call it before they send, so the resize frame goes out before the input on the same socket. - `TerminalInput` gets an optional `onFocus`, wired to the same helper. Tapping into the input line counts as the user being back, the same way a tap on the grid does. - No resize was added for the keyboard opening or closing. That is a separate policy question (viewport-resize-2). ## Tests - `scripts/chat-size-regression.ts` `checkFocusedWindowReclaimsTerminalSize` (already in `ui-contexts-checks.ts`, so it runs in the browser lane) now continues with two clients after its existing steps. A desktop at 1280x800 and a phone at 390x844 in line mode take turns: - a tap into the input line; - a line sent with Enter; - a key-bar Ctrl+C; - Enter on the empty line. Each phone step comes after a desktop key has taken the pty back. Each step asserts that the phone's own-size `resize` frame goes out before its `submit`/`input` frame, and that the pane's shell (`stty size`) reports the phone's size again. - `CHAT_SIZE_CASE=reclaim bun scripts/chat-size-regression.ts` now runs that check on its own. - Without the `src/` change the new step fails ("a tap into the input line takes the pane back to the phone's grid"). With it, `bun run check run --build bun scripts/chat-size-regression.ts` (with `CHAT_SIZE_CASE=reclaim`) passes. - `bun run typecheck` and `bun run test:unit` (3069 pass, 0 fail) pass. - `check full` / `concurrent` was not run. ## Notes - Source: a mobile-terminal stability audit, finding viewport-resize-1. Related: #472 (the chat-lens version) and #712 (an attach holds the pane's size). Neither covers this path. - No open PR touches `reclaim`, `sharedGridRef`, `sendTerminalLine`, `pressEnter` or `pressKey`. #951, #950, #946, #945, #936, #935, #932, #910, #894, #859 and #822 edit other parts of `PaneTerminal.tsx`. #936 also edits `TerminalInput.tsx` (dictation), so a small textual conflict is possible there. - The size tug-of-war between two devices that both type into the same pane remains by design: whichever device the user is typing on takes the pty. A "sized by another device" notice and passive adoption of the foreign grid were left out. 🤖 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>
Drop the Automatic chat focus setting: selecting a pane keeps today's autofocus (desktop yes, touch no), and Mod+Shift+E is the explicit way to reach the input from a hardware keyboard. The Turkish strings went with i18n.tr.ts on main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
|
Thanks! I merged main and kept the Mod+Shift+E shortcut, but dropped the Automatic chat focus setting: other agent UIs (Paseo, t3code) don't offer one, and the shortcut already gets a hardware-keyboard iPad to the input without a tap. The Turkish strings went with Checked on 0be3413: |
devswha
changed the title
kratam
added a commit
to kratam/herdr-web-ui
that referenced
this pull request
Oct 26, 2026
Upstream dropped it in devswha#822; the iPad with a hardware keyboard needs On to land in the message box after a pane switch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VoME51Ex6avEzqEBmkYzGd
kratam
added a commit
to kratam/herdr-web-ui
that referenced
this pull request
Oct 27, 2026
Upstream dropped it in devswha#822; the iPad with a hardware keyboard needs On to land in the message box after a pane switch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VoME51Ex6avEzqEBmkYzGd
kratam
added a commit
to kratam/herdr-web-ui
that referenced
this pull request
Oct 29, 2026
Upstream dropped it in devswha#822; the iPad with a hardware keyboard needs On to land in the message box after a pane switch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VoME51Ex6avEzqEBmkYzGd
kratam
added a commit
to kratam/herdr-web-ui
that referenced
this pull request
Oct 30, 2026
Upstream dropped it in devswha#822; the iPad with a hardware keyboard needs On to land in the message box after a pane switch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VoME51Ex6avEzqEBmkYzGd
kratam
added a commit
to kratam/herdr-web-ui
that referenced
this pull request
Nov 2, 2026
Upstream dropped it in devswha#822; the iPad with a hardware keyboard needs On to land in the message box after a pane switch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VoME51Ex6avEzqEBmkYzGd
kratam
added a commit
to kratam/herdr-web-ui
that referenced
this pull request
Nov 7, 2026
Upstream dropped it in devswha#822; the iPad with a hardware keyboard needs On to land in the message box after a pane switch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VoME51Ex6avEzqEBmkYzGd
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
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; head9f77bd67a37c09df4a8e8e23c1c2d18accf8ee02. 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 fastpassed at code commitde989f4e7463ad0a2755fc4d33d4944f0f3c1fa6: generated types, typecheck, production build, 3310 unit tests passed, 0 failed, 50 skipped. Workflow syntax validation skips locally because actionlint is unavailable.changelog.d/pane-input-focus.md; all 12 changelog tests passed.bun run check run --build bun scripts/pane-input-focus-demo-regression.tspassed 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.