Repository navigation
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughQuick Commands now support validated desktop agent submission through local and remote PTY transports. Raw shell, non-target, insert-only, and multiline input behavior remains unchanged. An ordering lane serializes typed input, accepted writes, and Quick Commands. Runtime routing validates pane, worktree, tab, leaf, PTY, connection, and lifecycle identity. Negotiated Kitty CSI-u submission uses bracketed paste for eligible agents and falls back when confirmation or protocol support is unavailable. Tests cover unit, integration, and Electron E2E behavior. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description provides a clear summary, reproduction details, implementation scope, validation results, and a known testing gap. However, it omits several required template sections, including the ELI5, What Changed, Why, Linked Issue with a Resolution Update the description to include all required template sections. Add the linked issue using Full details: Docstring CoverageExplanation Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 31 files. (2 skipped: 2 too large.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts (1)
1244-1260: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBound the viewport-claim wait before flushing an empty input.
sendQuickCommandToRuntimeruns in the single-flight ordering lane and callssendInputAcceptedToRuntime(''). IfpendingViewportClaimremains set while no multiplexed stream exists,sendInputAcceptedToRuntimewaits indefinitely onviewportClaimReadyWaiters, which blocks subsequent typed input in the lane. Add a bounded wait for this claim, or skip the claim wait whendatais empty.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 49915e40-156f-429b-a1b4-e088efabb27e
📥 Commits
Reviewing files that changed from the base of the PR and between f72dcb9 and 4a9c3c62428dba2018f1cde33376ccfe0bbc5fd0.
📒 Files selected for processing (18)
config/reliability-gates.jsoncsrc/main/runtime/rpc/methods/terminal/terminal-send-method.tssrc/main/runtime/rpc/methods/terminal/unary-schemas.tssrc/main/runtime/rpc/terminal-agent-prompt-send.test.tssrc/renderer/src/components/terminal-pane/pty-transport-input-write.test.tssrc/renderer/src/components/terminal-pane/pty-transport-types.tssrc/renderer/src/components/terminal-pane/pty-transport.tssrc/renderer/src/components/terminal-pane/remote-runtime-pty-transport-input-coalescing.test.tssrc/renderer/src/components/terminal-pane/remote-runtime-pty-transport.tssrc/renderer/src/components/terminal-pane/terminal-input-ordering-lane.test.tssrc/renderer/src/components/terminal-pane/terminal-input-ordering-lane.tssrc/renderer/src/components/terminal-pane/terminal-quick-command-dispatch.test.tssrc/renderer/src/components/terminal-pane/terminal-quick-command-dispatch.tssrc/renderer/src/components/terminal-pane/use-terminal-pane-context-menu.tssrc/renderer/src/runtime/runtime-terminal-quick-command.test.tssrc/renderer/src/runtime/runtime-terminal-quick-command.tstests/e2e/terminal-quick-command-agent-submit.spec.tstests/tools/repro-terminal-send-submit.mjs
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.
4a9c3c6 to
7885322
Compare
This reverts commit 548ce7b.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/main/runtime/orca-runtime.test.ts (1)
17415-17749: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated negotiated-agent PTY scaffold into one local factory.
Six of the new tests repeat the same block:
setPtyControllerwith a recordingwrite,kill,getForegroundProcess, andconfirmForegroundProcess, thencreateTerminalwithlaunchAgent, then an optional\x1b[>Nunegotiation frame. Only the pty id, agent, flags, and confirmation behavior vary.A single factory in this file (for example
createNegotiatedAgentTerminal) that returns{ runtime, handle, writes }would remove about 120 duplicated lines and keep the per-test assertions as the only visible difference. Name it after the domain concept, not a generic role.As per coding guidelines: "check whether an existing implementation already does the job (or nearly does). Extend or generalize it instead of building a parallel version".
Source: Coding guidelines
src/main/runtime/rpc/methods/terminal/terminal-send-method.ts (1)
25-38: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a regression test for omitted
client.type.
TerminalSendapplies'desktop'whenclient.typeis omitted under Zod 4.4.3. TestquickCommand: true,text: 'echo x\r', andclient: { id: 'desktop-1' }; assert the parsed type is'desktop'and the Quick Command guard accepts the request.Source: MCP tools
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 553d3276-7d66-4980-9f49-e799fae0ee52
📒 Files selected for processing (23)
config/reliability-gates.jsoncsrc/main/daemon/headless-emulator.test.tssrc/main/daemon/headless-emulator.tssrc/main/providers/ssh-pty-provider.test.tssrc/main/providers/ssh-pty-provider.tssrc/main/runtime/orca-runtime.test.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/rpc/methods/terminal/terminal-send-method.tssrc/main/runtime/rpc/terminal-agent-prompt-send.test.tssrc/relay/pty-handler-spawn-admission.test.tssrc/relay/pty-handler.tssrc/relay/pty-shell-utils.test.tssrc/relay/pty-shell-utils.tssrc/relay/shell-profile-list.tssrc/renderer/src/components/terminal-pane/pty-transport-input-write.test.tssrc/renderer/src/components/terminal-pane/pty-transport.tssrc/renderer/src/components/terminal-pane/remote-runtime-pty-transport-input-coalescing.test.tssrc/renderer/src/components/terminal-pane/remote-runtime-pty-transport.tssrc/shared/agent-prompt-injection.test.tssrc/shared/agent-prompt-injection.tssrc/shared/terminal-kitty-keyboard-flags.tstests/e2e/terminal-quick-command-agent-submit.spec.tstests/tools/repro-terminal-send-submit.mjs
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/tools/repro-terminal-send-submit.mjs (1)
289-298: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winGate CSI-U parsing on
negotiateCsiU.When
--negotiate-csi-uis absent,fakeAgentMainstill acceptsCSI_U_ENTERand completes submission. A fallback test can therefore pass after the runtime sends CSI-U to an unnegotiated agent. Skip this parser unlessnegotiateCsiUis true; otherwise accept only\r.This protects the mixed-version fallback path described in the PR objectives.
Also applies to: 359-378
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: aced2029-a276-4d48-9d5a-992be49b1de1
📒 Files selected for processing (9)
src/main/runtime/orca-runtime.test.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/rpc/terminal-agent-prompt-send.test.tssrc/relay/pty-shell-utils.test.tssrc/relay/pty-shell-utils.tssrc/renderer/src/components/terminal-pane/remote-runtime-pty-batching.tssrc/renderer/src/components/terminal-pane/remote-runtime-pty-transport-stream-reconnect.test.tssrc/renderer/src/components/terminal-pane/remote-runtime-pty-transport.tstests/tools/repro-terminal-send-submit.mjs
Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review.
| if ( | ||
| pendingViewportClaim && | ||
| !getCurrentMultiplexedStream(targetHandle) && | ||
| (data.length > 0 || inputBatcher.hasPending()) | ||
| ) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Wait when pendingClaimInput contains earlier input. If regular input has moved into pendingClaimInput, inputBatcher.hasPending() is false. An empty Quick Command then bypasses this wait and can submit before that earlier input flushes after stream installation. Include pendingClaimInput.length > 0 in the condition.
Proposed fix
- (data.length > 0 || inputBatcher.hasPending())
+ (data.length > 0 || inputBatcher.hasPending() || pendingClaimInput.length > 0)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ( | |
| pendingViewportClaim && | |
| !getCurrentMultiplexedStream(targetHandle) && | |
| (data.length > 0 || inputBatcher.hasPending()) | |
| ) { | |
| if ( | |
| pendingViewportClaim && | |
| !getCurrentMultiplexedStream(targetHandle) && | |
| (data.length > 0 || inputBatcher.hasPending() || pendingClaimInput.length > 0) | |
| ) { |
…nter-submit # Conflicts: # src/main/runtime/orca-runtime.test.ts # src/main/runtime/orca-runtime.ts # src/main/runtime/rpc/terminal-agent-prompt-send.test.ts
Summary
Quick Commands configured with “press Enter after” now submit text correctly when the exact foreground pane is a Claude/Codex TUI, while preserving raw shell and other terminal-application semantics.
The fix:
quickCommandintent toterminal.send;command + CRfor shells and unrecognized applications;Reproduction
A deterministic fake PTY/TUI oracle on latest
origin/mainand with the dispatch path disabled receives one rawprompt + CR, reportsprematureEnters=1, and does not submit.The candidate receives the exact PTY stream: bracketed paste (
ESC [200~prompt ESC [201~) followed byCR.The fake TUI reports
prematureEnters=0,receivedEnters=1, andsubmitted=true, and the rendered Electron terminal showsORCA_TERMINAL_SEND_REPORT ok.Validation
pnpm tc:webpnpm tc:nodetests/e2e/terminal-quick-command-agent-submit.spec.tsgit diff --checkNo sleeps, retries, global key synthesis, or unrelated prompt-delivery changes are used. Known evidence gap: no live SSH or physical Windows/Linux journey; shared contracts and deterministic tests cover those paths.
Related but distinct reports reviewed: #14525, #14347, #14608, and #14962.