Repository navigation
fix(terminal): a Windows terminal read no longer shows PowerShell's history suggestion as typed input - #29415
Conversation
|
Warning Review limit reachedYou'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 7 minutes. View limit details
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 |
3140ed7 to
7d0370c
Compare
|
Codex review (gpt-6.1-sol, high). Round 1 found 3 P2 issues, all fixed in 7d0370c: (1) visible-only TUI-idle probes now get the visible screen, not scrollback; (2) the provider fallback is bounded by the 750 ms visible-snapshot timeout; (3) the projection over-fetches by one screen of rows, so blank rows below the prompt don't consume the line limit (new test). Round 2 on 7d0370c: no actionable issues. |
…ReadLine's overwritten history prediction
…bes, and fill the line limit
7d0370c to
55186f5
Compare
ELI5
On Windows, PowerShell suggests a command while you type. It takes that suggestion from a history file that every PowerShell window on the machine shares, and draws it in faint grey after your cursor. Orca's default
orca terminal readkept that grey suggestion as if someone had typed it. A fresh terminal could therefore look as if another agent's command had been typed into it. Nothing was actually sent to the wrong terminal. The read now returns what the terminal really shows.What Changed
Before: on a Windows host, a pwsh terminal created with
orca terminal createcould give a defaultorca terminal readlike this, after an agent typed onlyWrite-Output OWN_MARKER:FOREIGN_LANE_COMMAND …is a command another PowerShell session on the same Windows account ran. PSReadLine (PredictionSource History,InlineView, which is the pwsh 7 default) showed it as a suggestion and then erased it as the user kept typing.After: the same read returns
PS C:\...> Write-Output OWN_MARKER,OWN_MARKER,PS C:\...>. The--screenread already returned this.Mechanism: on Windows, ConPTY does not pass the shell's text through. It re-sends the terminal as screen updates and moves the cursor with absolute positions (
ESC[row;colH) and erase-characters (ESC[nX). The text-stream tail behind a default read throws those away, so text that was drawn and then overwritten stays in the stream. That is how the old suggestion ended up joined in front of the real command. For default reads of a native Windows ConPTY terminal,withVisibleSnapshotFallback(src/main/runtime/terminal/output/provider-buffer-capture.ts) now returns the rendered tail instead. The source is the host's live terminal emulator, which already parses every byte, with scrollback up to the read limit. If that model is stale or still loading, the source is the terminal provider's snapshot. The result is labelledsource: 'screen'. The alternate-screen and blank-tail paths are unchanged. If no rendered rows are available, the read falls back to the stream. Reads on non-ConPTY terminals and--cursorreads are unchanged.Why
The ticket listed three possible causes: wrong output routing, history redraw corruption, or input delivered to the wrong terminal. I tested each on the paired Windows host named in the ticket (runtime
cf6d1083):Get-CimInstance … Get-Process …command glued in front of my own input.--screenon the same handle showed only my input.PredictionSource=HistoryAndPlugin,PredictionViewStyle=InlineView,HistorySaveStyle=SaveIncrementally. The shared history file contains the ticket'sXFERWIN_DONEcommand 10 times.config/scripts/capture-agent-pty-transcript.mjs, against a throwaway one-line history file. The recording now lives insrc/main/runtime/__fixtures__/. The suggestion is drawn in faint italic (ESC[97m ESC[2m ESC[3m) and erased withESC[1;65HandESC[13X.So the terminal handle, the pane, the subscription and the input path all target the right terminal. Only the default read misreports what is on screen. The foreign text is always a command and never its output, and it always starts with the prefix the agent had already typed. That pattern fits a history suggestion, not misrouting.
Alternatives considered:
cclclecleaclear-style stacking.The host's emulator already sees every byte in order, and the
--screenand alternate-screen reads already depend on it, so using it here adds no new mechanism.Residual, unchanged:
--cursorreads page through completed stream lines, which is a separate contract, so on ConPTY they can still include an erased suggestion. On a terminal reattached after an Orca restart, the ConPTY mark may not be set again, and those reads keep the old stream behavior.Linked Issue
Resolves Linear STA-10739 (seen while validating #29292; this PR does not change #29292).
Visual Proof
N/A. This changes the text that
orca terminal readreturns, not the UI. The before and after reads are above, and the test asserts them from the recorded bytes.Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
New
src/main/runtime/orca-runtime-tests/conpty-stream-read.spec.tsfeeds the recorded pwsh 7.6 ConPTY bytes into the runtime, then does a default read. It fails on main with the foreign command in front ofWrite-Output OWN_MARKER, and passes with this change. A control case confirms that a non-ConPTY terminal still returnssource: 'stream'output.src/main/runtime/orca-runtime.test.ts: 1388 passed, 1 skipped.pnpm tc:nodeandpnpm run check:code-quality:changedpass.Windows: reproduced and recorded on the paired Windows host (pwsh 7.6.7). The probe terminal used a temporary history file and is now closed.
Review
Codex review (
gpt-6.1-sol, high) results are in a PR comment.Agent skill upstream boundary
Notes
source: 'screen'value, which older clients ignore.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)