Repository navigation
fix(chat): split into a fresh chat beside the current conversation - #28950
Conversation
2130226 to
579b23d
Compare
c9e3d08 to
ce528da
Compare
ce528da to
d56006f
Compare
|
A new native chat could disappear when a host list created before it arrived after its create reply. Its pane collapsed, so immediate typing or Close could affect a neighbour. Split also moved the current conversation instead of opening a fresh one. Split Right/Down now keeps the source and opens a fresh same-agent chat on the same machine in a focused sibling pane. It reuses New Chat's launch. Ordinary New Chat shares the publication protection; Move and terminal-backed splitting keep their separate behavior. Source review, final reference comparison and requested real QA complete on Fixed during review: publication confirmation from rejected paired frames; expiry network work and late deleted-workspace resurrection; missing New Chat error feedback when Split cannot build a launch; a stale test import. Intended differences from the common pattern remain explicit: protection is a separate in-memory 30-second record until an accepted creating-host list includes the chat; reload before first inclusion loses it; an externally closed chat can remain until the next accepted list after the bound; a pre-create list delayed beyond the bound can still remove it. Expiry only forgets the record. Close and workspace teardown clear it. It does not gate sending, status, model choices or Stop. Final-head real QA passed on All six requested flows passed for both agents where applicable: restoring main-created chats/drafts and obtaining fresh contextual replies; Split Right by shortcut and Down by palette, keeping the source/draft and creating a focused same-agent chat on the same machine; Down → send → real reply → immediate Right → ordered typing → screenshot → Close, with only the newest pane closing; ordinary New Chat immediately after a reply; terminal-backed Split Right with typing in the new shell and the original chat reply retained; and restart with restored panes and new contextual replies in the retained split chats. Original rapid scripts put Right, typing, screenshot and Close directly after the scoped assistant reply observation, with metadata reads outside that interval. All three QA drivers exited successfully. Owned processes stopped, leased ports were clear, source remained clean, and real configuration hashes were unchanged. The lane owner released ticket 1524. The full PR workflow 39836414654 passed on exact head Earlier real QA on The earlier full run on Evidence limits remain explicit: no tiny-pane fix, matched hidden/frozen head result, mixed-version peer, SSH, Windows/Linux/mobile or native OS focus QA claim. No stale-list or expiry injection was performed in this final running-app round; automated tests cover those contracts. Earlier matched controls reproduced tiny transcript and terminal readability symptoms on main; the terminal split route is unchanged. The earlier peer-tab freeze observation remains outside this PR on source evidence, with a matched head lifecycle control unavailable. Main Detailed evidence: single review status. Earlier shared focus work is tracked in #28987. For Brennan's review Original full-window screenshots follow, with their heads labelled. Main-versus-branch behavior comparisons and broader coverage are from the earlier full run. Renewed final-head captures show real replies, Right/Down, rapid typing/Close and restart. Final head
Claude original rapid cycle on final head
Codex original rapid cycle on final head
Final-head baseline, terminal splitting and restart evidenceMain baseline Claude reply Main baseline Codex reply Terminal-backed split and shell input Restored split Claude contextual reply Restored split Codex contextual reply Final restored layout and contextual replies Earlier full-run main-versus-branch behavior comparison and broader coverageThese captures are main
Three useful source points: shared New Chat split action, bounded protection and expiry, and publication after accepted store commit. |
…t-split-draft # Conflicts: # src/renderer/src/components/native-chat/use-native-chat-composer-reveal-focus.ts # src/renderer/src/components/native-chat/use-structured-native-chat-pane-commands.test.tsx # src/renderer/src/components/tab-bar/QuickLaunchButton.tsx # src/renderer/src/components/tab-bar/use-tab-bar-create-menu-controller.ts # src/renderer/src/components/use-terminal-create-actions.ts # src/renderer/src/lib/launch-agent-in-new-tab.ts # src/renderer/src/lib/structured-agent-session-launch-admission.ts # src/renderer/src/lib/structured-agent-session-provisional-tab.test.ts # src/renderer/src/lib/structured-agent-session-provisional-tab.ts
Split Right/Down on a native chat used to move the chat into a new pane. It now opens an empty pane beside the chat and launches a new chat of the same agent on the same host into it through the New Chat launch, so the new composer takes focus at once. A launch that never opens a tab closes the empty pane. Drops the earlier admission and focus-transfer changes that main now covers.
ℹ️ Recent review info
📝 Walkthrough
Priority: ⚪ Pending latest changes Merge Risk: 🔵 Low · up to
|
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:
8110130f-87b8-42e4-b47f-567838f693c5
📒 Files selected for processing: 22
src/renderer/src/components/cmd-j/quick-action-context.test.tssrc/renderer/src/components/native-chat/native-chat-composer-reveal-focus.test.tsxsrc/renderer/src/components/native-chat/native-chat-layout-actions.test.tssrc/renderer/src/components/native-chat/native-chat-layout-actions.tssrc/renderer/src/components/native-chat/native-chat-split-flow.test.tssrc/renderer/src/components/native-chat/use-native-chat-composer-reveal-focus.tssrc/renderer/src/components/native-chat/use-structured-native-chat-pane-commands.test.tsxsrc/renderer/src/components/tab-bar/QuickLaunchButton.tsxsrc/renderer/src/components/tab-bar/use-tab-bar-create-menu-controller.tssrc/renderer/src/components/tab-group/TabGroupPanel.tsxsrc/renderer/src/components/use-terminal-create-actions.tssrc/renderer/src/lib/agent-launch-destination.tssrc/renderer/src/lib/agent-launch-funnel-test-harness.tssrc/renderer/src/lib/launch-agent-in-new-tab-host-agents.test.tssrc/renderer/src/lib/launch-agent-in-new-tab-host-agents.tssrc/renderer/src/lib/launch-agent-in-new-tab-structured-route.tssrc/renderer/src/lib/launch-agent-in-new-tab.test.tssrc/renderer/src/lib/launch-agent-in-new-tab.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-provisional-tab.test.tssrc/renderer/src/lib/structured-agent-session-provisional-tab.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
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 · Start the 1-second expiry when the destination tab… · use-native-chat-composer-reveal-focus.ts:43
src/renderer/src/components/native-chat/use-native-chat-composer-reveal-focus.ts:43
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winStart the 1-second expiry when the destination tab is requested, not when the intent is captured.
captureNativeChatComposerFocusIntentstartssetTimeout(cancel, PENDING_FOCUS_MS)at capture time. Capture time is before host admission. The callers below callrequest(tabId)only after admission creates the tab:
runNativeChatSplitTargetQuickLaunchButtonlaunchAgentFromNewTabEntryhandleNewAgentTab
beginStructuredAgentSessionProvisionalLaunchcallsfocusIntent.request(launch.tab.id)fromopenAdmitted.openAdmittedruns afteradmitStructuredLaunchOnHostresolves.launchOnceHostAnsweredcan also wait up toHOST_ANSWER_WAIT_MS(3 000 ms) before the relaunch.If admission takes more than 1 second,
cancel()has already clearedpendingFocusRequest, sorequest()returns early. A slow paired host commonly causes this delay. The result:
- In the split flow, focus stays on the empty
data-tab-group-body-idelement.- The new chat's composer never gets focus.
The PR states that focus moves to the composer after host admission. The pointer and
Tabcancellation already protect against stale focus during admission. Start the expiry inrequest(), so it limits only the wait for the composer to mount and become ready.Proposed fix
- const pending: { tabId: string | null; cancel: () => void } = { tabId: null, cancel } - const timeout = setTimeout(cancel, PENDING_FOCUS_MS) + const pending: { tabId: string | null; cancel: () => void } = { tabId: null, cancel } + let timeout: ReturnType<typeof setTimeout> | undefined pendingFocusRequest = pending document.addEventListener('pointerdown', cancel, true) document.addEventListener('keydown', cancelForNavigation, true) return { request: (tabId) => { if (pendingFocusRequest !== pending) { return } pending.tabId = tabId const reclaim = revealedComposerReclaims.get(tabId) if (reclaim) { cancel() reclaim() + return } + timeout = setTimeout(cancel, PENDING_FOCUS_MS) }, cancel }
clearTimeout(undefined)incancelis a safe no-op.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
8110130f-87b8-42e4-b47f-567838f693c5
📒 Files selected for processing: 22
src/renderer/src/components/cmd-j/quick-action-context.test.tssrc/renderer/src/components/native-chat/native-chat-composer-reveal-focus.test.tsxsrc/renderer/src/components/native-chat/native-chat-layout-actions.test.tssrc/renderer/src/components/native-chat/native-chat-layout-actions.tssrc/renderer/src/components/native-chat/native-chat-split-flow.test.tssrc/renderer/src/components/native-chat/use-native-chat-composer-reveal-focus.tssrc/renderer/src/components/native-chat/use-structured-native-chat-pane-commands.test.tsxsrc/renderer/src/components/tab-bar/QuickLaunchButton.tsxsrc/renderer/src/components/tab-bar/use-tab-bar-create-menu-controller.tssrc/renderer/src/components/tab-group/TabGroupPanel.tsxsrc/renderer/src/components/use-terminal-create-actions.tssrc/renderer/src/lib/agent-launch-destination.tssrc/renderer/src/lib/agent-launch-funnel-test-harness.tssrc/renderer/src/lib/launch-agent-in-new-tab-host-agents.test.tssrc/renderer/src/lib/launch-agent-in-new-tab-host-agents.tssrc/renderer/src/lib/launch-agent-in-new-tab-structured-route.tssrc/renderer/src/lib/launch-agent-in-new-tab.test.tssrc/renderer/src/lib/launch-agent-in-new-tab.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-provisional-tab.test.tssrc/renderer/src/lib/structured-agent-session-provisional-tab.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
A host tab list built before a chat's create landed can arrive after the create reply. Once the reply settled the launch, nothing protected the new tab, so that older list dropped it: Split's new pane collapsed and focus moved to another chat, and the next list re-added the chat elsewhere. Keep a small in-memory record for a chat this client created that no host list has shown yet. While it exists, a list that omits the chat keeps its tab. The record ends when an accepted list from that host includes the chat, when the tab closes, on workspace teardown, or 30 seconds after the reply, when the worktree is re-read from its host (including an unchanged version) so a chat the host really closed is dropped then. The launch lifecycle, status, persistence and settlement are unchanged.
…t-split-draft # Conflicts: # src/renderer/src/runtime/structured-agent-session-tab-retirement.ts
…t-split-draft # Conflicts: # src/renderer/src/lib/structured-agent-session-provisional-tab.test.ts























