fix(terminal): drop orphaned XTVERSION bodies; set TERM_PROGRAM on remote PTYs#7841
fix(terminal): drop orphaned XTVERSION bodies; set TERM_PROGRAM on remote PTYs#7841bbingz wants to merge 2 commits into
Conversation
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR injects Orca terminal identity environment variables into SSH and relay PTY spawn environments, with tests covering environment merging, deletion behavior, relay bridge variables, and Windows PATH handling. It also adds detection for frameless XTVERSION and OSC color reply bodies, filters them from renderer PTY input, and preserves routing for fully framed terminal replies. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/main/providers/ssh-pty-provider.test.ts (1)
54-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
ORCA_TERMINAL_IDENTITY_ENVconsistently in early tests.Lines 54-59 and 78-84 spell out
TERM_PROGRAM,COLORTERM, andFORCE_HYPERLINKindividually, while all subsequent tests (102+) use...ORCA_TERMINAL_IDENTITY_ENV. Spreading the constant here too keeps the assertions DRY and ensures a single update point if values change.♻️ Proposed refactor for lines 54-59
env: expect.objectContaining({ [POWERLEVEL10K_WIZARD_DISABLE_ENV]: 'true', - TERM_PROGRAM: 'Orca', - COLORTERM: 'truecolor', - FORCE_HYPERLINK: '1' + ...ORCA_TERMINAL_IDENTITY_ENV })
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d8230a5f-dd28-4b0e-aeec-e1a8c36d0b28
📒 Files selected for processing (7)
src/main/providers/ssh-pty-provider.test.tssrc/main/providers/ssh-pty-provider.tssrc/relay/pty-handler.tssrc/renderer/src/components/terminal-pane/pty-connection.test.tssrc/renderer/src/components/terminal-pane/pty-connection.tssrc/shared/terminal-query-reply.test.tssrc/shared/terminal-query-reply.ts
Address CodeRabbit on stablyai#7841: match daemon and always force TERM=xterm-256color on SSH/relay (avoid ambient TERM=dumb). Assert full identity env in tests including TERM and TERM_PROGRAM_VERSION.
Address CodeRabbit on stablyai#7841: match daemon and always force TERM=xterm-256color on SSH/relay (avoid ambient TERM=dumb). Assert full identity env in tests including TERM and TERM_PROGRAM_VERSION.
a03da2c to
0eeac67
Compare
|
Rebased onto latest |
Address CodeRabbit on stablyai#7841: match daemon and always force TERM=xterm-256color on SSH/relay (avoid ambient TERM=dumb). Assert full identity env in tests including TERM and TERM_PROGRAM_VERSION.
0eeac67 to
3e4e222
Compare
…mote PTYs Remote Grok can show `>|xterm.js(6.1.0-beta.287)` as the prompt when an XTVERSION query reply loses ESC framing or arrives late (stablyai#7839). Local PTYs set TERM_PROGRAM=Orca so Grok feature-gates without relying on XTVERSION alone; SSH/relay spawns did not. - Classify and drop frameless XTVERSION/OSC color reply bodies on onData - Seed TERM_PROGRAM/COLORTERM/FORCE_HYPERLINK on SSH + relay spawn env - Keep framed XTVERSION on the immediate send path (stablyai#7329) Fixes stablyai#7839
Address CodeRabbit on stablyai#7841: match daemon and always force TERM=xterm-256color on SSH/relay (avoid ambient TERM=dumb). Assert full identity env in tests including TERM and TERM_PROGRAM_VERSION.
3e4e222 to
aa27ee6
Compare
|
Superseded by #7944, which includes this XTVERSION and remote terminal fix along with the related Grok compatibility fixes and regression coverage. |
Summary
Fixes remote Grok showing
>|xterm.js(6.1.0-beta.287)as the prompt (same Orca build works locally).Root cause (investigation)
@xterm/xterm@6.1.0-beta.287. XTVERSION replies are:ESC P > | xterm.js(6.1.0-beta.287) ESC \CSI >0q) and has logic for “dropping stalled XTVERSION reply fragment”; when framing is lost, the printable body can land in the TUI input.TERM_PROGRAM=Orca(+ version /COLORTERM/FORCE_HYPERLINK). SSH/relay spawns did not, so remote Grok leans harder on XTVERSION over a higher-latency path.Related prior work: #7329 / #7736 (immediate query replies), #5523 (reply leak class), #6975 (Windows OSC body leak — same ESC-swallow shape).
Fix
isOrphanedTerminalQueryReplyBody+ drop ononData>|xterm.js(...)/ OSC color bodies as keystrokeswithRemoteCliBridgeEnv+ relaybuildSpawnEnvseed terminal identitysendInputImmediateScreenshots
No visual change (terminal identity / input sanitization only).
Testing
src/shared/terminal-query-reply.test.ts(framed beta XTVERSION + orphan cases)src/main/providers/ssh-pty-provider.test.ts(identity env on spawn)pty-connectiononData routing test extended for XTVERSION immediate + orphan droporigin/main: focused vitest 45 passed(
terminal-query-reply+ssh-pty-provider). Fullpty-connectionsuite load depends on renderer alias graph; core shared + SSH coverage green.AI Review Report
>\|xterm.js(...)/]10|11;...bodies — not prefixes of normal typing.TERMwhen already set (TERM = TERM \|\| xterm-256color).Security Audit
TERM_PROGRAM, version,COLORTERM, etc.) already used on local/daemon paths.Notes
origin/main(0eeac678a).