Skip to content

fix(herdr-core): read the screen's colors so Claude's grey suggestion is no draft - #2674

Merged
devswha merged 2 commits into
mainfrom
fix/issue-2609
Nov 3, 2026
Merged

devswha merged 2 commits into
mainfrom
fix/issue-2609

Conversation

@devswha

@devswha devswha commented Nov 3, 2026 •

Copy link
Copy Markdown
Owner

Fixes #2609

Problem

With OVIS_RUNTIME=herdr-core, every Chat send to a Claude Code pane that showed a grey suggested prompt was held with "Not sent: Claude Code's input box in the terminal already holds …" and needed Clear it and send.

The chat's box check tells Claude's grey (dim) suggestion apart from typed text by the colors of herdr's visible ANSI read. The herdr-core adapter answered that read with hc's terminal.read, which is plain text only, so every non-empty box counted as a draft.

Fix

server/herdr-core/adapter.ts: a pane.read with format: "ansi" and source: "visible" now takes the screen's colors from the first paint of an observe: true attach. Such an attach never types or resizes, and it is closed as soon as the screen has arrived. paintedScreen turns that paint into the same window hc's own text read gives:

  • Each row becomes one line with its SGR codes. Every other escape is dropped (cursor moves, modes, tab stops, title).
  • Trailing ASCII spaces are trimmed from each row, as hc does, except spaces drawn in a foreground or background color, which stay (OpenCode tells its Tab space from padding by color). A U+00A0 stays.
  • Blank rows at the bottom are dropped, and as many history lines are taken in at the top. The attach asks for rows lines of history for this.

This matters because the server only uses the colors once their text equals the live screen (viewportShowsLive, since herdr-core reports no scroll). Claude leaves a blank row under its footer once the chat has scrolled, and without the trimming and history alignment the colors were rejected in exactly that common case. A paint of an unexpected shape falls back to the plain text, which keeps the current behavior: the box counts as a draft.

One limit remains: while a soft-wrapped line is on screen, hc's text read joins it with the next line but the paint does not. The texts then differ and the send waits for confirmation, as before. The guide now states that limit instead of "screen reads carry no colors".

Reproduction (origin/main 4bc043d)

This used a real hc 0.1.17 in an isolated core (mkdtemp XDG_*/HOME, own HC_SESSION/HC_SOCKET) with the herdr-core adapter on a temp socket. A terminal reports app=claude-code and draws the recorded Claude Code 2.1.311 suggestion fixture (server/fixtures/claude-suggestion-2.1.311-119.ansi), and the script calls the chat send's box check, freeClaudeBox:

  • main: refused: input_draft … | draft: "do it". The visible ANSI read carries no SGR.
  • this branch: box check passed. This holds with no scrollback, 118 filler lines and 300 filler lines (blank cursor row under a scrolled screen).
  • this branch, with the box holding typed do it and Claude's cursor after it: still refused: input_draft | draft: "do it", so Clear it and send is offered as before.

Validation

  • bun test server/herdr-core/: 84 pass, 5 skip, 0 fail. The new chat (herdr-core): Claude Code's grey prompt suggestion is read as typed text, so every send needs "Clear it and send" #2609 adapter test fails on main (the read is plain text) and passes here; others cover the plain-text fallback (no paint, stream ended) and colored trailing spaces.
  • OVIS_TEST_HC_BIN=<hc 0.1.17> bun test server/herdr-core/live.test.ts: 3 pass against a real, isolated hc.
  • bun run check fast: green (workflow syntax, generated types/themes, typecheck, build, unit tests).
  • Not run: bun run check full, and a real Claude Code session (the recorded fixture stands in). Hermes' hint goes through the same verified-colors path (italic and background SGR survive in the paint), but it was not exercised.

@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 16 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 4bc043d and cc77304.

Files selected for processing: 4
  • changelog.d/herdr-core-grey-suggestion.md
  • docs/guide.md
  • server/herdr-core/adapter.test.ts
  • server/herdr-core/adapter.ts
  • 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.

Share

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.

@devswha

devswha commented Nov 3, 2026

Copy link
Copy Markdown
Owner Author

Review round 1 (omo and Codex adversarial, head 8139387). Here is what changed and why.

