fix(desktop): stop approval mascot popup loop

Co-authored-by: DotCraft <273930855+dotcraft-ai@users.noreply.github.com>

Author: AkiKurisu

desktop/src/renderer/components/conversation/useComposerMascot.ts

@@ -92,12 +92,16 @@ export function useComposerMascot({
     if (threadChanged) {
       if (lastTurnId) handledRef.current = `${lastTurnId}:${lastTurnStatus}`
       dismissedApprovalRef.current = null
-      setLocal(turnStatus === 'waitingApproval' ? { kind: 'approval' } : null)
+      setLocal(turnStatus === 'waitingApproval' && approvalItemId ? { kind: 'approval' } : null)
       return
     }
 
     if (turnStatus === 'waitingApproval') {
-      if (approvalItemId && dismissedApprovalRef.current === approvalItemId) return
+      if (!approvalItemId) {
+        setLocal((prev) => (prev?.kind === 'approval' ? null : prev))
+        return
+      }
+      if (dismissedApprovalRef.current === approvalItemId) return
       setLocal((prev) => (prev?.kind === 'approval' ? prev : { kind: 'approval' }))
       return
     }

desktop/src/renderer/stores/conversationStore.ts

@@ -1941,6 +1941,11 @@ export const useConversationStore = create<ConversationStore>((set, get) => ({
           : 'idle'
     const compactedNotice = latestCompactedNotice(rehydratedTurns)
     set((state) => {
+      // A stale thread/read can still report waitingApproval after the local decision
+      // has already cleared its request.
+      const restoredTurnStatus = activeTurnStatus === 'waitingApproval' && state.pendingApproval == null
+        ? 'running'
+        : activeTurnStatus
       const terminalApplied = applyPendingTerminalsToTurns(
         turnsForState,
         state.pendingTerminalByCallId
@@ -1951,7 +1956,7 @@ export const useConversationStore = create<ConversationStore>((set, get) => ({
       )
       return {
         turns: toolCompletionApplied.turns,
-        turnStatus: activeTurnStatus,
+        turnStatus: restoredTurnStatus,
         activeTurnId: activeTurn ? activeTurn.id : null,
         interruptingTurnId: null,
         streamingMessage: preserveEmptyRealtimeSnapshot ? state.streamingMessage : '',

desktop/src/renderer/tests/ApprovalDecisionComposer.test.tsx

@@ -146,6 +146,43 @@ describe('ApprovalDecisionComposer', () => {
     expect(onResponseAccepted).toHaveBeenCalledTimes(1)
   })
 
+  it('does not re-enter waitingApproval when a stale snapshot arrives after submission', async () => {
+    const pending = pendingApproval()
+    useConversationStore.setState({
+      turns: [{
+        id: 'turn-approval',
+        threadId: 'thread-approval',
+        status: 'running',
+        items: [],
+        startedAt: new Date().toISOString()
+      }],
+      activeTurnId: 'turn-approval'
+    })
+    setPendingApproval(pending)
+    renderWithLocale(<ApprovalDecisionComposer request={pending} />)
+
+    fireEvent.keyDown(window, { key: 'Enter' })
+    await waitFor(() => {
+      expect(useConversationStore.getState().pendingApproval).toBeNull()
+    })
+
+    act(() => {
+      useConversationStore.getState().setTurns([{
+        id: 'turn-approval',
+        threadId: 'thread-approval',
+        status: 'waitingApproval',
+        items: [],
+        startedAt: new Date().toISOString()
+      }], {
+        preserveExistingRealtime: true,
+        realtimeScopeThreadId: 'thread-approval'
+      })
+    })
+
+    expect(useConversationStore.getState().turnStatus).toBe('running')
+    expect(useConversationStore.getState().pendingApproval).toBeNull()
+  })
+
   it('uses number keys and Arrow keys to submit the selected decision', async () => {
     const pending = pendingApproval()
     setPendingApproval(pending)

desktop/src/renderer/tests/conversationStore.test.ts

@@ -1218,6 +1218,47 @@ describe('turn lifecycle', () => {
     expect(state.activeTurnId).toBeNull()
   })
 
+  it('does not resurrect waitingApproval from a stale snapshot after its request was resolved', () => {
+    s().onTurnStarted(makeTurn())
+    s().onApprovalRequest('bridge-stale-approval', {
+      threadId: 'thread-1',
+      turnId: 'turn-1',
+      requestId: 'request-stale-approval',
+      itemId: 'approval-stale-approval',
+      approvalType: 'shell',
+      operation: 'npm test'
+    })
+    s().onApprovalDecision('accept')
+
+    s().setTurns([makeTurn({ status: 'waitingApproval' })], {
+      preserveExistingRealtime: true,
+      realtimeScopeThreadId: 'thread-1'
+    })
+
+    expect(s().pendingApproval).toBeNull()
+    expect(s().turnStatus).toBe('running')
+  })
+
+  it('restores waitingApproval when the actionable request is still present', () => {
+    s().onTurnStarted(makeTurn())
+    s().onApprovalRequest('bridge-live-approval', {
+      threadId: 'thread-1',
+      turnId: 'turn-1',
+      requestId: 'request-live-approval',
+      itemId: 'approval-live-approval',
+      approvalType: 'shell',
+      operation: 'npm test'
+    })
+
+    s().setTurns([makeTurn({ status: 'waitingApproval' })], {
+      preserveExistingRealtime: true,
+      realtimeScopeThreadId: 'thread-1'
+    })
+
+    expect(s().pendingApproval?.requestId).toBe('request-live-approval')
+    expect(s().turnStatus).toBe('waitingApproval')
+  })
+
   it('does not preserve terminal history over a same-thread empty snapshot', () => {
     s().setTurns([
       makeTurn({
@@ -2248,12 +2289,12 @@ describe('setTurns', () => {
     expect(s().turnStartedAt).not.toBeNull()
   })
 
-  it('restores waitingApproval as the active turn state', () => {
+  it('keeps a waitingApproval turn active but runnable until its request is replayed', () => {
     s().setTurns([
       makeTurn({ id: 'turn-wait-approval', status: 'waitingApproval', items: [] })
     ])
 
-    expect(s().turnStatus).toBe('waitingApproval')
+    expect(s().turnStatus).toBe('running')
     expect(s().activeTurnId).toBe('turn-wait-approval')
     expect(s().turnStartedAt).not.toBeNull()
   })

desktop/src/renderer/tests/useComposerMascot.test.tsx

@@ -0,0 +1,115 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'
+import { LocaleProvider } from '../contexts/LocaleContext'
+import { useComposerMascot } from '../components/conversation/useComposerMascot'
+import { useConversationStore, type PendingApproval } from '../stores/conversationStore'
+import { installDesktopApiMock } from './desktopApiMock'
+
+const THREAD_ID = 'thread-mascot'
+
+function approval(overrides: Partial<PendingApproval> = {}): PendingApproval {
+  return {
+    bridgeId: 'bridge-mascot-1',
+    threadId: THREAD_ID,
+    turnId: 'turn-mascot',
+    requestId: 'request-mascot-1',
+    locallySubmittedDecision: null,
+    itemId: 'approval-mascot-1',
+    approvalType: 'shell',
+    operation: 'npm test',
+    target: '<workspace>',
+    reason: 'Run the test suite.',
+    ...overrides
+  }
+}
+
+function MascotHarness(): JSX.Element {
+  const interaction = useComposerMascot({ threadId: THREAD_ID, workspacePath: '<workspace>' })
+  const bubble = interaction?.bubble
+  return (
+    <div>
+      {bubble && (
+        <section aria-label="mascot bubble">
+          <h1>{bubble.title}</h1>
+          {bubble.body && <p>{bubble.body}</p>}
+          {bubble.actions?.map((action) => (
+            <button key={action.label} type="button" onClick={action.onClick}>
+              {action.label}
+            </button>
+          ))}
+        </section>
+      )}
+    </div>
+  )
+}
+
+function renderHarness(): void {
+  render(
+    <LocaleProvider>
+      <MascotHarness />
+    </LocaleProvider>
+  )
+}
+
+describe('useComposerMascot approval nudge', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    installDesktopApiMock({
+      settings: { get: vi.fn().mockResolvedValue({ locale: 'en' }) }
+    })
+    useConversationStore.getState().reset()
+  })
+
+  it('does not show an approval bubble for a waiting status without an actionable request', () => {
+    useConversationStore.setState({
+      turnStatus: 'waitingApproval',
+      pendingApproval: null,
+      pendingApprovals: []
+    })
+
+    renderHarness()
+
+    expect(screen.queryByRole('heading', { name: 'Your approval is needed' })).not.toBeInTheDocument()
+  })
+
+  it('keeps the same approval dismissed across rerenders and replay, but shows a new request', async () => {
+    const first = approval()
+    useConversationStore.setState({
+      turnStatus: 'waitingApproval',
+      pendingApproval: first,
+      pendingApprovals: [first]
+    })
+
+    renderHarness()
+
+    expect(await screen.findByRole('heading', { name: 'Your approval is needed' })).toBeInTheDocument()
+    fireEvent.click(screen.getByRole('button', { name: 'Got it' }))
+    await waitFor(() => {
+      expect(screen.queryByRole('heading', { name: 'Your approval is needed' })).not.toBeInTheDocument()
+    })
+
+    const replayed = { ...first, reason: 'Same request replayed with updated details.' }
+    act(() => {
+      useConversationStore.setState({
+        pendingApproval: replayed,
+        pendingApprovals: [replayed]
+      })
+    })
+    expect(screen.queryByRole('heading', { name: 'Your approval is needed' })).not.toBeInTheDocument()
+
+    const second = approval({
+      bridgeId: 'bridge-mascot-2',
+      requestId: 'request-mascot-2',
+      itemId: 'approval-mascot-2',
+      reason: 'Run the next command.'
+    })
+    act(() => {
+      useConversationStore.setState({
+        pendingApproval: second,
+        pendingApprovals: [second]
+      })
+    })
+
+    expect(await screen.findByRole('heading', { name: 'Your approval is needed' })).toBeInTheDocument()
+  })
+})