Problem and user-facing change
Split moved an existing native chat into another pane and did nothing when it was the only tab. A second race could remove a newly created chat when an older host list arrived after its create reply, collapsing the pane and sending immediate typing or Close to a neighbour.
Split Right and Split Down now keep the current conversation in place and open a fresh chat of the same agent on the same machine beside or below it, with focus in the new composer. Shortcuts and the command palette share this action. Move Tab remains separate and no longer shows shortcuts that create a fresh chat. Terminal-backed chats keep terminal splitting. Ordinary New Chat receives the same protection against older host lists.
Mechanism and intended limits
Split creates a destination pane and reuses the exact New Chat launch. An unbuildable launch closes the empty pane and uses the existing New Chat error feedback; host refusal preserves the failed chat, draft and Retry controls.
A separate in-memory record protects a just-created tab from older host lists until an accepted list from its creating host includes it, with a 30-second bound. The create reply settles the launch immediately. Rejected paired frames cannot release protection; accepted publication runs after the store commit. The record affects neither sending nor agent status, model choices or Stop. Close and workspace teardown clear it. Expiry only forgets the record; the next accepted list decides membership.
Intended differences and limits:
Verification and current readiness
Final-head real QA passed on
769cfcf1e913001a15c7e58e5b46580a222290b9on the M4Air. Main814bb59ef675created the saved state; the app genuinely quit before upgrading that same profile to the branch, then the branch genuinely quit and restarted. Real Claude and Codex produced 16 replies across those phases. The run saved 78 full-window screenshots at 3840×2400 CSS pixels, pixel density 2 (7680×4800 PNG); tested panes stayed above 150 pixels in both dimensions. The default window-owned layout build was used.All six requested flows passed for both agents where applicable: restoring main-created chats/drafts and obtaining fresh contextual replies; Split Right by shortcut and Down by palette, keeping the source/draft and creating a focused same-agent chat on the same machine; Down → send → real reply → immediate Right → ordered typing → screenshot → Close, with only the newest pane closing; ordinary New Chat immediately after a reply; terminal-backed Split Right with typing in the new shell and the original chat reply retained; and restart with restored panes and new contextual replies in the retained split chats. Original rapid scripts put Right, typing, screenshot and Close directly after the scoped assistant reply observation, with metadata reads outside that interval. All three QA drivers exited successfully. Owned processes stopped, leased ports were clear, source remained clean, and real configuration hashes were unchanged. The lane owner released ticket 1524.
The full PR workflow 39836414654 passed on exact head
769cfcf1e913001a15c7e58e5b46580a222290b9. Final source, whole-mechanism comparison, readiness and performance review found no new proven production finding. Earlier reviewed heads passed 11 targeted close/publication tests and a broader 119 targeted tests.Earlier real QA on
ba5456887bedpassed the same six requested flows, with 82 screenshots. That run's procedural failure was resolved by independent event ordering and original action-script evidence; ephemeral unsent Right-tab IDs remained an evidence limit. Its separate stricter supplemental Claude attempt failed on a load-balancer HTTP 429 before the reply/Right action, and its Codex supplement was not run. Its changed real Codex config hash remained unattributed. Those are earlier-run limits; the final769cfcfrun renewed real replies, rapid cycles, upgrade and restart on the merged code.The earlier full run on
5808d997d426supplies broader coverage: all eight shortcut/palette Right/Down combinations across Claude and Codex; six rapid cycles; Move; folder split; paired-server split/reply/reload; and 14 saved tabs through restart, with fresh contextual replies in 11 native chats and the terminal chat. These broader cases were not all renewed on769cfcf. Paired evidence is from that earlier head and an explicitly enabled same-machine server, not a mixed-version or SSH run.Evidence limits remain explicit: no tiny-pane fix, matched hidden/frozen head result, mixed-version peer, SSH, Windows/Linux/mobile or native OS focus QA claim. No stale-list or expiry injection was performed in this final running-app round; automated tests cover those contracts. Earlier matched controls reproduced tiny transcript and terminal readability symptoms on main; the terminal split route is unchanged. The earlier peer-tab freeze observation remains outside this PR on source evidence, with a matched head lifecycle control unavailable.
Main
814bb59was merged normally before final QA. Newer main at the finishing check overlaps onlysrc/renderer/src/i18n/locales/en.jsonthrough #31593 and #28569. The lane owner handles the final pre-merge sync; this finishing pass made no further main merge.Detailed evidence: single review status. Earlier shared focus work is tracked in #28987.
3b02945afe35: existing chat moves769cfcf: fresh sibling, source retainedAuthor: @BrennanKB5