[devswha]

Change

Fixes #1552. While Codex's transcript search or Claude Code's /config covered the agent's input row in the terminal, a Chat message was typed into that screen's search box and shown as accepted, so it was silently lost.

#1637 (merged after the report) added the input-row recognizers and a "still open" guard that holds Chat's Send. That guard only arms when a terminal peek is ended by hand. A screen opened any other way never armed it: from the terminal itself, or a peek's screen kept with Keep Terminal and then the header's Chat. So #1637 does not cover this.

src/lib/usePeekReturn.ts now also infers the guard from whole frames and when the chat comes back by its lens. It reuses the existing unnamed notice, A screen is still open in the terminal, and the note Not sent: a screen is still open in the terminal. with Send anyway, so no new strings. It arms only when all of these hold:

• the agent is Claude Code or Codex (SCREEN_OVER_ROW). Their recognizers read a missing or covered row at rest as a screen over it. Other recognizers answer gone for layouts they simply don't know (pi's extra footer rows), which is no proof a message would be lost.
• that recognizer read this pane's empty row before (ready), so an unknown variant never gets the notice.
• the chat is mounted, no peek is up, no guard is already set, and the agent is at rest (not working or blocked).
• the screen has not already raised a guard by any path (screenHeldRef, set by setStillOpen and so also by the existing peek path): once per screen. A dismissal holds until a frame shows the row again, and frames are still read under the Terminal lens while it holds, so a screen that closed there ends the episode. A turn of the agent drops the inferred guard and ends the episode unless a peek's guard still stands.

The notice clears on the first whole frame that shows the row again, as before. For this unnamed screen, Show terminal opens the Terminal lens rather than a peek, because no command of the chat's opened the screen. Frames are read outside a peek only while the agent is not working, and once the row is known, only while the chat is mounted. Nothing is typed to close anything.

No changes to PaneTerminal.tsx, App.tsx, i18n, server/ or shared/. DESIGN.md and docs/agent-commands.md each get one sentence.

Design precedent: the same as the still-open guard's DESIGN.md paragraph, where this sentence lives. It follows the composer's refusal of a Claude box that may not be empty and t3code's composer banner stack. Warp and Zed give an interactive program the input until it exits, which this bridge can only see on the screen.

Validation

• Browser: new case in scripts/terminal-autoswitch-demo-regression.ts at 1280 and 390 px, using the demo's Claude pane.
  • Steps: open /btw's screen through a peek, Keep Terminal, then the header's Chat.
  • Asserts: Send keeps the message with the unnamed note and notice, and nothing is submitted. Escape draws Claude's box back, which clears the notice, and the kept message then sends.
  • Before the fix it fails at that note. After it, the whole script passes (70 PASS), including every existing peek and still-open case.
• Unit: new src/lib/usePeekReturn.hook.test.ts (react-test-renderer, real hook, recognizer answer set per frame).
  • Claude gone and Codex covered arm the guard.
  • pi's gone does not; nothing arms without a prior ready or while blocked.
  • A dismissal holds for the same screen and re-arms after the row comes back, including when the row comes back while the Terminal lens is up. A dismissed peek notice is not replaced by an unnamed one for the same screen, even after a turn.
  • A turn drops the inferred guard; the chat's return by its lens arms it.
• scripts/command-outcome-demo-regression.ts passes (23 PASS).
• bun run check fast passes: workflow syntax, generated types and themes, typecheck, build, and 6268 unit tests with 0 failures.
• Gate review (a different model) took two rounds. The first round found pi's footer rows giving a false hold, the inferred guard surviving a turn, and a dismissal re-arming on the next frame. The re-review found a screen closed under the Terminal lens not ending the episode, and a dismissed peek notice being replaced by an unnamed one. All are fixed above, each locked by a hook test case that failed before its fix.
• Not checked: live Codex 0.163.0 transcript search and Claude Code /config were not driven in this run. Their recognizers' gone/covered readings come from feat(chat): return the terminal peek to Chat from each agent's input row, and hold sends while a screen is open #1637's recorded screens.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: devswha/ovis
• Branch: fix/chat-overlay-send-guard
• File: .coderabbit.yaml
• Commit: 362c4bb
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 7dd89b84-57d1-46d5-8911-519b5687972e

📥 Commits

Reviewing files that changed from the base of the PR and between 0bc6691 and 362c4bb.

📒 Files selected for processing: 6

• DESIGN.md
• changelog.d/chat-holds-send-over-open-screen.md
• docs/agent-commands.md
• scripts/terminal-autoswitch-demo-regression.ts
• src/lib/usePeekReturn.hook.test.ts
• src/lib/usePeekReturn.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

📝 Summary

Summary by CodeRabbit

• New Features
  • Chat now shows a notice when Claude Code or Codex has an open screen outside a terminal peek. Show terminal opens the Terminal lens.
• Bug Fixes
  • Sending is blocked with a warning while the screen remains open, with an option to Send anyway. The draft is retained; the notice clears when the input row returns or the agent begins a turn.

For Claude Code and Codex, usePeekReturn now detects certain screens that cover a previously observed input row and raises an unnamed notice. Tests and a terminal autoswitch regression cover the guard lifecycle and Chat’s handling of a retained draft.

Changes: Open-screen notice

Layer / File(s)	Summary
Detect and track open screens src/lib/usePeekReturn.ts	The hook tracks ready input rows and infers an unnamed guard when an eligible row becomes missing or covered. It resets this state on pane or terminal changes and agent turns. The Show terminal action opens the Terminal lens for an empty-command guard.
Cover guard lifecycle in hook tests src/lib/usePeekReturn.hook.test.ts	Tests cover when missing or covered rows raise a notice, when they do not, and how dismissal, row return, chat mounting, named peeks, and agent status affect the guard.
Verify and document the Chat send behavior scripts/terminal-autoswitch-demo-regression.ts, DESIGN.md, docs/agent-commands.md, changelog.d/chat-holds-send-over-open-screen.md	The regression verifies that Send retains the draft while the screen remains open and submits it after the input row returns. The design, documentation, and changelog describe the notice behavior.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

