Repository navigation
fix(native-chat): open a New Chat's composer at once, before its host answers - #29358
Conversation
…er focus The redesign opens the new chat's editor at the action and lets the ordinary at-rest create run in the background, so the local draft record, saved first submission, reserved first-Send sender and pre-session upload capability are no longer needed.
…t-draft-first-send
… answers The new chat's tab and composer waited for the host to admit the chat, so keys typed right after New Chat went to whatever had focus. A New Chat the user asked for now opens its chat tab immediately on the host it was routed to, with focus in the composer. Its ordinary launch asks the host in the background, the same probe it already ran before create, and the chat shows that launch's starting, failed and Retry states. - A launch that carries a prompt still asks first, so a refusal hands the prompt to a terminal as before. - A refused chat nobody typed in closes and its terminal opens in its place; a chat with typing keeps it. - A paired host that would run the chat with more access than asked for gets no create; the chat's permission notice is the only one shown. - A chat closed while its host was being asked issues no create. - While the chat starts on a paired server, uploads wait and the model picker stays disabled until the server names what it seeds.
brennanb2025
changed the title
…t-draft-first-send # Conflicts: # src/renderer/src/components/native-chat/use-native-chat-external-attachments.ts # src/renderer/src/lib/structured-agent-launch-settlement.ts # src/renderer/src/lib/structured-agent-session-launch.ts # src/renderer/src/lib/structured-agent-session-provisional-tab.ts
…s chat rows - The + menu hands focus to the new chat's composer when it closes, instead of a frame later. - A chat picked by name that its host refuses stays a chat and says why, including the WSL or SSH reason; it never becomes a terminal. - A chat the user typed in before its host refused keeps its text, and the agent's terminal opens behind it rather than taking the screen; a paired server's refusal says a terminal opened. - The paired permission check runs once, right before create, for every launch; the chat's own permission notice is the only message. - A chat closed while its host was being asked shares the existing pre-create gate, so nothing is created. - Session attachment notice copy lives with its only caller; tests that mirrored the implementation are trimmed.
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 53.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 58 functions across 52 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the primary change: opening a New Chat composer before the host responds. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The pull request description is detailed and covers the required ELI5, changes, rationale, issue references, visual proof, testing, review, scope boundary, and compatibility notes. It does not include… Full details: Description check
-
Explanation
The pull request description is detailed and covers the required ELI5, changes, rationale, issue references, visual proof, testing, review, scope boundary, and compatibility notes. It does not include the template’s explicit Checklist section or a
Fixes #...line, but it provides issue references and substantial validation evidence, so the description is mostly complete.
-
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
…t-draft-first-send # Conflicts: # src/renderer/src/lib/structured-agent-session-launch-failure.ts
![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
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
13fd60d8-a5fd-45b3-b8fa-3785da1b52fc
📒 Files selected for processing: 57
src/renderer/src/components/native-chat/NativeChatStructuredSession.tsxsrc/renderer/src/components/native-chat/native-chat-attachment-upload.tssrc/renderer/src/components/native-chat/native-chat-composer-reveal-focus.test.tsxsrc/renderer/src/components/native-chat/native-chat-composer-types.tssrc/renderer/src/components/native-chat/native-chat-pending-attachment-cache.tssrc/renderer/src/components/native-chat/native-chat-structured-session-notices.tssrc/renderer/src/components/native-chat/structured-agent-session-status-bridge-unconfirmed-start.test.tsxsrc/renderer/src/components/native-chat/use-native-chat-composer-paste.tssrc/renderer/src/components/native-chat/use-native-chat-composer-reveal-focus.tssrc/renderer/src/components/native-chat/use-native-chat-external-attachments.test.tsxsrc/renderer/src/components/native-chat/use-native-chat-external-attachments.tssrc/renderer/src/components/native-chat/use-native-chat-host-permission-gate.tssrc/renderer/src/components/native-chat/use-native-chat-provisional-launch.test.tsxsrc/renderer/src/components/native-chat/use-native-chat-provisional-launch.tssrc/renderer/src/components/native-chat/use-native-chat-structured-composer-transport.test.tsxsrc/renderer/src/components/native-chat/use-native-chat-structured-composer-transport.tssrc/renderer/src/components/native-chat/use-structured-agent-session-options.tssrc/renderer/src/components/native-chat/use-structured-native-chat-submit-reveal.test.tssrc/renderer/src/components/tab-bar/QuickLaunchButton.tsxsrc/renderer/src/components/tab-bar/use-tab-bar-create-menu-controller.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/lib/launch-agent-in-new-tab-structured-route.tssrc/renderer/src/lib/launch-agent-in-new-tab-web-runtime.test.tssrc/renderer/src/lib/launch-agent-in-new-tab.test.tssrc/renderer/src/lib/launch-agent-in-new-tab.tssrc/renderer/src/lib/launch-agent-structured-chat-guard.test.tssrc/renderer/src/lib/launch-agent-web-host-tab.tssrc/renderer/src/lib/launch-structured-agent-session.test.tssrc/renderer/src/lib/launch-structured-agent-session.tssrc/renderer/src/lib/structured-agent-launch-settlement.tssrc/renderer/src/lib/structured-agent-session-action-launch.tssrc/renderer/src/lib/structured-agent-session-launch-admission.test.tssrc/renderer/src/lib/structured-agent-session-launch-admission.tssrc/renderer/src/lib/structured-agent-session-launch-callers.tssrc/renderer/src/lib/structured-agent-session-launch-create-call.tssrc/renderer/src/lib/structured-agent-session-launch-create-message.test.tssrc/renderer/src/lib/structured-agent-session-launch-errors.tssrc/renderer/src/lib/structured-agent-session-launch-failure.tssrc/renderer/src/lib/structured-agent-session-launch-options.tssrc/renderer/src/lib/structured-agent-session-launch-permission-notice.tssrc/renderer/src/lib/structured-agent-session-launch-probe-outage.test.tssrc/renderer/src/lib/structured-agent-session-launch-recovery.tssrc/renderer/src/lib/structured-agent-session-launch-reload.test.tssrc/renderer/src/lib/structured-agent-session-launch-reload.tssrc/renderer/src/lib/structured-agent-session-launch.test.tssrc/renderer/src/lib/structured-agent-session-launch.tssrc/renderer/src/lib/structured-agent-session-provisional-tab.test.tssrc/renderer/src/lib/structured-agent-session-provisional-tab.tssrc/renderer/src/runtime/structured-chat-host-permission.test.tssrc/renderer/src/runtime/structured-chat-host-permission.tssrc/renderer/src/runtime/web-runtime-session-terminal-workspace-routing.test.tssrc/renderer/src/store/slices/tabs-label-and-pin-state.test.tssrc/renderer/src/store/slices/tabs/tabs-label-actions.tssrc/renderer/src/store/slices/tabs/tabs-slice-contract.tssrc/renderer/src/store/terminals/terminal-actions.tssrc/renderer/src/store/terminals/terminal-tab-appearance.tssrc/renderer/src/store/terminals/terminal-tab-layout-owner-actions.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.
|
Opening New Chat used to wait for its host before its composer existed, so typing during that wait could reach the previous surface. This PR opens and focuses the ordinary composer first. It also prevents a dismissed menu from taking focus back during its closing animation. The final correction makes older-host warnings wrap, keeping their full update instruction visible in a narrow pane. Review and bounded finishing QA complete on fb898ea. One reviewer checked only the final warning-wrap commit and found no new source defect; proportional readiness, performance, compatibility and platform follow-up were clear. Six design rounds and their rulings remain binding. No source changes, new design round or main sync occurred during this finish. Production changes total 798 lines against pinned main, within the 800-line cap. Fixed: composer availability before host admission; the closing-menu focus takeover; full visibility of the host warning. Kept intentionally: older hosts retain their existing launch path; an invested composer survives unsupported-host fallback beside a background terminal. Existing Split Right/Down moves an existing chat; fresh-chat Split remains the separate consumer tracked in #28950. Deferred: the immediate-typing order race in STA-11369, reproduced on main, and issued-close retirement tracked in #27583/#27561. No open P0/P1 or newly proved source defect remains in this bounded finish. Exact-head full PR workflow and its PR checks passed aggregate succeeded. Workflow identity is Current bounded Electron QA ran on the M4Air admitted by qa-gate, ticket 830, with real Codex and Claude through their load balancers under dedicated test homes:
The supplied finishing criterion passes because main reproduces at least once in each direction. These observations do not establish equal failure rates. The race is tracked in STA-11369 and the caret-restoration code is unchanged by this PR. The prior two-byte warm-relay accounting discrepancy was traced to the test relay dropping a held chunk when a connection closes; that is not a current product failure. Unsupported-host fallback and placement (Q3/Q6) retain explicitly accepted historical 2a18d27 evidence on unchanged paths, rather than being relabelled as current-head runs. All 55 recorded test processes exited, configured ports were clear, Playwright was closed, the old disk image was detached, real config hashes were unchanged, and the QA worktree was restored to the exact branch head with no tracked changes. Not exercised in this finish: strict 0–30 ms timing, cold cancel, reload/Retry, tours, image contents, fresh-chat Split creation, rendered direct SSH/WSL behavior, touch/pen or checkbox/radio/subtrigger variants. Standalone ContextMenuItem has source review only; both tested real menus use DropdownMenu. The Q4 diagnostic needed normal terminal layout established first on branch and the same actual folder recreated through its production owner after old-host restart on main; these setup observations are retained without claiming normal-menu Chat coverage on the old host. Both heads logged listener-count warnings; this is an observation, not a proven leak. Original branch launch isolation was preserved in the fixture but its live process environment was not separately audited before shutdown; corrected main/peer audits and unchanged real-config hashes are retained. For Brennan's review Current branch captures are fb898ea on the admitted M4Air. Main controls are pinned f884b9f. The complete old-host warning now wraps. Main cannot create the refused chat or show its inline notice with the same diagnostic; this is the explicitly permitted “main cannot show it” control, not a clipping comparison. Both normal old-host menus lack Chat, so this case uses the real production Chat entrypoint against a genuine 1.4.189 server.
The unchanged New Tab menu fixture kept its complete prompt and received a real Codex reply:
One uninterrupted direct paired-Claude run retained all 41 characters and received a real reply. These images show ready and reply, respectively; the earlier disabled Send/Attach pending state is established by the passive trace, not these images.
Matched existing-chat Split Right/Down: race reproduced on both headsSame machine, settings and per-trial prerequisites, with no draft repair. These are moves of existing chats, not fresh-chat Split creation. Representative reordered drafts below are main then branch. Right:
Down:
Right trial 1 on each head uses the original Attach-ready prerequisite. Right trials 2–10 add waiting for the real launch promise and a normal tab left-click on both heads; both Down runs use that same amended script. One branch menu detached before direction selection and before any burst; that prerequisite failure is retained outside the 40 measured trials. No wait or focus action is inserted between final direction selection and typing. Current menu dismissal and navigation evidenceBoth actual New Tab and tab right-click surfaces use DropdownMenu. Hover changed the highlight; ArrowDown then rendered Grok and Close respectively:
After outside mouse clicks through an inert Projects label locator, both menus are absent:
Escape also removes each menu:
Accepted historical Q3/Q6 evidence, unchanged paths onlyThese captures are 2a18d27, not current-head QA. The empty replacement keeps explicit pin/title/colour; a composer with user content remains beside the fallback terminal.
Two source locations for judgment:
|
…t-draft-first-send # Conflicts: # src/renderer/src/runtime/structured-chat-host-permission.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Retire the terminal created after cancellation. · structured-agent-session-action-launch.ts:79-87
src/renderer/src/lib/structured-agent-session-action-launch.ts:79-87
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRetire the terminal created after cancellation.
If the provisional chat closes while
openDeclinedTerminalis awaiting its dynamic import, the opener can create a terminal after cancellation. The cancellation path does not retain that tab ID. The current branch then continues without closing the terminal, which leaves an unwanted terminal tab.Close
terminal.tabIdimmediately after the opener resolves when the launch was cancelled. Do not await its completion after closing it.Suggested fix
const terminal = await openDeclinedTerminal(args, worktreeId, target, false) + if (launchState?.cancelled || args.hooks.signal?.aborted) { + if (terminal.tabId) { + useAppStore.getState().closeUnifiedTab(terminal.tabId, { recordInteraction: false }) + } + return settlement + } const completed = terminal.completion
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
87ef1b04-ef58-4696-9a91-d28708ac8c5d
📒 Files selected for processing: 3
src/renderer/src/components/native-chat/use-native-chat-composer-reveal-focus.tssrc/renderer/src/components/native-chat/use-native-chat-provisional-launch.test.tsxsrc/renderer/src/lib/structured-agent-session-launch-recovery.ts
🚧 Files skipped from review as they are similar to previous changes: 3
- src/renderer/src/lib/structured-agent-session-launch-recovery.ts
- src/renderer/src/components/native-chat/use-native-chat-composer-reveal-focus.ts
- src/renderer/src/components/native-chat/use-native-chat-provisional-launch.test.tsx
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.
…tem opened A New Chat chosen from the New Tab menu kept only the first typed key: the menu stays mounted while it animates out, and Radix focuses its content when the pointer leaves an item, so the remaining keys went to the closed menu. Dropdown and context menu items now cancel that pointer focus while their menu is closing. Trims three hunks back to main's form to stay in budget.
…ructions The paired-permission notice now shows on a New Chat as soon as it opens, and its "Update Orca on this host" instruction was cut off with an ellipsis and no tooltip. Host notices now wrap like every other composer notice.
…t-draft-first-send # Conflicts: # src/renderer/src/lib/structured-agent-session-launch-options.ts
ELI5
New Chat used to wait for the computer or paired server running the chat before its text box existed. Typing during that wait could reach the previous chat or another control. The new chat now opens and focuses its ordinary composer first, giving input a destination while its host answers.
What Changed
Eligible foreground New Chat actions open the real chat tab before host admission finishes. The existing conversation identity and saved draft remain in use. The empty host conversation is created in the background; modern hosts start the agent on the first Send. Automatic, resume, background and immediately submitted prompt launches retain their existing admission path.
Typing stays available during startup. Send waits for the host conversation, model and effort changes wait for the host defaults, and paired uploads wait for publication. Local attachment staging remains available. Permission refusal uses one notice with the requested agent's permission default. Host notices now wrap so the full recovery instruction stays visible in a narrow pane. An unverified permission result offers the existing Check again action.
A dismissed menu could move focus back to itself during its closing animation, leaving only the first typed character in the new composer. Shared dropdown and context menu items now skip that pointer-focus action while their own menu is closed.
Explicit Chat requests stay chats when refused. For a generic agent launch that cannot use structured chat, terminal fallback waits for an actual terminal before replacing an empty composer. The replacement keeps its position, pin, and any explicitly chosen title and colour. If text, images or pending attachments have arrived, the chat and its content stay and the terminal opens in the background. Completion does not navigate back after the user switches away or closes the source.
Split Right and Split Down relocate an existing chat in this head. Fresh-chat Split is a separate consumer tracked in #28950. A separate immediate-typing order race also occurs on main and is tracked in STA-11369; this PR does not correct that race.
Why
The structural problem was making the local editing destination depend on an asynchronous host answer. Opening the existing composer first removes that dependency without adding a second draft lifecycle. Conversation creation, process startup, saved drafts and terminal launch ownership are reused rather than adding saved state or remote operations.
The common pattern gives local editing its own destination before remote work completes. Intended differences are ordinary empty host-record creation at open, the existing provider-at-open behavior on older paired servers, and retaining a composer containing user content beside a fallback terminal. No new saved fields, wire operations, capabilities or minimum server version are introduced. Issued-close retirement remains tracked in #27583 and #27561.
Linked Issue
Continues the focus work tracked in #28987 and integrates the menu routing from #29074. The separate typing-order race is tracked in STA-11369.
Visual Proof
Current branch captures are fb898ea on the admitted M4Air. Main controls are pinned f884b9f.
The complete old-host warning now wraps. Main cannot create the refused chat or show its inline notice with the same diagnostic; this is the explicitly permitted “main cannot show it” control, not a clipping comparison. Both normal old-host menus lack Chat, so this case uses the real production Chat entrypoint against a genuine 1.4.189 server.
The unchanged New Tab menu fixture kept its complete prompt and received a real Codex reply:
One uninterrupted direct paired-Claude run retained all 41 characters and received a real reply. These images show ready and reply, respectively; the earlier disabled Send/Attach pending state is established by the passive trace, not these images.
Matched existing-chat Split Right/Down: race reproduced on both heads
Same machine, settings and per-trial prerequisites, with no draft repair. These are moves of existing chats, not fresh-chat Split creation. Representative reordered drafts below are main then branch.
Right:
Down:
Right trial 1 on each head uses the original Attach-ready prerequisite. Right trials 2–10 add waiting for the real launch promise and a normal tab left-click on both heads; both Down runs use that same amended script. One branch menu detached before direction selection and before any burst; that prerequisite failure is retained outside the 40 measured trials. No wait or focus action is inserted between final direction selection and typing.
Current menu dismissal and navigation evidence
Both actual New Tab and tab right-click surfaces use DropdownMenu. Hover changed the highlight; ArrowDown then rendered Grok and Close respectively:
After outside mouse clicks through an inert Projects label locator, both menus are absent:
Escape also removes each menu:
Historical 2a18d27, accepted only for the unchanged unsupported-host fallback and placement paths:
Testing
Exact-head full PR workflow and its PR checks passed aggregate succeeded. Workflow identity is
.github/workflows/pr.yml; all five unit shards, selected layout/relay checks and both packaging jobs passed.Current bounded Electron QA ran on the M4Air admitted by qa-gate, ticket 830, with real Codex and Claude through their load balancers under dedicated test homes:
OK_Q5_PENDINGin the same session.OK_Q1_MENU.The supplied finishing criterion passes because main reproduces at least once in each direction. These observations do not establish equal failure rates. The race is tracked in STA-11369 and the caret-restoration code is unchanged by this PR. The prior two-byte warm-relay accounting discrepancy was traced to the test relay dropping a held chunk when a connection closes; that is not a current product failure.
Unsupported-host fallback and placement (Q3/Q6) retain explicitly accepted historical 2a18d27 evidence on unchanged paths, rather than being relabelled as current-head runs. All 55 recorded test processes exited, configured ports were clear, Playwright was closed, the old disk image was detached, real config hashes were unchanged, and the QA worktree was restored to the exact branch head with no tracked changes.
Strict 0–30 ms timing, cold start/cancel, reload/Retry, tours and image contents were outside this finishing scope. The short pending state is evidenced by a passive event/DOM trace; its later screenshot shows the ready state. Both tested tab menus use DropdownMenu; ContextMenuItem has source review only. Physical touch/pen and checkbox/radio/subtrigger variants were not certified.
Review
Full path: lifecycle, asynchronous completion, ordering and mixed client/server behavior. Six design rounds and their rulings remain binding. The final one-commit review and proportional readiness follow-up found no new source defect. Production changes total 798 lines against pinned main, within the 800-line cap. No code changed during this finishing pass.
Agent skill upstream boundary
Notes
Folder workspaces and execution-host ownership remain supported by the source design. Direct SSH and WSL retain their existing routes; this finish does not certify their rendered behavior. Saved schemas and mobile protocols are unchanged. No new minimum paired-server version is introduced.