From 9aef3b2d1a64f1f988edb2fb1d82d65cd38220bb Mon Sep 17 00:00:00 2001
From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
Date: Tue, 4 Aug 2026 03:57:43 -0700
Subject: [PATCH] fix(mobile): require a landed read before an errored
 transcript retires a dismissal
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`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.
---
 .../use-mobile-native-chat-ask-dismiss.ts     | 10 ++--
 .../use-mobile-native-chat-controller.test.ts | 46 +++++++++++++++----
 .../use-mobile-native-chat-controller.ts      | 10 +++-
 3 files changed, 52 insertions(+), 14 deletions(-)

diff --git a/mobile/src/session/use-mobile-native-chat-ask-dismiss.ts b/mobile/src/session/use-mobile-native-chat-ask-dismiss.ts
index fd410013ba3..4bbf4abbec0 100644
--- a/mobile/src/session/use-mobile-native-chat-ask-dismiss.ts
+++ b/mobile/src/session/use-mobile-native-chat-ask-dismiss.ts
@@ -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>())
diff --git a/mobile/src/session/use-mobile-native-chat-controller.test.ts b/mobile/src/session/use-mobile-native-chat-controller.test.ts
index da2aa259479..11b57f86417 100644
--- a/mobile/src/session/use-mobile-native-chat-controller.test.ts
+++ b/mobile/src/session/use-mobile-native-chat-controller.test.ts
@@ -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()
 
diff --git a/mobile/src/session/use-mobile-native-chat-controller.ts b/mobile/src/session/use-mobile-native-chat-controller.ts
index cc1751ce8eb..8e51d9d13e5 100644
--- a/mobile/src/session/use-mobile-native-chat-controller.ts
+++ b/mobile/src/session/use-mobile-native-chat-controller.ts
@@ -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 {