Repository navigation
feat(terminal): paste into the input line and insert a line without Enter - #1853
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
| Layer / File(s) | Summary |
|---|---|
Insert contract and server handling shared/protocol.ts, shared/terminal-insert.ts, server/core/*, server/terminal-insert.contract.test.ts, site/demo/transport.ts |
The protocol adds insert requests and results, with shared text limits and validation. Server handlers check connection and pane state, reject unsafe input, and send text without Enter. Contract tests cover accepted text, refusals, and write-time state changes. |
Client insert lifecycle src/lib/ws.ts, src/lib/ws.test.ts, src/lib/useTerminalSends.ts, src/lib/compose.ts, src/lib/terminalDraft.ts, src/lib/*test.ts |
The client tracks input leases and insert acknowledgements, and does not send when capability or readiness checks fail. The send hook maps results to notes. Draft versions prevent late acknowledgements from clearing newer text. |
Input-line Insert and Paste UI src/components/TerminalInput.*, src/components/KeyBar.*, src/components/PaneTerminal.tsx, src/lib/i18n/*, scripts/terminal-input-regression.ts, scripts/terminal-input-spacing-regression.ts, scripts/key-bar-customization-demo-regression.ts, DESIGN.md, changelog.d/terminal-paste-insert.md |
The input line adds Insert and tap-triggered clipboard Paste. Paste is applied only when the captured input state remains current. The key bar displays paste guidance and status notes. Related styling, translations, and regression checks are updated. |
Demo browser regression and CI scripts/terminal-paste-insert-demo-regression.ts, scripts/ci-browser.sh, scripts/ci-browser.test.ts, docs/development.md |
The browser regression covers paste races, insert acknowledgements, refusal cases, and display variants. Browser CI and the development command list include the script. |
Priority:
Estimated code review effort: 4 (Complex) | ~60 minutes
Change: Feature
Merge Risk: 
63cbb
-
No actionable merge-blocking issue was established. The Paste guidance is reachable, and the Insert regression checks for grid resizing.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 57.58% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 33 functions across 31 files. (5 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title clearly summarizes the two main changes: terminal Paste into the input line and Insert without Enter. Description check The description explains the user behavior, implementation scope, rationale, known limits, issue reference, and validation results. It includes WebSocket contract-test coverage and relevant regression… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description explains the user behavior, implementation scope, rationale, known limits, issue reference, and validation results. It includes WebSocket contract-test coverage and relevant regression checks, although it uses headings different from the template.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 57.58% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 33 functions across 31 files. (5 skipped: 5 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- 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.
|
CodeRabbit's review limit was reached on this PR, so no CodeRabbit review ran. Per AGENTS.md, here is the review that stands in for it on this WS-contract change, with how each finding was resolved. Code-review agent (omo-native-code-reviewer) on the rebased diff: REQUEST_CHANGES
Codex (gpt-6-astra): no HIGH
A real review is still needed before merging: this PR adds a WS contract ( |
|
Merge pass review-work for 8a05cb3: REVIEW FAILED on one flaky assertion. The production code holds up. This PR adds a WS contract and CodeRabbit was rate-limited, so this comment records the independent review. Blocker: the new "Insert takes no grid back" assertion ( Checked and passing
Non-blocking
|
|
Thanks. Both test failures from the merge pass review and this run's ui-regression lane are fixed in test-only commits; no production code changed:
Typecheck and the conventions and ci-browser tests pass. The non-blocking notes are left for a follow-up. |
|
Merge pass review-work for a0cf07b: one mismatch left. The flake is fixed: the resize count now waits for the grid's refit, which is a condition and not a time, and terminal-input-regression passed 3 runs in a row. Blocker: Insert accepts TAB, but the description says it refuses every control character.
Checked and passing:
|
|
Thanks. Insert now refuses a TAB, as the description says (f704f23): a shell takes TAB as completion, so the prompt could show something other than the field. A TAB now gets the same This push also fixes the "Browser regression (other scripts)" failure (f4aad65). Main's |
|
Codex review on Sol 6.1 Fast (xhigh), on the production diff after the TAB change: no HIGH or MEDIUM findings. Its one LOW is fixed in f3c6424: a paste restores the caret one frame later, and that frame could focus the old line after another pane had been picked in the split, which selects that pane again. The frame now re-checks the paste target and the draft first. Typecheck passes. The paste/insert demo regression is queued locally, and CI's browser lane runs it too. |
|
CI on f3c6424 passed Fast checks, Integration tests, Native Windows install and Browser regression (other scripts), so the key-bar demo fix is confirmed there. The ui-regression lane hit the Insert resize count again (10 !== 9). The settle condition in 0fd16d7 was not enough: PaneTerminal's observer fits and sends the resize together after its 120 ms rest, so a refit still waiting out that rest is invisible to "last resize == rows drawn". 63cbbcd moves the Insert check to right after the input line's Send. There the test already shows that the grid's own ResizeObserver never fired, and that stillness is now asserted beside the resize count, so a late refit can no longer land in it. The Insert assertions themselves are unchanged. Locally, ui-regression passed with the moved check. Two more runs are under way, and CI runs it as well. |
|
All required checks pass on 63cbbcd: Fast checks, Integration tests, Browser regression (ui-regression), Browser regression (other scripts) and Native Windows install. The Insert resize count passed in CI's ui-regression lane and in two full local runs. Every point from both merge-pass reviews is addressed above. A real review is still needed for the new WS contract. |
|
Merge pass: one rebase needed. #1874 is merged on main. |
63cbbcd to
04121ec
Compare
|
Rebased onto main after #1874 and pushed 04121ec (was 63cbbcd). I dropped this PR's change to |
04121ec to
d8c0095
Compare
|
Rebased onto main after the batch (#1831, #1890, #1895, #1897, #1830) and pushed d8c0095 (was 04121ec). The only conflict was in |
d8c0095 to
da02cd9
Compare
|
Rebased onto main after #1837 and #1899 and pushed da02cd9 (was d8c0095). The only conflict was the |
…usal Review follow-ups on the input line's Insert and Paste: - HerdrSocket.insert keeps the pane's input lease from the tap and sends nothing when it changed while the insert waited on the snapshot or a liveness probe, so a draft never lands in a reattached pane. - It returns capabilities_unknown, as submit does, when the snapshot is late, instead of telling the user to update the PC. - invalid_insert_text (a control character, such as an escape copied with coloured output) gets its own note instead of "the terminal is not ready". - A paste that waited on the clipboard is dropped when its pane was put out of sight (another pane picked in the split). - "Enter" moves to the composer part of the dictionaries, where its only user lives. Refs #1047
…gression Main renamed browser storage keys to ovis:*; the demo seeded and read the old herdr-web-ui:* keys, which the app no longer reads and the conventions test refuses. Refs #1047
… out The merge pass saw "Insert takes no grid back" fail in 1 of 4 runs: the lens switches before it refit the grid, and that refit's resize could land just after the count was taken. The count now starts once the last resize sent matches the rows drawn, so only a resize Insert itself sent can change it. Refs #1047
Main's #1527 checks that the input line's field and send button have as much room below as above. This branch puts the line's label above the field (as the send button carries its word above its face), so the field is measured with its label: the room above is the line's padding over the label, which still has to match the room under the field. Refs #1047
Insert types the line at the prompt as the field shows it, but a shell takes a tab as completion, so what landed could differ from what the user saw. A tab is now refused with the other control characters (invalid_insert_text, "the line holds a control character"), as the PR description already said; Send is unchanged. Refs #1047
The touch key bar's fixed keys are now the Input line toggle and Paste; the key-bar customization regression from main lists them both ahead of the user's keys. Refs #1047
A paste puts the caret after its text one frame later. If another pane was picked in the split, or the line was written again, before that frame, focusing the old line would select its pane again and move the caret. The frame now checks the paste target and the draft first. Refs #1047
Waiting for the last resize to match the drawn rows did not settle the grid: PaneTerminal's observer fits and sends together after its 120 ms rest, so a refit from the earlier lens switches could still land in the count (CI: 10 !== 9). Insert is now checked right after the input line's Send, where the grid's own observer is shown never to fire, and that stillness is asserted beside the resize count. Refs #1047
da02cd9 to
c11d0ff
Compare
|
Rebased onto main after #1852 and pushed c11d0ff (was da02cd9). The one conflict was the |
|
Merge pass review-work report. Overall: PASSED. This PR adds a WS contract and touches held input. CodeRabbit didn't review it, so per AGENTS.md the independent review is recorded here and in the two earlier comments.
QA
Behaviour checked
Follow-ups (MINOR)
All required checks are green and no review thread is open.
CodeRabbit did not review this PR (rate-limited or skipped). The reviews above stand in for it. |
Fixes #2493 ## What An opt-in **Paste and send** icon button (lucide `ClipboardPaste`, beside Attach) in the chat composer. It reads **this device's** clipboard on the press (never the PC's) and sends its text through Send's own path, so it is queued while the agent works, refused or kept in the box exactly as a typed message would be. The issue proposed no setting; per the maintainer's rule that the composer's row stays minimal, it is behind a new device setting, **Settings → Chat → Composer → Paste and send button** (`composerPasteSend`, off by default). With it off, nothing changes. Behaviour: - Enabled only for an empty box with no files, while connected and not sending; one read per press (a double press reads once). - The text is put in the box and then sent by `send()` once the box is drawn with it, so draft protection, the cross-tab send lock, acknowledgement and the comment carry all apply unchanged. A refused or unconfirmed send leaves the pasted text in the box; nothing is retried. - A read that answers after the pane/PC, the connection or the box changed sends nothing. - Empty, refused/unavailable, or over-limit clipboards are said in the composer note; nothing is cut to fit. Design: the Codex and Claude apps have no such button; it follows Ovis's own terminal key bar Paste (#1853). DESIGN.md is updated. ## Screenshots (demo app, 1280 and 390, dark and light) Desktop dark / light: <img src="https://raw.githubusercontent.com/devswha/ovis/9ae724d9e7df24ebd0851ec4f83c0b85d1e18baf/desktop-dark-composer.png" width="640"> <img src="https://raw.githubusercontent.com/devswha/ovis/9ae724d9e7df24ebd0851ec4f83c0b85d1e18baf/desktop-light-composer.png" width="640"> Phone 390 dark / light: <img src="https://raw.githubusercontent.com/devswha/ovis/9ae724d9e7df24ebd0851ec4f83c0b85d1e18baf/phone-dark-composer.png" width="390"> <img src="https://raw.githubusercontent.com/devswha/ovis/9ae724d9e7df24ebd0851ec4f83c0b85d1e18baf/phone-light-composer.png" width="390"> Empty clipboard note (desktop dark, phone light): <img src="https://raw.githubusercontent.com/devswha/ovis/9ae724d9e7df24ebd0851ec4f83c0b85d1e18baf/desktop-dark-empty-note.png" width="640"> <img src="https://raw.githubusercontent.com/devswha/ovis/9ae724d9e7df24ebd0851ec4f83c0b85d1e18baf/phone-light-empty-note.png" width="390"> Settings (desktop dark, phone light): <img src="https://raw.githubusercontent.com/devswha/ovis/9ae724d9e7df24ebd0851ec4f83c0b85d1e18baf/desktop-dark-settings.png" width="640"> <img src="https://raw.githubusercontent.com/devswha/ovis/9ae724d9e7df24ebd0851ec4f83c0b85d1e18baf/phone-light-settings.png" width="320"> ## Validation - Failing first: `src/components/ComposerPasteSend.test.tsx` (11 cases) and the new `settings.test.ts` case failed on origin/main (12 fail), pass now. - `bun test` on ComposerPasteSend, settings, paletteSettings, conventions, i18n, changelog-fold, ComposerRestored and ComposerCrossTab: all pass. - `bun run check fast` (run on old-desktop through the heavy runner): ok (typecheck, build, unit 11680 pass / 0 fail). - Manual QA in a real Chromium on the demo app at 1280 and 390 (mobile emulation), dark and light, with clipboard permission granted: off by default; once on, the button sits beside Attach on the controls row, sends the clipboard text exactly once (counted on the socket's submit frames), is disabled over a draft, says so for an empty clipboard, and the Settings toggle reads on; no page errors. - Skipped: Mac app (WKWebView) clipboard prompt, Integration and browser regressions (left to CI).
Refs #1047.
What it does for the user
On a phone, the terminal's key bar gets a Paste key, and the terminal's input line gets an Insert button beside Send.
Server side (
server/core/input.ts): the server checks the text's bounds, refuses observe connections, held or unready panes, a password prompt on screen and stale claims, and re-checks all of these after every wait and at the write. It then makes a singlepane.send_textcall with herdr's guard. It sends no Enter, no keys, no focus, no resize and no--takeover.Why
Putting text at the prompt without running it (a path to edit, a flag to add) had no way in from a phone: the input line always sent Enter, and direct typing has no reliable paste. No widely used coding-agent app (the Codex and Claude apps, Paseo, T3 Code) offers a phone terminal draft with a separate no-Enter insert. agent-of-empires' compose sheet has Insert beside Send, but it clears its draft on a synchronous boolean, which this design rejects: the draft clears on the bridge's acknowledgement, as Send's does (DESIGN.md entry).
Rebase notes
The branch was cut about 180 commits ago, so it was squashed and rebased onto current main. Later commits on this PR are the review fixes and test updates for main's changes:
insertTerminalTextmoves intosrc/lib/useTerminalSends.ts, next tosendTerminalLine, following main's split ofPaneTerminal.tsxinto hooks.pasteTargetand the paste guidance stay inPaneTerminal.tsx, which owns their refs.src/lib/i18n/composer.*.tsandterminal.*.ts.2 * --space-1) and adds the label row. The slot is taller than on main, so in line mode on touch screens the grid gets a little shorter.server/core/composer.ts) is untouched: Send behaves as on main.How it was verified
bun run check faston the rebased branch: workflow syntax, generated files, typecheck and build pass in every run. One full run passed all unit tests (6397 pass, 0 fail). The PC is shared by many test runs, and later runs each failed one unrelated timing-bound test (install-star.test.tson an inotify EMFILE,portal.test.tson a 2s bound). Both pass alone, and this branch does not touch them. CI's Fast checks are the clean run.server/terminal-insert.contract.test.tson the isolated herdr test session: 9/9 pass. It covers the insert frame typed with no Enter, the refusals (several lines, escapes, a program that turned bracketed paste off, observe, held, secret prompt, stale claim) and the write guard's race.ws,compose,terminalDraft,shared/terminal-insert,i18n,ci-browserandconventions: 181 pass. The newws.test.tscase (an Insert waiting on a liveness probe while the pane's input lease changes) fails with the fix removed.scripts/terminal-paste-insert-demo-regression.ts(enrolled inci-browser.sh) passes on the rebased branch, with 141 captures at 390 and 1440, light and dark, and Korean. On the rebase it now uses theovis:*storage keys, and it measures Paste's 44px target through the key bar's expander from fix(terminal): space the phone's input line and key bar evenly #1527. I looked at the 390 rest state, the 1440 in-flight insert (text at the prompt, no Enter) and the Korean direct-typing guidance (one line in the key bar's row).Review
Enterkey sat in the base i18n dictionary, soi18n.test.tsfailed. Fixed. Its MEDIUM about a control-character refusal reading "the terminal is not ready" is fixed with a message of its own. Its LOW about a late snapshot answering "Update this PC" is fixed: Insert now answerscapabilities_unknown, as submit does.ws.test.tscase, which fails without the fix), and a paste whose pane was put out of sight is dropped.Known limits (not in this PR)
selectionchange, which would drop the read, is not checked on iOS Safari.HerdrSocket.canInsert()is used only by tests.