Repository navigation
feat(herdr-core): send the terminal's own replies as hc's automatic input - #2678
Conversation
…nput hc's frame kind 6 (AUTOMATIC_INPUT) carries reply bytes a viewer's terminal made itself, so they reach the program without counting as that viewer's activity (the active viewer sets the terminal's size). hc's protocol lets a client send it only when hello names automatic_input and terminal.attach confirms it; a core that does not know the kind drops the frame. - The browser tags xterm's own replies at their source: xterm fires its core's onUserInput before onData for keys, paste, IME and mouse reports, not for query answers. The input frame carries automatic: true. - The server passes the flag to a herdr-core attach only, which sends kind 6 where hc confirmed it and DATA otherwise, as before. - The live contract checks a tagged reply reaches the program on the pinned hc (0.1.14 and 0.1.17 do not name the feature, and drop kind 6 unasked).
|
Caution Review failedFailed to post review comments. GitHub was unavailable or timed out while CodeRabbit was posting the review. Please request a new review later if the pull request still needs one. This happened while posting 1 inline comment. Use
 |
| Layer / File(s) | Summary |
|---|---|
Browser input classification shared/protocol.ts, src/lib/terminalReplies.ts, src/lib/terminalReplies.test.ts, src/lib/useTerminalMount.ts, src/lib/ws.ts |
The terminal tracker distinguishes user input from terminal replies. The browser sends the automatic flag only for replies, and tracker tests cover reply detection and the fallback when xterm’s signal is unavailable. |
Automatic-input protocol negotiation server/herdr-core/connection.ts |
Hello parsing now includes string-valued features. The connection sends the automatic-input frame only when the feature is advertised and the attach reply confirms it; otherwise, it sends DATA. |
Server routing and attach-session handling server/core/input.ts, server/core/terminals.ts, server/herdr-core/attach-session.ts, server/herdr-core/attach-session.test.ts, server/herdr-core/live.test.ts, changelog.d/hc-automatic-input.md |
The server forwards the automatic flag to herdr-core sessions. The attach session applies it only to qualifying escape sequences and preserves it for buffered input. Tests cover negotiation and routing, and the changelog describes compatibility behavior. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Feature
Possibly related issues
- Issue 2708 has a related objective: it proposes identifying terminal replies from xterm’s input signal. This change adds that source-based tracker, but does not change the
retainTypedbyte filter, so the listed keys can still be dropped.
Pre-merge checks: 
-
Check name Status Explanation Title check The title "feat(herdr-core): send the terminal's own replies as hc's automatic input" matches the main change: the browser tags xterm's own replies, the server forwards the tag, and hc sends them as A… Description check The description follows the template's two sections, Change and Validation. The Change section explains the problem (query answers make the replying viewer active and set terminal size), the resulting… Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… 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: Title check
-
Explanation
The title "feat(herdr-core): send the terminal's own replies as hc's automatic input" matches the main change: the browser tags xterm's own replies, the server forwards the tag, and hc sends them as AUTOMATIC_INPUT (kind 6) frames only when both sides negotiate the feature. It is specific, single-sentence, and free of file lists or vague terms. It omits the negotiation condition, which is acceptable for a title.
Full details: Description check
-
Explanation
The description follows the template's two sections, Change and Validation. The Change section explains the problem (query answers make the replying viewer active and set terminal size), the resulting behavior across browser, server, and hc connection layers, and links the contract in hc's schema README. It does not link a maintainer-agreed issue, which the template asks for on features; the objectives reference PR
#2678but the description does not link an issue. The Validation section lists commands run, results, and skipped lanes (integration, browser, Windows). It also states that the HTTP/WS contract-test coverage is not explicitly addressed, though the WS message gained an optional field.Resolution
Optional: link the issue where a maintainer agreed to this feature, as the template requests. Optionally state whether contract tests cover the new optional automatic flag on the WebSocket input message.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 11 files. (1 skipped: 1 unsupported.)
-
Finishing Touches
-
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.
…terminal reply Codex review: the client sets the tag, so a writable client could send a command and its Enter as automatic input and skip hc's activity rules. The tag now holds only for an escape sequence without a line break, which is every reply xterm makes; anything else is typing.
|
Reviews at bd62756:
|
|
@coderabbitai review |
Action not completed
|
|
CodeRabbit hit its review limit on this PR ("Review limit reached"), so no CodeRabbit review ran. Per the lane brief I'm not waiting for it. Standing reviews at 28368c0: omo (dori review) CLEAR, plus the Codex adversarial review summarised above. |
…hod name Main now types HcConnection.call by the hc method (#2671); the test written before that passed a result type instead.
|
Merged origin/main (40dcae1..f73d57d, incl. #2671 typed hc calls, #2674, #2693, #2676, #2703) into this branch, no force-push. One follow-up commit (1f9120a, test only): the new live test passed a result type to hc.call, which main now types by method name. |
Deploying ovis with
|
| Latest commit: |
21317bd
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://33448b92.ovis.pages.dev |
| Branch Preview URL: | https://feat-hc-automatic-input.ovis.pages.dev |
|
Merge note: merged on the omo review (CLEAR at this head), Codex Terra review (no HIGH; remaining notes in the PR comments) and green required checks. CodeRabbit did not review this head (rate-limited or skipped). |
…-tab lock grant (#2766) ## Change `scripts/chat-greeting-demo-regression.ts:213` failed on PR CI (#2678, #2736, twice in a row each) with `held 0 !== 1`. The cause is a product race introduced by #2709, not the test. Since #2709, Send runs its whole claim inside `navigator.locks.request("ovis-send:<draft>")` (`underSendLock`). A Web Lock is granted in a later task, so `onSend`, and with it `afterSend` (which takes an empty chat's greeting away and docks the composer), ran after the click's frame. On an idle host the grant usually landed before the next frame. Under CI load, a frame was painted first: the greeting was still up and the composer was still lifted, the state the test checks for. - `Composer` reports a pressed Send whose claim is waiting (`onClaim(+1)` at the press, `onClaim(-1)` once the claim ran, whether it sent or not). - `PaneTerminal` counts the waiting claims. While any is waiting, the greeting is not due and the chat's empty line stays out (`sending`). The greeting therefore goes in the frame of the press, as it did before #2709. A claim that sends nothing (another tab sent the draft) brings the greeting back when it ends. ## Test The regression now holds the pane's `ovis-send` lock from the page (standing in for another tab's acknowledgement) over the click. This makes the late grant deterministic: on origin/main the test failed every time (`greetings: 1`, `transform: matrix(... -305)`, `gap: 305`). With the fix, the greeting is gone and the composer docked in the click's frame while the claim waits. After the lock is released, the claim runs (`heldSubmits.length === 1`, a bounded wait on the state, with no sleep) and the greeting stays away. ## Validation - `bun scripts/chat-greeting-demo-regression.ts`: failed on origin/main with the new test, passes with the fix. Six parallel runs under load all passed. - `bun test src/components/Composer src/lib/composerDraft src/lib/greeting src/components/PaneTerminal`: 72 pass. - `bun run check fast`: passed (typecheck, build, 11084 unit tests). - Not run: `bun run check full` (CI runs the browser jobs).
Summary
herdr-core's framed protocol has a frame kind 6,
AUTOMATIC_INPUT, for reply bytes that a viewer's terminal makes on its own, such as a cursor position or mode report. hc passes these to the program like DATA, but does not count them as that viewer's activity. With DATA, a query answer makes the replying viewer "active", and the active viewer sets the terminal's size. hc's schema README says a client may send kind 6 only whenhello.featuresnamesautomatic_inputandterminal.attachconfirmsautomatic_input: true. Otherwise it must send DATA, because older cores drop unknown frame kinds, and the input with them.This PR follows that contract end to end:
src/lib/terminalReplies.tstags xterm's own replies at their source. xterm fires its core'sonUserInputright beforeonDatafor keys, paste, IME and mouse reports, and not for query answers. The public API has no such signal, so this reads xterm's core. Without it, everything counts as typed, as before. A tagged reply goes out as{ type: "input", ..., automatic: true }, which is optional and ignored by older bridges.CoreAttachSession.write(data, automatic)). herdr's TUI path is unchanged.HelloInfonow keepsfeatures, and each opened channel knows whether both ends confirmed automatic input.Channel.writeAutomaticsends kind 6 only then, and DATA otherwise.Checked on real cores: hc 0.1.14 and 0.1.17 (pinned and latest) do not name
automatic_inputin hello or in the attach answer. On them this PR changes nothing on the wire: replies stay DATA. I also confirmed the hazard the contract warns about: forcing kind 6 without negotiation, against an isolated hc 0.1.14, loses the bytes, and the program never sees them (the new live test times out).Design idea credit: the idea of looking at hc's kind 6 came from a study of another project's hc client. No code was taken from it; this follows hc's own
schema/README.md.Validation
bun run check fast: all green (unit 10889 pass, 0 fail).bun test server/herdr-core/attach-session.test.ts: 14 pass. Four new tests cover: kind 6 sent only when hello and attach both confirm; a reply written before the attach answer keeps its order and its tag; DATA when hello lacks the feature; DATA when the attach does not confirm it.bun test src/lib/terminalReplies.test.ts: on a real xterm, a DSR 6 answer is tagged automatic andterm.inputis not. The test fails when the tracker always reports user input.OVIS_TEST_HC_BIN=<hc 0.1.14 | 0.1.17> bun test ./server/herdr-core/live.test.ts: 4 pass on each version, against an isolated core. The new test fails (the reply never arrives) if kind 6 is sent without negotiation.