Repository navigation
fix(runtime): let a settled screen clear a blocked agent prompt the line tail kept after the TUI erased it - #33485
Conversation
… not an erased dialog in the tail The retained line tail keeps a dialog a TUI has erased in place, so an idle Codex composer kept refusing sends with agent_prompt_blocked whenever no hook or OSC 7501 status cleared the stale stamp. Read blocked text off the live emulator grid once it has applied every PTY chunk at the PTY's own size, and keep the tail as the fallback otherwise. Fixes #24187 Co-authored-by: Wooseong Kim <2222333+innocarpe@users.noreply.github.com>
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check |  |
[#24187] has two groups of coding requirements. (1) Re-check blocked rules against the live screen when a send would be refused, so a stale wait does not persist on an idle pane. The PR meets this. `r… |
Add the matched blocked rule and the matched line to the agent_prompt_blocked error and to agentWait output, or have the [#24187] owner explicitly accept the live-screen re-check as the complete resolution for the diagnostic request. |
-
Check name Status Explanation Title check The title clearly describes the primary runtime fix: allowing a settled screen to clear a stale blocked prompt. Description check The description is detailed and covers the user impact, root cause, implementation, testing, risks, and linked issue. It omits several template sections, including visual proof or an explicit N/A, com… Out of Scope Changes check All changed production files support the [ #24187] fix.blocked-tail-screen-clear.tsandblocked-text-layer.ts(wholeTextoption) add the whole-screen blocker check. `runtime-terminal-agent-statu…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. Analyzed 3 functions across 9 files. Full details: Description check
-
Explanation
The description is detailed and covers the user impact, root cause, implementation, testing, risks, and linked issue. It omits several template sections, including visual proof or an explicit N/A, completed checklist items, and review notes, but it is mostly complete and relevant.
Full details: Out of Scope Changes check
-
Explanation
All changed production files support the [
#24187] fix.blocked-tail-screen-clear.tsandblocked-text-layer.ts(wholeTextoption) add the whole-screen blocker check.runtime-terminal-agent-status-query.tsandruntime-service-state.tswire the screen read intowaitText.terminal-visible-state.tsandruntime-construction-services.tsexpose the caught-up screen read.terminal-wait-blocked-checks.tsclears the stalewaitBlockedAtstamp so the cleared state holds for the send.terminal-buffer-operations.tssetsoutputSequenceafter the seed lands, which the caught-up check depends on. The test file adds Codex and OpenCode regression cases for this behavior. No unrelated behavior change is evident. The Claude seed timing effect is listed as a risk in the PR description. It is a side effect of the in-scope seed change, not an unrelated change.
-
Full details: Linked Issues check
-
Explanation
[
#24187] has two groups of coding requirements. (1) Re-check blocked rules against the live screen when a send would be refused, so a stale wait does not persist on an idle pane. The PR meets this.readWaitTextinruntime-terminal-agent-status-query.tscallsreadScreenClearingBlockedTailonly when the retained tail says blocked.blocked-tail-screen-clear.ts(screenClearsBlockedTail) clears the block only when no blocker appears on the whole screen and the agent's screen rules returnidle. The PR also adds a regression test for an erased Codex dialog. (2) Include the matched rule and matched line in theagent_prompt_blockederror and inagentWait. The PR does not meet this. The changed files do not touch the send error or theagentWaitshape, which stays{source, reason, since}. The PR description also says this part is not addressed. A supported stale-wait clear path exists only for a settled, quiet pane with a ready composer. Within 3 s of the last output, the tail still decides, so this path does not cover every case.Resolution
Add the matched blocked rule and the matched line to the
agent_prompt_blockederror and toagentWaitoutput, or have the [#24187] owner explicitly accept the live-screen re-check as the complete resolution for the diagnostic request.
Finishing Touches
-
📝 Generate docstrings
-
- 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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
-
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
83cb4f75-1b27-4507-9469-1fd367cd39f6
-
src/main/runtime/agent-prompt-submission-runtime.test.tssrc/main/runtime/construction/runtime-construction-services.tssrc/main/runtime/construction/runtime-service-state.tssrc/main/runtime/runtime-terminal-agent-status-query.tssrc/main/runtime/terminal/output/terminal-visible-state.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.
-
| return null | ||
| } | ||
| const lines = this.readRuledScreen(ptyId)?.lines | ||
| return lines && lines.length > 0 ? lines.join('\n') : null |
There was a problem hiding this comment.
🔎 Supported by static analysis
-
Script executed:
sed -n '175,220p' src/main/runtime/terminal/output/terminal-buffer-operations.ts rg -n 'outputSequence:|outputSequence =|getPtyOutputSequence|seedHeadlessTerminal\\(' src/main/runtime/terminal src/main/runtime/terminal/output
Repository: stablyai/orca
Length of output: 2422
Script executed:
# Locate the initializer, the PTY sequence source, and all outputSequence writes/reads (fixed-string search, no regex). rg -n -F -e 'createPtyHeadlessTerminalState' -e 'getPtyOutputSequence' -e 'outputSequence' -- src/main/runtime || test "$?" -eq 1 # Show the headless state factory with surrounding context. rg -n -F -A 40 -e 'createPtyHeadlessTerminalState(' -- src/main/runtime || test "$?" -eq 1 # Show the reader as currently committed. sed -n '170,220p' src/main/runtime/terminal/output/terminal-visible-state.ts
Repository: stablyai/orca
Length of output: 37362
Return '' for a painted blank grid, except while a seed write is pending.
If Codex erases a dialog and leaves a blank grid, readRuledScreen returns lines: []. This line returns null, so getSnapshot falls back to the retained tail. The tail can still contain the erased dialog and keep sends refused.
The fix needs a guard for the seed window. seedHeadlessTerminal stores the fresh emulator before its seed write applies, so the emulator is blank during that window. Moving the outputSequence assignment does not close the window. createPtyHeadlessTerminalState initializes outputSequence to 0, and a PTY with no output reports 0. In that case the sequences match before the seed write, so the reader would return '' for a blank grid that is not yet caught up. Track the pending seed instead of the sequence.
Declare pendingSeedStates beside the headlessTerminals field.
Suggested fix
-
--- a/src/main/runtime/terminal/output/terminal-buffer-operations.ts +++ b/src/main/runtime/terminal/output/terminal-buffer-operations.ts @@ -202,4 +202,5 @@ const state = this.createPtyHeadlessTerminalState(ptyId, dims) state.outputSequence = this.getPtyOutputSequence(ptyId) + this.pendingSeedStates.set(ptyId, state) this.headlessTerminals.set(ptyId, state) @@ -214,2 +215,5 @@ await state.emulator.write(data) + if (this.pendingSeedStates.get(ptyId) === state) { + this.pendingSeedStates.delete(ptyId) + } if (this.headlessTerminals.get(ptyId) !== state) {
--- a/src/main/runtime/terminal/output/terminal-visible-state.ts +++ b/src/main/runtime/terminal/output/terminal-visible-state.ts @@ -177,8 +177,12 @@ protected readCaughtUpTerminalScreenText(ptyId: string): string | null { - if (this.headlessTerminals.get(ptyId)?.outputSequence !== this.getPtyOutputSequence(ptyId)) { + const state = this.headlessTerminals.get(ptyId) + if ( + !state || + this.pendingSeedStates.get(ptyId) === state || + state.outputSequence !== this.getPtyOutputSequence(ptyId) + ) { return null } const lines = this.readRuledScreen(ptyId)?.lines - return lines && lines.length > 0 ? lines.join('\n') : null + return lines ? lines.join('\n') : null }
…r decide it alone Review of the first cut: the blocked matcher scans the last 12 non-blank lines, and on a grid every row counts, so a 15-row OpenCode 2 permission dialog read as unblocked and the prompt + Enter approved it. The tail now decides as on main; the screen may lift its verdict only when it is caught up, quiet for TUI_IDLE_QUIESCENCE_MS, shows no blocker on any row, and the agent's own screen rules see its ready composer. A daemon re-attach seed now marks the grid current only after the seed lands. Co-authored-by: Wooseong Kim <2222333+innocarpe@users.noreply.github.com>
nwparker
changed the title
… dialog erased Our own paste echo ended the quiet window, so the pre-Enter check fell back to the tail and the old stamp refused the send with the paste written and no Enter. A screen clear now also drops waitBlockedAt on the PTY and its mirroring leaves; a newly painted blocker stamps again. Co-authored-by: Wooseong Kim <2222333+innocarpe@users.noreply.github.com>
ELI5
Orca kept refusing to send a prompt to an idle Codex pane with
agent_prompt_blocked, even though the screen showed only the empty composer. It was reading a permission dialog that Codex had already erased: Orca's line-by-line copy of past output still had it. Now, if that copy says "blocked", Orca double-checks the live screen. The screen can lift the block only when it is fully up to date, the pane has gone quiet, no dialog wording appears anywhere on screen, and the agent's own ready composer is showing. Otherwise Orca behaves exactly as it does today.Root cause
detectTerminalWaitBlockedReasonreadssnapshot.waitText, which is built from the retained line tail (buildTerminalWaitText). Codex draws and erases dialogs in place by cell diff (\x1b[row;colH,\x1b[J). A line tail can't undo that, so an answered dialog stays inside the 12-line window thatblocked-text-layer.tstreats as live.waitBlockedAtstamp, and the stamp only lifts when a later hook, lifecycle event, or OSC 7501 status clears it. Codex 0.154, the reporter's version, never emits OSC 7501:stringson the@openai/codex@0.154.0-darwin-arm64binary finds no7501. The reporter's Stop hook also failed ("invalid stop hook JSON output"), so every later send was refused. Codex 0.164 sends7501;state=done, which hides the bug only while hooks or 7501 keep working.What changed
RuntimeTerminalAgentStatusQuery.readWaitText: the retained tail still decides. Only when the tail says blocked canreadScreenClearingBlockedTailswap in the screen text, and only if all of these hold:headless.outputSequence === ptyOutputSequence) and passes the existingreadRuledScreenchecks: whole-screen model, drawn at the PTY's own size, no unrepainted reflow;TUI_IDLE_QUIESCENCE_MS(3 s), so a frame caught mid-paint can't clear;findTerminalWaitBlockedSignal(screen, { wholeText: true })finds nothing on any row (new option; the 12-line window stays for tails);idle. For Codex that iscodex-composer-ready; Codex dialogs replace the composer. Agents without screen rules never clear here.waitBlockedAtstamp on the PTY and its mirroring leaves. Without that, the paste's own echo in the composer would end the quiet window, the pre-Enter check would fall back to the tail, and the old stamp would refuse the send after the paste was already written. A newly painted blocker stamps again. None of the agents that can clear useagent-approval-prompt, the one reason that blocks without a stamp; onlycursor.jsondoes.seedHeadlessTerminal:outputSequenceis now set inside the write chain after the seed lands, so a daemon re-attach can't pass for a current grid before it's filled.waitTextsees this: the send gate,agentWaitinterminal show,getTerminalAgentStatus, and tui-idle's hook-lane blocked check. No wire or contract change.How verified
CODEX_HOME, in a 120x40 PTY with-a on-request -s read-only. The real approval dialog ("Would you like to run the following command? … Press enter to confirm or esc to cancel") was answered withy, and I replayed the captured bytes throughOrcaRuntimeService. On main, the idle pane showsagentWait prompt-text/agent-interactive-promptand the send fails withagent_prompt_blocked, 0 writes. With this PR the prompt is written. While the dialog is still open, the send is refused.src/main/runtime/__fixtures__transcript with meta, in 64-char frames viareplayTranscript:freebuff-ready, frame 9 ofantigravity-dialog-trust-workspace, and frame 558 ofcodex-0-160-0-plan-implement-menu. That last one is the half-painted plan menu.agent-prompt-submission-runtime.test.ts:permission.tsx:┃border on every row,maxHeight: 15, "Permission required" on row 2): refused once settled. Passes on main, fails on the first cut of this PR.pnpm tcpasses.pnpm test src/main/runtime src/shared: 30640 passed. The 6 failures (editor-view-layout-apply, restored-editor-move-restart, sweep-terminal-placement, orca-runtime folder import) fail the same way on cleanorigin/main.check:code-quality:changedpasses.donehides the stamp.Risk
unknown-pane.json's Codex-header rule. Everyone else keeps main's behavior.outputSequenceonly after it lands, Claude's interactive-wait screen read fails its sequence check in the window before the seed lands. A daemon-reattached Claude pane can therefore report the wait as unconfirmed (undefined) until the seed lands, instead of reading an empty grid.Credit
Supersedes #24308 by @innocarpe. It keeps that PR's core idea (check the current screen before refusing a send) and drops the stamp-clearing machinery. Co-authored in the commits.
Fixes #24187