fix(mobile): require a landed read before an errored transcript retires a dismissal

`status === 'error'` was treated as settled on the claim that an error keeps
the last successful read in `messages`. That only holds for an error that lands
on top of an earlier read. The host forwards an initial-drain failure as an
error frame carrying an EMPTY list (transcript-watch-error.test.ts), the mobile
frame applier checks `frame.error` before the messages array so those rows are
discarded, and the session hook's error path never calls `setMessages` — so a
first-read error leaves `messages` at the `[]` the identity-change effect wrote.

That frame is also not terminal: the watcher keeps `initialDrain` true and a
real snapshot follows once the read recovers. So a re-subscribe whose first
read errors made the never-populated list read as "no ask", retired the live
dismissal, and the recovered snapshot brought the answered card back over the
composer — the exact resurfacing this guard exists to close, and most likely on
remote/SSH transcript reads.

Require rows for the error case. Rows can only be present once a read landed,
so the predicate is never wrong in the resurfacing direction; it only declines
to retire a dismissal when the transcript was never observed.

Also drop the dismiss hook's `detectedAsk = ask` default and make both prompts
required. That default silently fed the gated prompt in as the detected one,
which is the pre-fix behavior: a paused-out card would read as "prompt gone"
and retire the dismissal. tsc now enforces the ungated payload at every call
site instead of leaving a trap for the next caller.
This commit is contained in:
Brennan Benson
2026-08-04 03:57:43 -07:00
parent ffd13d17ac
commit 9aef3b2d1a
3 changed files with 52 additions and 14 deletions
@@ -8,10 +8,12 @@ import type { AskPrompt } from './mobile-native-chat-ask'
* Owned by the controller, not the chat subtree: the overlay unmounts on a
* chat↔terminal view toggle, and a dismissal must survive that round-trip. */
export function useMobileNativeChatAskDismiss(args: {
ask?: AskPrompt | null
ask: AskPrompt | null
/** Ungated prompt payload. A working/done status hides the card but does not
* prove the sticky prompt itself cleared. */
detectedAsk?: AskPrompt | null
* prove the sticky prompt itself cleared. Required, and never defaulted to
* `ask`: reading the gated prompt as the detected one is the resurfacing bug
* this hook exists to close. */
detectedAsk: AskPrompt | null
/** Dismissals are scoped to the tab that showed the card, so one tab's
* dismissal can't hide an identical question on another tab. */
scopeKey: string | null
@@ -24,7 +26,7 @@ export function useMobileNativeChatAskDismiss(args: {
showAsk: boolean
dismissAsk: () => void
} {
const { ask, detectedAsk = ask, scopeKey, observing } = args
const { ask, detectedAsk, scopeKey, observing } = args
const askKey = ask ? JSON.stringify(ask.questions) : null
const detectedAskKey = detectedAsk ? JSON.stringify(detectedAsk.questions) : null
const detectedAskKeysRef = useRef(new Map<string | null, string | null>())
@@ -464,12 +464,16 @@ describe('useMobileNativeChatController ask dismissal across a transcript reload
})
}
/** Drive the transcript stand-in the way the real hook couples the two fields
* (`transcriptLoading` is exactly `status === 'loading'`), so these tests can
* only express states the session hook can actually reach. */
function setTranscript(status: MobileNativeChatStatus): void {
/** Drive the transcript stand-in the way the real hook couples its fields, so
* these tests can only express states the session hook can actually reach:
* `transcriptLoading` is exactly `status === 'loading'`, and `messages` is
* withheld until a read lands. `rows` is what the last landed read left
* behind — 0 means no read has ever landed for this identity. */
function setTranscript(status: MobileNativeChatStatus, rows = 1): void {
sessionState.status = status
sessionState.transcriptLoading = status === 'loading'
sessionState.messages =
status === 'ready' || status === 'error' ? Array.from({ length: rows }, () => ({})) : []
}
beforeEach(() => {
@@ -500,7 +504,7 @@ describe('useMobileNativeChatController ask dismissal across a transcript reload
controller = null
promptsState.ask = null
promptsState.detectedAsk = null
setTranscript('ready')
setTranscript('ready', 0)
viewMode.isTabChatView = () => true
activeTab.id = 'tab-1'
})
@@ -617,11 +621,35 @@ describe('useMobileNativeChatController ask dismissal across a transcript reload
expect(controller?.nativeChatAsk).toBeNull()
})
it('keeps the dismissal when the first read of a transcript errors', () => {
// The host forwards an initial-drain failure as an error frame carrying an
// EMPTY list, and that frame is not terminal — a real snapshot follows once
// the read recovers. Judging the prompt from that never-populated list
// retires the dismissal, and the recovered read brings the answered card
// back over the composer.
act(() => controller?.dismissNativeChatAsk())
expect(controller?.nativeChatAsk).toBeNull()
setTranscript('error', 0)
promptsState.ask = null
promptsState.detectedAsk = null
step()
// The read recovers with the same question still pending.
setTranscript('ready')
promptsState.ask = PROMPT
promptsState.detectedAsk = PROMPT
step()
expect(controller?.nativeChatAsk).toBeNull()
})
it('retires the dismissal when a failed read still reports the last transcript', () => {
// A read error leaves the last successful read in `messages`, so a prompt
// that clears under it is real evidence — unlike the never-read empty list
// of 'idle'/'waiting-session'. Treating 'error' as unobservable would freeze
// the dismissal and hide the next identical question for good.
// A read error that lands on top of rows from an earlier read leaves those
// rows in `messages`, so a prompt that clears under it is real evidence —
// unlike the never-read empty list of 'idle'/'waiting-session'. Treating
// every error as unobservable would freeze the dismissal and hide the next
// identical question for good.
act(() => controller?.dismissNativeChatAsk())
expect(controller?.nativeChatAsk).toBeNull()
@@ -194,8 +194,16 @@ export function useMobileNativeChatController(args: {
// `messages` when the client is gone ('idle') or the tab has not reported a
// provider session yet ('waiting-session') — both of which leave the flag false
// over an empty list that was never read.
//
// 'error' only qualifies while rows survive from an earlier read. The host's
// initial-drain error frame carries an empty list and is not terminal (a real
// snapshot follows once the read recovers), so an error before any read leaves
// `messages` never populated — exactly the never-read case this guard exists
// to reject. Rows can only be present once a read landed, so requiring them is
// never wrong in the resurfacing direction.
const nativeChatTranscriptSettled =
nativeChatSession.status === 'ready' || nativeChatSession.status === 'error'
nativeChatSession.status === 'ready' ||
(nativeChatSession.status === 'error' && nativeChatSession.messages.length > 0)
const nativeChatAskObservable =
showNativeChat && (nativeChatDetectedAsk != null || nativeChatTranscriptSettled)
const {