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:
📝 WalkthroughWalkthroughThe changes centralize TUI launch command resolution and add shell-specific Windows command-length handling. Shell-ready startup delivery now requires explicit configuration, while Codex draft readiness requires both prompt signals. Startup and PTY draft delivery paths now return delivery outcomes, release failed attempts, and requeue retries when launch state remains valid. Related tests cover positional Codex prompts, readiness scanning, retry behavior, PTY handoffs, and paste timing. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 055ebc34-01d0-4a97-a4af-01df44c25192
📥 Commits
Reviewing files that changed from the base of the PR and between ff8192a and 5b36bc66b822c266221c3ca474559f5a404c3b57.
📒 Files selected for processing (22)
src/main/daemon/pty-subprocess.test.tssrc/main/ipc/pty.test.tssrc/main/runtime/orca-runtime.test.tssrc/relay/pty-handler.test.tssrc/renderer/src/components/terminal-pane/pty-connection.test.tssrc/renderer/src/components/terminal-pane/pty-connection.tssrc/renderer/src/lib/agent-paste-draft.test.tssrc/renderer/src/lib/agent-paste-draft.tssrc/renderer/src/lib/agent-startup-delayed-delivery-perf.test.tssrc/renderer/src/lib/agent-startup-delayed-delivery.tssrc/renderer/src/lib/launch-agent-background-session.test.tssrc/renderer/src/lib/new-workspace.test.tssrc/renderer/src/lib/new-workspace.tssrc/shared/agent-draft-platform-limit.tssrc/shared/codex-startup-delivery.test.tssrc/shared/codex-startup-delivery.tssrc/shared/draft-paste-ready-scanner.test.tssrc/shared/draft-paste-ready-scanner.tssrc/shared/tui-agent-config.tssrc/shared/tui-agent-launch-command.tssrc/shared/tui-agent-startup.test.tssrc/shared/tui-agent-startup.ts
5b36bc6 to
cde44f4
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
e74e1b2 to
606dbc0
Compare
|
Rebased onto latest Conflict resolution notes:
|
b4e05d2 to
aa212c4
Compare
|
Rebased onto current |
ebec2b6 to
7e1c571
Compare
Greptile SummaryThis PR recovers Codex startup draft delivery after the removal of
Confidence Score: 5/5Safe to merge. The changes are tightly scoped to startup/draft delivery, introduce no new external dependencies, and the boolean-compat wrappers keep all existing callers working without modification. The outcome-type refactor is internally consistent: Files Needing Attention: No files require special attention. The large
|
| Filename | Overview |
|---|---|
| src/renderer/src/components/terminal-pane/pty-connection.ts | Split startupDraftPasteAttempted into three flags; handOffFailedStartupDraftPaste correctly distinguishes same-PTY retry from replacement-PTY retry; dispose during in-flight correctly defers release until the async settles. |
| src/renderer/src/lib/new-workspace.ts | Refactors deliverAgentStartupToTerminal to return AgentStartupDeliveryOutcome; strips followupPrompt from retry to prevent double-submission; immediate retry then state-change-gated re-queue prevents spin loops. |
| src/renderer/src/lib/agent-startup-delayed-delivery.ts | New requeueFailedAgentStartupDelivery gates the toast on tab/launch-token expiry; uncertain outcomes keep the delivery consumed to prevent duplicate replay. |
| src/renderer/src/lib/agent-draft-paste-content.ts | Chunk-level deliveryStarted flag correctly distinguishes zero-write from partial-write failures; backward-compatible boolean wrapper preserved. |
| src/shared/draft-paste-ready-scanner.ts | Codex readiness now requires both the › glyph AND the Ask Codex placeholder after DECSET 2004; redundant first observeCodexMarkers(data) call is harmless. |
| src/shared/tui-agent-startup.ts | Adds Windows oversized-argv fallback routing oversized prompts to followupPrompt instead of truncating. |
| src/shared/tui-agent-draft-launch-plan.ts | Extracted buildAgentDraftLaunchPlan; removed dead codex-specific startupCommandDelivery: 'shell-ready' inside draftPromptFlag block. |
| src/shared/codex-startup-delivery.ts | Removes hasCodexNativeDraftFlag and the --prefill tokenizer; function now checks only the explicit startupCommandDelivery field. |
| src/shared/agent-draft-platform-limit.ts | Adds tighter 7,500-char limit for cmd shell to respect 8,191-char cmd.exe cap. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[ensureAgentStartupInTerminal] --> B{PTY available?}
B -- No --> C[queuePendingAgentStartupDelivery]
B -- Yes --> D[beginAgentStartupDeliveryAttempt]
D --> E[deliverAgentStartupToTerminal]
E --> F{followupPrompt?}
F -- Yes --> G[sendFollowupPromptWhenAgentReadyWithOutcome]
G -- delivered --> H[strip followupPrompt from remainingStartup]
G -- not-written --> R1[retryable]
G -- delivery-uncertain --> U[consumed, no toast]
H --> I{draftPrompt?}
F -- No --> I
I -- Yes --> J[pasteDraftToAgentPtyWhenReadyWithOutcome]
J -- delivered --> OK[kind: delivered]
J -- not-written --> R2[retryable, followupPrompt cleared]
J -- delivery-uncertain --> U
I -- No --> OK
R1 & R2 --> RQ[release + queuePendingAgentStartupDelivery]
RQ --> FD[immediate retry via flush]
FD -- not-written again --> RF[requeueFailedAgentStartupDelivery]
RF -- tab gone / token stale --> T[showAutomationPromptNotSentToast]
RF -- guards pass --> WAIT[wait for store state change]
Reviews (2): Last reviewed commit: "test(agents): hoist startup toast mock" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
7e1c571 to
a22ea63
Compare
a22ea63 to
bffe3f8
Compare
|
Closing as part of fork PR backlog cleanup. This branch is long-conflicted (DIRTY) against current |
3 participants
Summary
Replacement slice for #7950, limited to Codex startup commands and draft delivery.
codex --prefillassumption with explicit positional-prompt startup planningnot-writtenfromdelivery-uncertain: retry only when no bytes were accepted, never replay partial writes or a request whose acknowledgement was lostWhy this is separate
#7950 combined unrelated hook, auth/rate-limit, session/native-chat, and startup behavior. This PR contains only the startup/drafts slice so its delivery ownership and side-effect boundary can be reviewed and reverted independently.
Correctness and compatibility
Testing
pnpm typecheckpnpm lintoxfmt --checkpnpm check:max-lines-ratchetgit diff --check origin/main...HEADAI Review Report
The first independent gate approved the original startup split. After submission, CodeRabbit correctly identified that a boolean failure could represent either zero writes or partial delivery. The fix introduces an explicit side-effect-aware outcome through the paste/follow-up and delayed-delivery boundary.
A second independent gate verified that only
not-writtenrequeues; partial chunks, lost acknowledgements, and rejected Promises do not replay; successful follow-up state is removed from a draft retry; and dispose/remount cannot release an uncertain claim. Its only follow-up was to avoid the existing manual-paste toast for uncertain delivery; that was fixed and the final re-gate ended APPROVED.Security Audit
Generated task text is never replayed when delivery may already have occurred. The change adds no credentials, commands, IPC methods, dependencies, or persisted data, and keeps the existing PTY/runtime ownership checks.
Notes
The full 430-test PTY file had one unrelated timer-sensitive Droid assertion fail in the combined run; that exact test passed immediately in isolation. No assertion or production behavior was weakened. Validation used Node 26.5.0 while the repository declares Node 24.
ELI5
Codex startup could fail to deliver the first prompt/draft (especially on Windows or after remounts) after the old
--prefillpath went away. Startup planning, paste-when-ready, and ownership across dispose/remount are fixed so drafts actually reach the composer.