fix(native-chat): keep a tab launch draft out of split sibling composers (#16883)

This commit is contained in:
Neil
2026-08-29 21:16:35 -07:00
committed by GitHub
parent 7e7f241ec6
commit 668c1bc0c0
9 changed files with 249 additions and 27 deletions
@@ -72,8 +72,7 @@ export const NativeChatComposer = forwardRef<NativeChatComposerHandle, NativeCha
onSlashCommand,
onSwitchToTerminal,
readTerminalScreen,
launchDraft,
launchDraftResolved = false,
launchSeed,
structuredTransport
},
ref
@@ -88,8 +87,9 @@ export const NativeChatComposer = forwardRef<NativeChatComposerHandle, NativeCha
useNativeChatLaunchDraftAdoption({
terminalTabId,
agent,
launchDraft,
launchDraftResolved,
launchDraft: launchSeed?.launchDraft,
launchDraftResolved: launchSeed?.launchDraftResolved === true,
ownsTabWideLaunchDraft: launchSeed?.ownsTabWideLaunchDraft === true,
draft,
setDraft,
setCaret
@@ -285,8 +285,8 @@ export const NativeChatComposer = forwardRef<NativeChatComposerHandle, NativeCha
imageAttachments,
disabled,
isDispatchingSessionOption,
launchDraft,
launchDraftResolved,
launchDraft: launchSeed?.launchDraft,
launchDraftResolved: launchSeed?.launchDraftResolved === true,
readTerminalScreen,
resolveTarget,
classifySend,
@@ -73,6 +73,7 @@ function NativeChatBridgeView({
targetPtyId = null,
launchAgent,
resolvedAgent,
ownsTabWideLaunchDraft,
onSwitchToTerminal,
readTerminalScreen,
contextMenuActions,
@@ -99,6 +100,7 @@ function NativeChatBridgeView({
isVisible={isVisible}
targetPtyId={targetPtyId}
terminalTabId={terminalTabId}
ownsTabWideLaunchDraft={ownsTabWideLaunchDraft}
onSwitchToTerminal={onSwitchToTerminal}
readTerminalScreen={readTerminalScreen}
contextMenuActions={contextMenuActions}
@@ -117,6 +119,7 @@ function NativeChatResolvedView({
isVisible,
targetPtyId,
terminalTabId,
ownsTabWideLaunchDraft,
onSwitchToTerminal,
readTerminalScreen,
contextMenuActions,
@@ -140,7 +143,6 @@ function NativeChatResolvedView({
const paneLaunchPrompt = launchPrompt?.agent === agent ? launchPrompt : null
// Launch context prefilled into the TUI input as an unsent draft; the
// composer adopts it so the GUI view shows the same context as the TUI.
// Shape matches NativeChatComposer's two launch-draft props, so it spreads.
const launchDraftSignal = useNativeChatLaunchDraftSignal({
terminalTabId,
agent,
@@ -450,7 +452,7 @@ function NativeChatResolvedView({
onSlashCommand={onSlashCommand}
onSwitchToTerminal={onSwitchToTerminal}
readTerminalScreen={readTerminalScreen}
{...launchDraftSignal}
launchSeed={{ ...launchDraftSignal, ownsTabWideLaunchDraft }}
/>
)}
{contextMenu.menu}
@@ -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
@@ -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(
@@ -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
}
@@ -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<NativeChatContextMenuActions, 'onPaste'>
@@ -29,11 +29,13 @@ vi.mock('../../store', () => {
return { useAppStore }
})
const SEED_TEXT = 'https://github.com/o/r/issues/12'
function launchDraft(overrides: Partial<NativeChatLaunchDraft> = {}): 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<typeof vi.fn>; setCaret: ReturnType<typeof vi.fn> } {
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<typeof vi.fn>; setCaret: ReturnType<typeof vi.fn> } {
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')
})
})
@@ -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
])
}
@@ -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={{