From 668c1bc0c02428753ff4d4b96da2e29373e2dfe4 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 29 Aug 2026 21:16:35 -0700 Subject: [PATCH] fix(native-chat): keep a tab launch draft out of split sibling composers (#16883) --- .../native-chat/NativeChatComposer.tsx | 12 +- .../components/native-chat/NativeChatView.tsx | 6 +- .../native-chat/native-chat-composer-types.ts | 16 ++- .../native-chat-leaf-routing.test.ts | 36 ++++++ .../native-chat/native-chat-leaf-routing.ts | 26 +++- .../native-chat/native-chat-view-types.ts | 3 + ...native-chat-launch-draft-adoption.test.tsx | 115 +++++++++++++++++- .../use-native-chat-launch-draft-adoption.ts | 53 ++++++-- .../components/terminal-pane/TerminalPane.tsx | 9 ++ 9 files changed, 249 insertions(+), 27 deletions(-) diff --git a/src/renderer/src/components/native-chat/NativeChatComposer.tsx b/src/renderer/src/components/native-chat/NativeChatComposer.tsx index 256e7878f7a..8e47eb1a8f4 100644 --- a/src/renderer/src/components/native-chat/NativeChatComposer.tsx +++ b/src/renderer/src/components/native-chat/NativeChatComposer.tsx @@ -72,8 +72,7 @@ export const NativeChatComposer = forwardRef )} {contextMenu.menu} diff --git a/src/renderer/src/components/native-chat/native-chat-composer-types.ts b/src/renderer/src/components/native-chat/native-chat-composer-types.ts index 45ebde1b375..9c314f90264 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-types.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-types.ts @@ -47,14 +47,22 @@ export type NativeChatComposerProps = { onSwitchToTerminal?: () => void /** Reads the hosted TUI's current rendered screen when chat is entered. */ readTerminalScreen?: () => string | null - /** Launch context prefilled into the TUI input as an unsent draft; adopted as the composer draft. */ - launchDraft?: NativeChatLaunchDraft | null - /** True once the transcript shows the TUI-side draft was submitted or cleared. */ - launchDraftResolved?: boolean + /** The tab's launch seed as this pane sees it. */ + launchSeed?: NativeChatLaunchSeed /** Structured journal transport; absent keeps the existing PTY path unchanged. */ structuredTransport?: NativeChatStructuredComposerTransport } +/** Launch context prefilled into the TUI input as an unsent draft, plus the two + * facts that decide its fate in this pane's composer. */ +export type NativeChatLaunchSeed = { + launchDraft: NativeChatLaunchDraft | null + /** True once the transcript shows the TUI-side draft was submitted or cleared. */ + launchDraftResolved: boolean + /** False for every pane of a split tab; gates adopting the seed, not cleanup. */ + ownsTabWideLaunchDraft: boolean +} + export type NativeChatComposerHandle = { focus: () => boolean insertTypedText: (text: string) => boolean diff --git a/src/renderer/src/components/native-chat/native-chat-leaf-routing.test.ts b/src/renderer/src/components/native-chat/native-chat-leaf-routing.test.ts index dc1ba79be3a..4b4696720e9 100644 --- a/src/renderer/src/components/native-chat/native-chat-leaf-routing.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-leaf-routing.test.ts @@ -1,9 +1,45 @@ import { describe, expect, it } from 'vitest' import { nativeChatLaunchAgentForLeaf, + nativeChatLeafOwnsTabWideEvidence, resolveNativeChatLeafRoute } from './native-chat-leaf-routing' +describe('nativeChatLeafOwnsTabWideEvidence', () => { + it("owns tab-wide evidence only while the bound leaf is still the tab's sole pane", () => { + expect( + nativeChatLeafOwnsTabWideEvidence({ + ownerLeafId: 'leaf-a', + leafId: 'leaf-a', + leafIds: ['leaf-a'] + }) + ).toBe(true) + // A split sibling must not inherit the tab's launch draft (issue #16695). + expect( + nativeChatLeafOwnsTabWideEvidence({ + ownerLeafId: 'leaf-a', + leafId: 'leaf-b', + leafIds: ['leaf-a', 'leaf-b'] + }) + ).toBe(false) + // Not even the original pane keeps it once the tab is split. + expect( + nativeChatLeafOwnsTabWideEvidence({ + ownerLeafId: 'leaf-a', + leafId: 'leaf-a', + leafIds: ['leaf-a', 'leaf-b'] + }) + ).toBe(false) + expect( + nativeChatLeafOwnsTabWideEvidence({ + ownerLeafId: null, + leafId: 'leaf-a', + leafIds: ['leaf-a'] + }) + ).toBe(false) + }) +}) + describe('nativeChatLaunchAgentForLeaf', () => { it('uses the tab launch hint only for its sole leaf', () => { expect( diff --git a/src/renderer/src/components/native-chat/native-chat-leaf-routing.ts b/src/renderer/src/components/native-chat/native-chat-leaf-routing.ts index 59c660930c5..2e842b429dc 100644 --- a/src/renderer/src/components/native-chat/native-chat-leaf-routing.ts +++ b/src/renderer/src/components/native-chat/native-chat-leaf-routing.ts @@ -44,6 +44,22 @@ export function isNativeChatTabWideFallbackSafe( return !layout.activeLeafId || layout.activeLeafId === layout.root.leafId } +/** Whether tab-wide launch evidence (agent hint, launch draft) describes this + * leaf: it must still be the tab's sole pane and the one the evidence bound to. */ +export function nativeChatLeafOwnsTabWideEvidence(args: { + ownerLeafId: string | null + leafId: string | null + leafIds: readonly string[] +}): boolean { + const { ownerLeafId, leafId, leafIds } = args + if (!ownerLeafId || !leafId) { + return false + } + // Why: the evidence belongs to the tab's original pane. Once a split exists, + // it says nothing about any particular sibling. + return leafIds.length === 1 && leafIds[0] === leafId && ownerLeafId === leafId +} + export function nativeChatLaunchAgentForLeaf(args: { launchAgent?: TuiAgent | null launchAgentLeafId: string | null @@ -51,12 +67,14 @@ export function nativeChatLaunchAgentForLeaf(args: { leafIds: readonly string[] }): TuiAgent | null { const { launchAgent, launchAgentLeafId, leafId, leafIds } = args - if (!launchAgent || !launchAgentLeafId || !leafId) { + if (!launchAgent) { return null } - // Why: launchAgent belongs to the tab's original pane. Once a split exists, - // it is not evidence that an agent is running in any particular sibling. - return leafIds.length === 1 && leafIds[0] === leafId && launchAgentLeafId === leafId + return nativeChatLeafOwnsTabWideEvidence({ + ownerLeafId: launchAgentLeafId, + leafId, + leafIds + }) ? launchAgent : null } diff --git a/src/renderer/src/components/native-chat/native-chat-view-types.ts b/src/renderer/src/components/native-chat/native-chat-view-types.ts index 4dd9a7bac03..106b29b429d 100644 --- a/src/renderer/src/components/native-chat/native-chat-view-types.ts +++ b/src/renderer/src/components/native-chat/native-chat-view-types.ts @@ -25,6 +25,8 @@ export type NativeChatBridgeViewProps = NativeChatOrchestrationProps & { launchAgent?: TuiAgent | null /** Trusted title/foreground fallback for manually-started agents. */ resolvedAgent?: TuiAgent | null + /** Whether this pane owns the tab's launch draft; false for split siblings. */ + ownsTabWideLaunchDraft: boolean /** Return this pane to the hosted terminal surface. */ onSwitchToTerminal?: () => void /** Current xterm screen reader used to recover agent-reported session state. */ @@ -50,6 +52,7 @@ export type NativeChatResolvedViewProps = NativeChatOrchestrationProps & { isVisible: boolean targetPtyId: string | null terminalTabId: string + ownsTabWideLaunchDraft: boolean onSwitchToTerminal?: () => void readTerminalScreen?: () => string | null contextMenuActions?: Omit diff --git a/src/renderer/src/components/native-chat/use-native-chat-launch-draft-adoption.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-launch-draft-adoption.test.tsx index 2c0bfaf6570..71da8b11b60 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-launch-draft-adoption.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-launch-draft-adoption.test.tsx @@ -29,11 +29,13 @@ vi.mock('../../store', () => { return { useAppStore } }) +const SEED_TEXT = 'https://github.com/o/r/issues/12' + function launchDraft(overrides: Partial = {}): NativeChatLaunchDraft { return { tabId: 'tab-1', agent: 'claude', - text: 'https://github.com/o/r/issues/12', + text: SEED_TEXT, createdAt: 1000, ...overrides } @@ -44,6 +46,7 @@ function setup(args: { launchDraftResolved?: boolean draft?: string agent?: string + ownsTabWideLaunchDraft?: boolean }): { setDraft: ReturnType; setCaret: ReturnType } { const setDraft = vi.fn() const setCaret = vi.fn() @@ -55,7 +58,8 @@ function setup(args: { launchDraftResolved: args.launchDraftResolved ?? false, draft: args.draft ?? '', setDraft, - setCaret + setCaret, + ownsTabWideLaunchDraft: args.ownsTabWideLaunchDraft ?? true }) ) return { setDraft, setCaret } @@ -67,7 +71,10 @@ function userTurn(id: string, timestamp: number | null): NativeChatMessage { return { id, role: 'user', blocks: [{ type: 'text', text: id }], timestamp, source: 'transcript' } } -type SignalProps = { messages: NativeChatMessage[]; transcriptLoading?: boolean } +type SignalProps = { + messages: NativeChatMessage[] + transcriptLoading?: boolean +} function renderSignal(messages: NativeChatMessage[], transcriptLoading = false) { const initialProps: SignalProps = { messages, transcriptLoading } @@ -186,6 +193,14 @@ describe('useNativeChatLaunchDraftSignal', () => { expect(result.current.launchDraft?.resolved).toBe(true) }) + it("selects the tab's seed for every pane so the resolution machine keeps running", () => { + // Pane ownership gates the composer *write*, not the signal: nulling it here + // would also kill the resolved/cleanup branch for a split tab. + const { result } = renderSignal([]) + + expect(result.current.launchDraft?.text).toBe(SEED_TEXT) + }) + it('ignores a draft seeded for another agent', () => { mocks.storeState.nativeChatLaunchDraftByTabId = { 'tab-1': launchDraft({ agent: 'codex', createdAt: SEEDED_AT }) @@ -258,6 +273,21 @@ describe('useNativeChatLaunchDraftAdoption', () => { expect(mocks.clearNativeChatLaunchDraft).toHaveBeenCalledWith('tab-1') }) + it('does not mirror the seed into a pane that does not own the tab-wide evidence', () => { + const { setDraft } = setup({ launchDraft: launchDraft(), ownsTabWideLaunchDraft: false }) + + expect(setDraft).not.toHaveBeenCalled() + expect(mocks.markNativeChatLaunchDraftAdopted).not.toHaveBeenCalled() + }) + + it('leaves an unadopted seed alone when a non-owning pane resolves it', () => { + // The owner may not have mirrored it yet; a sibling's transcript is no + // evidence about the owner's copy. + setup({ launchDraft: launchDraft(), launchDraftResolved: true, ownsTabWideLaunchDraft: false }) + + expect(mocks.clearNativeChatLaunchDraft).not.toHaveBeenCalled() + }) + it('drops an unadopted seed once the transcript resolves it', () => { const { setDraft } = setup({ launchDraft: launchDraft(), launchDraftResolved: true }) @@ -266,3 +296,82 @@ describe('useNativeChatLaunchDraftAdoption', () => { expect(mocks.clearNativeChatLaunchDraft).toHaveBeenCalledWith('tab-1') }) }) + +describe('launch draft adoption across a split', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.storeState.nativeChatLaunchDraftByTabId = { + 'tab-1': launchDraft({ createdAt: SEEDED_AT }) + } + }) + + function renderPaneComposer(args: { + ownsTabWideLaunchDraft: boolean + draft?: string + messages?: NativeChatMessage[] + }): { setDraft: ReturnType; setCaret: ReturnType } { + const setDraft = vi.fn() + const setCaret = vi.fn() + renderHook(() => { + const signal = useNativeChatLaunchDraftSignal({ + terminalTabId: 'tab-1', + agent: 'claude', + messages: args.messages ?? [], + transcriptLoading: false + }) + useNativeChatLaunchDraftAdoption({ + terminalTabId: 'tab-1', + agent: 'claude', + launchDraft: signal.launchDraft, + launchDraftResolved: signal.launchDraftResolved, + draft: args.draft ?? '', + setDraft, + setCaret, + ownsTabWideLaunchDraft: args.ownsTabWideLaunchDraft + }) + }) + return { setDraft, setCaret } + } + + it("never fills a non-owning pane's composer with the tab's launch draft", () => { + const { setDraft } = renderPaneComposer({ ownsTabWideLaunchDraft: false }) + + expect(setDraft).not.toHaveBeenCalled() + expect(mocks.markNativeChatLaunchDraftAdopted).not.toHaveBeenCalled() + }) + + it('never lets a non-owning pane destroy a seed nobody has adopted yet', () => { + const { setDraft } = renderPaneComposer({ + ownsTabWideLaunchDraft: false, + messages: [userTurn('u1', null)] + }) + + expect(setDraft).not.toHaveBeenCalled() + expect(mocks.clearNativeChatLaunchDraft).not.toHaveBeenCalled() + }) + + it('still mirrors the launch draft into the owning pane composer', () => { + const { setDraft } = renderPaneComposer({ ownsTabWideLaunchDraft: true }) + + expect(setDraft).toHaveBeenCalledWith(SEED_TEXT) + }) + + it('clears the adopted copy after a split once the transcript resolves the seed', () => { + // Splitting drops ownership for *both* panes, so the pane that already + // mirrored the seed must still clean up — otherwise the submitted prompt + // sits in its composer forever and a later Enter re-sends it (#16695). + mocks.storeState.nativeChatLaunchDraftByTabId = { + 'tab-1': launchDraft({ adopted: true, createdAt: SEEDED_AT }) + } + + const { setDraft, setCaret } = renderPaneComposer({ + ownsTabWideLaunchDraft: false, + draft: SEED_TEXT, + messages: [userTurn('u1', null)] + }) + + expect(setDraft).toHaveBeenCalledWith('') + expect(setCaret).toHaveBeenCalledWith(0) + expect(mocks.clearNativeChatLaunchDraft).toHaveBeenCalledWith('tab-1') + }) +}) diff --git a/src/renderer/src/components/native-chat/use-native-chat-launch-draft-adoption.ts b/src/renderer/src/components/native-chat/use-native-chat-launch-draft-adoption.ts index f82d5ab1278..2b2e176a30b 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-launch-draft-adoption.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-launch-draft-adoption.ts @@ -17,6 +17,9 @@ export function useNativeChatLaunchDraftSignal(args: { * session's real history. Same gate mobile's drafts hook uses. */ transcriptLoading?: boolean }): { launchDraft: NativeChatLaunchDraft | null; launchDraftResolved: boolean } { + // Deliberately pane-agnostic: pane ownership gates the composer *write* in + // useNativeChatLaunchDraftAdoption. Nulling the seed here would also strand a + // copy an owning pane already mirrored, since the adoption effect early-returns. const launchDraft = useAppStore((s) => s.nativeChatLaunchDraftByTabId[args.terminalTabId] ?? null) const paneLaunchDraft = launchDraft?.agent === args.agent ? launchDraft : null const messages = args.messages @@ -66,10 +69,11 @@ export function useNativeChatLaunchDraftSignal(args: { * transcript shows the TUI-side copy was resolved (submitted or cleared). * * State machine per seeded draft: - * - unadopted + composer empty → copy text into the composer, mark adopted - * - unadopted + composer in use → mark adopted without copying (never stomp) - * - resolved by transcript → clear the seed; also clear the composer copy - * only when it is still the untouched seed text + * - unadopted + not the owning pane → ignore (the seed is keyed by tab; #16695) + * - unadopted + composer empty → copy text into the composer, mark adopted + * - unadopted + composer in use → mark adopted without copying (never stomp) + * - resolved by transcript → clear the composer copy while it is still + * the untouched seed text, and drop the seed */ export function useNativeChatLaunchDraftAdoption(args: { terminalTabId: string @@ -79,23 +83,47 @@ export function useNativeChatLaunchDraftAdoption(args: { draft: string setDraft: (next: string) => void setCaret: (next: number) => void + /** This pane is the tab-wide evidence's owner (`nativeChatLeafOwnsTabWideEvidence`). + * Splitting drops it for every pane, so it gates pickup, never cleanup. */ + ownsTabWideLaunchDraft: boolean }): void { - const { terminalTabId, agent, launchDraft, launchDraftResolved, draft, setDraft, setCaret } = args + const { + terminalTabId, + agent, + launchDraft, + launchDraftResolved, + draft, + setDraft, + setCaret, + ownsTabWideLaunchDraft + } = args useEffect(() => { if (!launchDraft || launchDraft.agent !== agent) { return } if (launchDraftResolved) { - if (launchDraft.adopted && draft === launchDraft.text) { + // Cleanup must survive a split: ownership is gone for both panes by then, + // so the pane still holding the untouched copy is the one that cleans up. + const holdsUntouchedCopy = launchDraft.adopted && draft === launchDraft.text + if (holdsUntouchedCopy) { setDraft('') setCaret(0) } - useAppStore.getState().clearNativeChatLaunchDraft(terminalTabId) + // A pane that neither owns the seed nor holds its copy has no standing to + // drop it — the owning pane may not have mirrored it yet. + if (ownsTabWideLaunchDraft || holdsUntouchedCopy) { + useAppStore.getState().clearNativeChatLaunchDraft(terminalTabId) + } return } if (launchDraft.adopted) { return } + // #16695: the seed describes the tab's original sole pane, so once a split + // exists no pane may mirror it into a composer. + if (!ownsTabWideLaunchDraft) { + return + } // Mark adopted before copying so a composer that already holds user text // declines the seed permanently instead of resurrecting it on a later clear. useAppStore.getState().markNativeChatLaunchDraftAdopted(terminalTabId) @@ -103,5 +131,14 @@ export function useNativeChatLaunchDraftAdoption(args: { setDraft(launchDraft.text) setCaret(launchDraft.text.length) } - }, [agent, draft, launchDraft, launchDraftResolved, setCaret, setDraft, terminalTabId]) + }, [ + agent, + draft, + launchDraft, + launchDraftResolved, + ownsTabWideLaunchDraft, + setCaret, + setDraft, + terminalTabId + ]) } diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index 6157af7d36d..4b8d43cac31 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -116,6 +116,7 @@ import { shouldChatTakeOverMobileSurface } from '../native-chat/native-chat-send import { canToggleNativeChat } from '../native-chat/native-chat-availability' import { nativeChatLaunchAgentForLeaf, + nativeChatLeafOwnsTabWideEvidence, resolveNativeChatLeafRoute, type NativeChatLeafRoute } from '../native-chat/native-chat-leaf-routing' @@ -2983,6 +2984,13 @@ function TerminalPane( }) const structuredChatAgent = structuredSessionAgent ?? chatPaneResolvedAgent ?? chatPaneLaunchAgent const structuredChatTarget = useMemo(() => ({ kind: 'local' as const }), []) + // The launch draft is keyed by tab, so gate it on the same pane ownership the + // launch agent uses: a split sibling must not inherit the seeded text. + const chatPaneOwnsTabWideLaunchDraft = nativeChatLeafOwnsTabWideEvidence({ + ownerLeafId: getTabWideAgentHintLeafId(), + leafId: chatPane?.leafId ?? null, + leafIds: getNativeChatLeafIds() + }) // A split can host different agents, so continuation resolves the specific leaf before using tab-wide hints. const resolveAgentForLeaf = (leafId: string | null): string | null => { const detectedAgent = leafId ? (tabAgentTypeByLeaf[leafId] ?? null) : null @@ -3155,6 +3163,7 @@ function TerminalPane( targetPtyId={chatPanePtyId} launchAgent={chatPaneLaunchAgent} resolvedAgent={chatPaneResolvedAgent} + ownsTabWideLaunchDraft={chatPaneOwnsTabWideLaunchDraft} onSwitchToTerminal={switchNativeChatToTerminal} readTerminalScreen={readNativeChatTerminalScreen} contextMenuActions={{