fix(native-chat): keep TUI seed latch out of render (STA-4317)

React can replay or discard render, so writing the TUI-seeded flag inside
useMemo could leak a derived false over an async true and re-enable the
first hook-model report the TUI already named. Derive seededFromScreen
and latch the pty id only after a committed scrape.
This commit is contained in:
Brennan Benson
2026-08-20 18:32:50 -07:00
parent fb85ab6ce5
commit 057dbddab5
2 changed files with 71 additions and 33 deletions
@@ -254,6 +254,38 @@ describe('useNativeChatSessionOptions model reporting', () => {
expect(effortValue(result.current.snapshot)).toBe('max')
})
it('does not let a late hook model replace a TUI seed after the frame scrolls away', async () => {
// Why: the first SessionStart slug is the launched id. Once the TUI has
// named the effective model, a later render that can no longer parse the
// frame must not treat that slug as a fresh report.
seedNativeChatAppliedSessionOptions('pty-codex-scrolled', 'codex', {
model: 'gpt-5.6-terra'
})
const { result, rerender } = renderHook(
({ reportedModel, frameVisible }) =>
useNativeChatSessionOptions({
agent: 'codex',
terminalTabId: 'tab-codex-scrolled',
targetPtyId: 'pty-codex-scrolled',
dispatchCommand: vi.fn(),
readTerminalScreen: () =>
frameVisible ? CODEX_STATUS_LINE : 'conversation has scrolled past the frame',
reportedModel
}),
{ initialProps: { reportedModel: null as string | null, frameVisible: true } }
)
await waitFor(() =>
expect(modelDescriptor(result.current.snapshot).currentValue).toBe('gpt-5.6-luna')
)
rerender({ reportedModel: 'gpt-5.6-terra', frameVisible: false })
await waitFor(() =>
expect(modelDescriptor(result.current.snapshot).currentValue).toBe('gpt-5.6-luna')
)
})
it('treats a Codex catalog label and id as the same model', async () => {
seedNativeChatAppliedSessionOptions('pty-codex-label', 'codex', { model: 'gpt-5.6-terra' })
const { result } = renderHook(() =>
@@ -123,18 +123,19 @@ export function useNativeChatSessionOptions(args: {
// The screen text that last parsed into reported values, so a later model
// discovery can re-resolve it against the host's real ids.
const reportedScreenRef = useRef<string | null>(null)
const tuiSeededRef = useRef(false)
// Pty whose TUI already named the model. Written only after a committed
// scrape — never during render, and never cleared by a later unparseable frame.
const tuiSeededPtyIdRef = useRef<string | null>(null)
const previousHookModelRef = useRef<string | null>(null)
const discoveryContext = useMemo(
() => resolveNativeChatModelDiscoveryContext(terminalTabId),
[terminalTabId]
)
const surface = useMemo(() => {
const { surface, seededFromScreen } = useMemo(() => {
// Why: native chat currently attaches only after startup is already queued;
// exposing a draft picker here would claim it can still mutate that command.
if (!targetPtyId) {
tuiSeededRef.current = false
return null
return { surface: null, seededFromScreen: false }
}
const scopeKey = targetPtyId ?? terminalTabId
const discoveredModels = discoveryContext
@@ -145,31 +146,33 @@ export function useNativeChatSessionOptions(args: {
readTerminalScreen?.(),
discoveredModels ?? undefined
)
tuiSeededRef.current = screenValues != null
return createNativeChatPtySessionOptions({
agent,
scopeKey,
...(targetPtyId ? { fallbackScopeKey: terminalTabId } : {}),
// Why: the catalog seed carries version-neutral family labels, so it is
// safe on every host while the once-per-host probe runs or after it fails
// — without it the whole picker would pop in late or never appear.
...(discoveryContext ? { initialModels: discoveredModels ?? undefined } : {}),
mode: targetPtyId ? 'live' : 'draft',
reportedValues: screenValues,
dispatchCommand,
onAgentPicker,
persistSelection: ({ modelId, optionId, value, adoptModelAsLaunchDefault }) =>
enqueueSessionOptionSettingsWrite((persisted) =>
updateNativeChatSessionOptionDefaults({
persisted,
agent,
modelId,
optionId,
value,
adoptModelAsLaunchDefault
})
)
})
return {
surface: createNativeChatPtySessionOptions({
agent,
scopeKey,
...(targetPtyId ? { fallbackScopeKey: terminalTabId } : {}),
// Why: the catalog seed carries version-neutral family labels, so it is
// safe on every host while the once-per-host probe runs or after it fails
// — without it the whole picker would pop in late or never appear.
...(discoveryContext ? { initialModels: discoveredModels ?? undefined } : {}),
mode: targetPtyId ? 'live' : 'draft',
reportedValues: screenValues,
dispatchCommand,
onAgentPicker,
persistSelection: ({ modelId, optionId, value, adoptModelAsLaunchDefault }) =>
enqueueSessionOptionSettingsWrite((persisted) =>
updateNativeChatSessionOptionDefaults({
persisted,
agent,
modelId,
optionId,
value,
adoptModelAsLaunchDefault
})
)
}),
seededFromScreen: screenValues != null
}
}, [
agent,
dispatchCommand,
@@ -186,6 +189,9 @@ export function useNativeChatSessionOptions(args: {
}
let cancelled = false
reportedScreenRef.current = null
if (seededFromScreen && targetPtyId) {
tuiSeededPtyIdRef.current = targetPtyId
}
const reportCurrentValues = async (): Promise<void> => {
let authoritativeScreen: string | null = null
if (targetPtyId && window.api?.pty?.getMainBufferSnapshot) {
@@ -219,7 +225,7 @@ export function useNativeChatSessionOptions(args: {
return
}
reportedScreenRef.current = screen
tuiSeededRef.current = true
tuiSeededPtyIdRef.current = targetPtyId
surface.reportSessionOptions(reportedValues)
return
}
@@ -228,7 +234,7 @@ export function useNativeChatSessionOptions(args: {
return () => {
cancelled = true
}
}, [agent, discoveryContext, readTerminalScreen, surface, targetPtyId])
}, [agent, discoveryContext, readTerminalScreen, seededFromScreen, surface, targetPtyId])
// Hook reports follow later /model and provider-fallback changes. The first
// slug is skipped only when the TUI screen already named the model — otherwise
@@ -250,11 +256,11 @@ export function useNativeChatSessionOptions(args: {
return
}
previousHookModelRef.current = matched
if (!previous && tuiSeededRef.current) {
if (!previous && (seededFromScreen || tuiSeededPtyIdRef.current === targetPtyId)) {
return
}
surface.reportSessionOptions(values)
}, [agent, reportedModel, surface])
}, [agent, reportedModel, seededFromScreen, surface, targetPtyId])
useEffect(() => {
if (!surface || !discoveryContext) {