• devswha/ovis#1767: This PR continues the usePeekReturn implementation by adding inferred unnamed guards for screens outside a peek.

Merge Risk: ⚪ Minimal · up to 362c4

The documented send guard and dismissal behavior show no established merge-blocking issue; the change is ready for normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (3 skipped: 3 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the primary change: preventing Chat from sending messages while an agent-owned screen remains open outside a peek.
Description check		The description includes the required Change and Validation sections. It explains the problem, resulting behavior, linked issue, implementation scope, regression coverage, test results, and known vali…
Linked Issues check		Issue #1552 asks that Chat not type a send into an agent overlay it cannot read (Codex transcript search, Claude /config). It asks for an honest held state that keeps the draft. Implementation at hea…
Out of Scope Changes check		Changed files between merge base 28c7c8b and head 362c4bb: - src/lib/usePeekReturn.ts: the core inferred-guard logic for #1552. - src/lib/usePeekReturn.hook.test.ts: unit tests for that logic. - scr…

Full details: Description check

Explanation

The description includes the required Change and Validation sections. It explains the problem, resulting behavior, linked issue, implementation scope, regression coverage, test results, and known validation limits.

Full details: Linked Issues check

Explanation

Issue #1552 asks that Chat not type a send into an agent overlay it cannot read (Codex transcript search, Claude /config). It asks for an honest held state that keeps the draft.

Implementation at head (src/lib/usePeekReturn.ts):

• New noteRow path infers a still-open screen from whole frames. It arms only when the agent is in SCREEN_OVER_ROW (claude, codex), the recognizer read this pane's empty row before ('ready'), the chat is mounted, no peek or guard is up, and the agent is not working or blocked.
• It raises the existing StillOpen guard with named:false. The composer then refuses the send and keeps the draft, using the existing unnamed still-open note.
• The guard clears on the first frame that shows the row again. A turn of the agent drops an inferred guard.
• Show terminal opens the Terminal lens when command is empty.

The recognizer that reads missing or covered rows is existing code (src/lib/inputRow.ts, from merged #1637). The PR relies on it for the Codex and Claude overlay readings.

Tests:

• src/lib/usePeekReturn.hook.test.ts drives the real hook with per-frame recognizer answers. It covers Claude 'gone' and Codex 'covered' arming, pi 'gone' not arming, no arming without a prior 'ready' or while 'blocked', dismissal and re-arming, and turn reset.
• scripts/terminal-autoswitch-demo-regression.ts adds a browser case at 1280 and 390 px that asserts the draft is kept and nothing is submitted.

Deviations, which do not change the requirement:

• The issue suggests the 'Not sent: ... answer it first' wording. The PR reuses the existing unnamed still-open notice text ('A screen is still open in the terminal' and 'Not sent: a screen is still open in the terminal.').
• Coverage is limited to Claude and Codex. The issue's 'or, generally' option is not implemented. The PR states the limit is deliberate, because other recognizers answer 'gone' for unknown layouts.

Residual uncertainty: the hook tests supply recognizer answers directly. Live Codex 0.163.0 transcript search and Claude Code /config were not driven. This review did not inspect inputRow.ts, so the real overlay readings are taken from the PR's stated evidence.

Full details: Out of Scope Changes check

Explanation

Changed files between merge base 28c7c8b and head 362c4bb:

• src/lib/usePeekReturn.ts: the core inferred-guard logic for #1552.
• src/lib/usePeekReturn.hook.test.ts: unit tests for that logic.
• scripts/terminal-autoswitch-demo-regression.ts: browser case for the same behavior.
• DESIGN.md and docs/agent-commands.md: one sentence each documenting the behavior.
• changelog.d/chat-holds-send-over-open-screen.md: changelog entry for the fix.

Each change supports the #1552 objective. The PR states it does not touch PaneTerminal.tsx, App.tsx, i18n, server/, or shared/. The reviewed summary is consistent with that. No change was found that is unrelated to the guard. The PR description's test count (6268) differs from the objectives text (6265). This does not affect scope.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (3 skipped: 3 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[devswha]

Merge pass: this changes when a typed Chat message is held or sent, so this comment records the review, as for queued input.

Gate review (reviewer on a different model than the author's):

• Tests: 69 requested tests, the typecheck and the built terminal-autoswitch-demo-regression pass. So do 29 more recognizer and frame-gate tests.
• Hook test against main: run against main's usePeekReturn.ts (swapped in memory), the new hook test fails with six assertion failures.
• No auto-send: a held message stays in the composer until the user sends it or presses Send anyway. Nothing is typed to close a screen, and no new server request or polling starts.

The reviewer's one blocker, overruled by the merge pass with reasons. Back in the chat by its lens, the new effect (usePeekReturn.ts ~188-192) calls inputRowNow() once even while the agent is working. That contradicts the PR's line "frames are read outside a peek only while the agent is not working". The read is harmless:

• Cost: it is a single read of the local xterm buffer on mount, not a per-frame read and not a request. The per-frame reads in onInputRowFrame are gated as described.
• It can't arm the guard: noteRow returns before arming while working/blocked (line ~149).
• "ready" is consistent: a ready read only records the seen row and ends the screen's episode, which a turn does anyway.

Follow-up nit: add the working check to that effect, or say "frame-by-frame reads" in the docs.

Not verified: live Codex search, Claude /config and native mirror panes. The browser case uses the demo's Claude /btw screen.

All required checks are green and no review thread is open.