From 585b24ba5a2a1b4edb8f24d5f1c0dcdbee0ec642 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 4 Aug 2026 02:21:20 -0700 Subject: [PATCH] fix(mobile): give the stale-status hold its own timer and keep the label readable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects in the first cut of this fix, both found in review. The composer lock latches on the first non-null reason and never unlatches between reasons, so once a lease wait had held its 600ms, a later drop to 'disconnected' flipped the row instantly with no hold at all. Reconnect always lands on 'waiting' first — the lease clears its ready handles on disconnect — so a flapping link strobed the label, tore down and rebuilt the dot animation, and toggled Stop at flap rate. Staleness now runs its own 600ms hold on 'disconnected'; `lockReason` and the composer are untouched, and recovery stays immediate. Dimming the row to 0.55 opacity dropped the label from 5.62:1 to 2.53:1 against bgBase, under AA and even under the large-text floor, on the one element left carrying the state once the dots go. The existing 0.55 uses are all inactive controls, which WCAG exempts; informational text is not. The label swap and the removed dots already read as "not live", so the dim is gone. Also drops a vacuous assertion: `onStop` was asserted un-called without any press being attempted, under a Pressable mocked as a bare host string, so it passed with the guard deleted. Refs STA-3333. --- .../session/MobileAgentWorkingIndicator.tsx | 7 +-- .../src/session/MobileNativeChatView.test.ts | 57 +++++++++++++------ mobile/src/session/MobileNativeChatView.tsx | 14 ++++- 3 files changed, 56 insertions(+), 22 deletions(-) diff --git a/mobile/src/session/MobileAgentWorkingIndicator.tsx b/mobile/src/session/MobileAgentWorkingIndicator.tsx index fc66c2525b8..d2e6cc12763 100644 --- a/mobile/src/session/MobileAgentWorkingIndicator.tsx +++ b/mobile/src/session/MobileAgentWorkingIndicator.tsx @@ -39,7 +39,9 @@ export function MobileAgentWorkingIndicator({ }, [stale]) return ( - + // Not dimmed: the label is the only thing left carrying the state once the + // dots go, and fading it under `textMuted` drops it below AA on `bgBase`. + {stale ? 'Agent status stale' : 'Agent is working'} {stale ? null : ( @@ -60,9 +62,6 @@ const styles = StyleSheet.create({ paddingHorizontal: spacing.md, paddingVertical: spacing.sm }, - rowStale: { - opacity: 0.55 - }, label: { color: colors.textMuted, fontSize: typography.metaSize, diff --git a/mobile/src/session/MobileNativeChatView.test.ts b/mobile/src/session/MobileNativeChatView.test.ts index 0c2a5623f36..dd5401b4898 100644 --- a/mobile/src/session/MobileNativeChatView.test.ts +++ b/mobile/src/session/MobileNativeChatView.test.ts @@ -180,6 +180,23 @@ describe('MobileNativeChatView', () => { }) } + async function relock(inputLockReason: Overrides['inputLockReason']): Promise { + await act(async () => { + renderer?.update( + createElement(MobileNativeChatView, { + messages: [], + status: 'ready', + agentWorking: true, + inputLockReason, + onSend: vi.fn().mockResolvedValue(true), + pending: [], + composerText: '', + onComposerTextChange: vi.fn() + }) + ) + }) + } + it('marks the working row stale and blocks Stop once a disconnect settles', async () => { vi.useFakeTimers() await render({ agentWorking: true, inputLockReason: 'disconnected' }) @@ -201,10 +218,11 @@ describe('MobileNativeChatView', () => { await settleLockDebounce() const stop = stopButton() + // `disabled` is the real block — Pressable never claims the responder. The + // dimmed style is the matching affordance, not the guard. expect(stop.props.disabled).toBe(true) - // Pressable swallows the press itself; assert the handler stays unreachable. expect(stop.props.style({ pressed: false })).toContainEqual(styles.stopDisabled) - expect(onStop).not.toHaveBeenCalled() + expect(stop.props.onPress).toBe(onStop) }) it('keeps the row live when the agent is merely waiting on a lease', async () => { @@ -216,26 +234,33 @@ describe('MobileNativeChatView', () => { expect(stopButton().props.disabled).toBe(false) }) + // The composer lock latches on the first non-null reason and never unlatches + // between reasons, so reading it directly would skip the hold on this edge. + it('still holds 600ms when a settled lease wait drops to disconnected', async () => { + vi.useFakeTimers() + await render({ agentWorking: true, inputLockReason: 'waiting' }) + await settleLockDebounce() + + await relock('disconnected') + + expect(workingIndicator().props.stale).toBe(false) + expect(stopButton().props.disabled).toBe(false) + + await settleLockDebounce() + + expect(workingIndicator().props.stale).toBe(true) + expect(stopButton().props.disabled).toBe(true) + }) + it('restores a live Stop as soon as the transport reconnects', async () => { vi.useFakeTimers() await render({ agentWorking: true, inputLockReason: 'disconnected' }) await settleLockDebounce() expect(stopButton().props.disabled).toBe(true) - await act(async () => { - renderer?.update( - createElement(MobileNativeChatView, { - messages: [], - status: 'ready', - agentWorking: true, - inputLockReason: null, - onSend: vi.fn().mockResolvedValue(true), - pending: [], - composerText: '', - onComposerTextChange: vi.fn() - }) - ) - }) + // Reconnect always lands on 'waiting' first — the lease drops its ready + // handles on disconnect, so it has to be re-acked before the lock clears. + await relock('waiting') // Unlock is immediate (no debounce on the way out), so Stop comes straight back. expect(workingIndicator().props.stale).toBe(false) diff --git a/mobile/src/session/MobileNativeChatView.tsx b/mobile/src/session/MobileNativeChatView.tsx index ff9adaf7f85..8ad897400a0 100644 --- a/mobile/src/session/MobileNativeChatView.tsx +++ b/mobile/src/session/MobileNativeChatView.tsx @@ -258,8 +258,18 @@ export function MobileNativeChatView({ const lockReason = lockHeld ? rawLockReason : null // Everything derived from agent status is pre-disconnect data while the // transport is down: mute the working row and disable Stop (a tap could not - // reach the PTY anyway). Shares the lock's debounce so blips don't flicker it. - const statusStale = lockReason === 'disconnected' + // reach the PTY anyway). Needs its own hold rather than reading `lockReason`, + // which stays latched across 'waiting' -> 'disconnected' and would strobe the + // row on a flapping link. Recovery is immediate, so Stop comes straight back. + const [statusStale, setStatusStale] = useState(false) + useEffect(() => { + if (rawLockReason !== 'disconnected') { + setStatusStale(false) + return + } + const timer = setTimeout(() => setStatusStale(true), 600) + return () => clearTimeout(timer) + }, [rawLockReason]) return (