Repository navigation
Conversation
12bf37b to
fc2c6cd
Compare
fc2c6cd to
e2f655e
Compare
📝 WalkthroughWalkthroughThe change adds transaction ownership for PTY input across renderer, automation, and agent prompt writes. It adds generation fencing, bracketed-paste handling, bounded query-reply queues, and lifecycle invalidation. Agent prompt delivery now supports readiness policies and Merge Risk: 🔵 Low · up to The PR centralizes PTY input ownership and prompt/mailbox sequencing, but a flush failure can leave the terminal in bracketed-paste mode, and partially implemented controllers can lose their native transaction behavior. These are bounded edge-case correctness risks requiring explicit owner follow-up before or after merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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 (2)
src/main/runtime/orca-runtime.ts (1)
6219-6236: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHandle controllers that implement only one transaction method.
The condition installs the compatibility owner when either
beginInputTransactionorinvalidateInputTransactionsis missing. If a controller suppliesbeginInputTransactionbut notinvalidateInputTransactions, this replaces the controller's ownbeginInputTransactionand itswritewith the compatibility owner. The controller's native transaction implementation is then unreachable.Fill in only the missing method, or require both methods together.
♻️ Proposed fix
- if ( - controller && - (!controller.beginInputTransaction || !controller.invalidateInputTransactions) - ) { + if (controller && !controller.beginInputTransaction) { const write = controller.write.bind(controller)src/main/runtime/orca-runtime.test.ts (1)
977-993: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse
acknowledgeAgentPromptSubmitinsidemakePromptAwareWrite.Lines 988-990 duplicate the
'\r'handling already defined byacknowledgeAgentPromptSubmitat Line 175. Call that helper to keep one definition of the submit acknowledgement.♻️ Proposed refactor
function makePromptAwareWrite(runtime: OrcaRuntimeService) { return vi.fn((ptyId: string, data: string) => { if (data.includes('\x1b[201~')) { runtime.onPtyData(ptyId, '\x1b[?25h', Date.now()) } - if (data === '\r') { - runtime.onPtyData(ptyId, '\x1b]0;Codex working\x07', Date.now()) - } + acknowledgeAgentPromptSubmit(runtime, ptyId, data) return true }) }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3c45599e-8803-4f0c-9f25-5083852fc7b2
📒 Files selected for processing (34)
config/reliability-gates.jsoncsrc/main/ipc/pty-write-ipc-validation.test.tssrc/main/ipc/pty.tssrc/main/pty-input-transaction-lifecycle.tssrc/main/pty-input-transaction-state.tssrc/main/pty-input-transaction.test.tssrc/main/pty-input-transaction.tssrc/main/runtime/agent-prompt-delivery-outcome.test.tssrc/main/runtime/agent-prompt-delivery-outcome.tssrc/main/runtime/agent-prompt-delivery-state-machine.test.tssrc/main/runtime/agent-prompt-submission-runtime.test.tssrc/main/runtime/agent-prompt-submission-verification.tssrc/main/runtime/orca-runtime.test.tssrc/main/runtime/orca-runtime.tssrc/main/runtime/orchestration-mailbox-notification-consistency.test.tssrc/main/runtime/orchestration-mailbox-notification-test-harness.tssrc/main/runtime/orchestration-mailbox-transport-settlement.test.tssrc/main/runtime/orchestration/db/contract-constants.tssrc/main/runtime/orchestration/db/messages/message-inbox.tssrc/main/runtime/orchestration/db/schema/create-core-tables-sql.tssrc/main/runtime/orchestration/db/schema/migrate.tssrc/main/runtime/orchestration/mailbox-pointer-delivery-contract.tssrc/main/runtime/orchestration/mailbox-pointer-delivery-validation.tssrc/main/runtime/orchestration/mailbox-pointer-delivery.tssrc/main/runtime/orchestration/mailbox-pointer-redrive.tssrc/main/runtime/orchestration/mailbox-pointer-state.test.tssrc/main/runtime/orchestration/mailbox-pointer-state.tssrc/main/runtime/orchestration/mailbox-pointer-submit.tssrc/main/runtime/orchestration/types.tssrc/main/runtime/rpc/errors.test.tssrc/main/runtime/rpc/errors.tssrc/main/runtime/rpc/methods/orchestration-composed-workers.test.tssrc/main/runtime/rpc/methods/orchestration-federation-start-receipt.test.tssrc/main/runtime/terminal-send-stale-leaf-liveness.test.ts
💤 Files with no reviewable changes (1)
- src/main/runtime/orchestration/mailbox-pointer-submit.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
| private interruptOwner(ptyId: string, owner: InputOwner): boolean { | ||
| if ( | ||
| (owner.pasteOpen || owner.pasteMutationPending) && | ||
| !this.writeInput(ptyId, AGENT_PROMPT_BRACKETED_PASTE_END) | ||
| ) { | ||
| return false | ||
| } | ||
| owner.pasteOpen = false | ||
| owner.pasteMutationPending = false | ||
| if (!flushQueryReplies(owner, (reply) => this.writeInput(ptyId, reply))) { | ||
| return false | ||
| } | ||
| invalidateOwner(this.ownerByPtyId, ptyId, owner, 'terminal_input_superseded', (token, reason) => | ||
| invalidateTransactionToken(token, reason) | ||
| ) | ||
| return true | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clear paste state only after the paste-end write and the flush both succeed.
interruptOwner sets owner.pasteOpen = false and owner.pasteMutationPending = false at lines 286-287, then returns false when flushQueryReplies fails at line 288. The caller then reports a failed write, but the owner remains registered with paste state already cleared. A later interruptOwner call for the same owner skips the bracketed-paste-end write, so the terminal can stay inside an open paste frame.
Restore the previous paste state when the flush fails.
🐛 Proposed fix
owner.pasteOpen = false
owner.pasteMutationPending = false
- if (!flushQueryReplies(owner, (reply) => this.writeInput(ptyId, reply))) {
- return false
- }
+ if (!flushQueryReplies(owner, (reply) => this.writeInput(ptyId, reply))) {
+ owner.pasteOpen = pasteWasOpen
+ owner.pasteMutationPending = pasteMutationWasPending
+ return false
+ }Capture pasteWasOpen and pasteMutationWasPending before the paste-end write.
Summary
Validation
No new wire fields or host-receipt claims are introduced. Provider transport publication remains distinct from execution-host receipt.