Fixed

  • HIGH (both reviewers): a slow attach reply left the paint timeout's rejection unhandled. It was reproduced with a fake core that answers terminal.attach after 6 s: Bun reported herdr-core sent no screen of t1 within 5000ms as an unhandled error. coloredScreen no longer rejects at all. No paint within PAINT_MS, or a stream that ends before painting, now answers hc's plain text, as a read without colors, so a send treats the box as a draft as before. With the fix, the same repro answers "plain" and nothing is reported as unhandled. This also covers omo's MEDIUM: a missing paint used to fail with a timeout instead of falling back.
  • LOW (omo): the watch-only attach outlived the callers' deadline. PAINT_MS is now 1 s, within the 1.5 s a prompt poll gives its colored read.
  • MEDIUM (Codex): trailing spaces drawn in a color were trimmed. OpenCode tells the space Tab typed after a skill from panel padding by its color. paintedScreen now trims only trailing spaces with no foreground or background color, as hc's text read does; text comparisons trim the rest anyway. With the recorded OpenCode 2.0.32 screens (ansi-accepted, -closed, -first-selected and the narrow variants) fed through paintedScreen, the @effect input rows are unchanged and skillAccepted agrees with the raw screens.
  • Tests (omo LOW): new cases cover a stream that ends before painting, an attach that never paints (both answer the plain text and close what is still open), and a colored trailing space that is kept next to Claude's inverse cursor, which is trimmed.

Not changed

  • Tab accepting the suggestion between the color read and the text read (Codex HIGH, omo MEDIUM non-blocking). This is the same window herdr's own visible path in claudeBoxReads has: one colored read, then text reads, then the write. Only the scrolled-history path takes a second colored read. Before this PR, herdr-core refused every non-empty box; with it, herdr-core behaves as herdr does. Closing that window for both runtimes would be a separate change to claudeBoxReads.
  • Cursor moves dropped rather than expanded to spaces (omo LOW, unreproduced). hc 0.1.17's paint writes gaps as spaces: gap\e[10Cafter printed into a terminal comes back painted as gap after. If a paint ever did skip cells, the texts would differ and the send would wait for confirmation as before, which fails safe.

Re-verified at the new head: bun test server/herdr-core/ (84 pass, 5 skip), the real isolated hc 0.1.17 repro (the suggestion sends with no scrollback, 118 or 300 filler lines; a typed do it is still refused and offered for clearing as do it), and bun run check fast.

@devswha

devswha commented Nov 3, 2026

Copy link
Copy Markdown
Owner Author

CodeRabbit hit its review limit on this PR ("Review limit reached" at 8139387), so no CodeRabbit review ran. The omo review and a Codex adversarial review stand in; their findings and resolutions are in the comment above.

@devswha

devswha commented Nov 3, 2026

Copy link
Copy Markdown
Owner Author

omo re-review at cc77304: CLEAR. LOW and non-blocking items, left as follow-ups:

  • The Tab race in claudeBoxReads's unscrolled path is code that already exists and also runs on the default herdr runtime. A follow-up could read the colors again after the live read, as historyEndShowsLive does for the scrolled path.
  • await channel.close() sits outside PAINT_MS. If hc delivers the paint quickly but acknowledges CLOSE late (reproduced only against a fake core), the read times out instead of falling back. A plain terminal.read on a stalled hc times out the same way.
  • Cost: every visible ANSI read on herdr-core now opens a short watch-only attach with rows lines of history. It never types or resizes; it could be cached per screen revision if the cost shows.
  • PR description: the line about trailing spaces is corrected (colored ones are kept).

@devswha
devswha merged commit 12a7c63 into main Nov 3, 2026
21 of 22 checks passed
@devswha
devswha deleted the fix/issue-2609 branch November 3, 2026 23:15
devswha added a commit that referenced this pull request Nov 4, 2026
…echeck (#2693)

## Summary

origin/main (12a7c63) fails `bun run typecheck` with four errors in
`server/herdr-core/adapter.ts`. #2671 typed `HcConnection.call` and
`open` by hc method name, generated from hc's own schema. #2674, merged
right after it, added a `hc.open<{ channel: number; terminal?:
HcTerminal }>("terminal.attach", ...)` and a `hc.call<{ text: string
}>("terminal.read", ...)` written for the old untyped signature, where
the type argument was the result type.

This drops those two type arguments. The method name now types both
calls: `terminal.attach` gives `AttachResult` (whose `terminal.rows` the
code reads) and `terminal.read` gives `ReadResult` (whose `text` it
reads). Nothing changes at runtime.

No changelog fragment: users notice nothing.

## Validation

- `bun run typecheck`: clean (four errors on main before).
- `HERDR_TEST_MODE=unit bun test server/herdr-core`: 90 pass, 0 fail.
- `bun run check fast`: not run locally. The heavy-run slots were all
held by other lanes, so the lead chose to let CI's Fast checks gate this
PR.
- Not run: Integration and browser lanes, and Windows. CI runs them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chat (herdr-core): Claude Code's grey prompt suggestion is read as typed text, so every send needs "Clear it and send"

1 participant