diff --git a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/lifecycle.ts b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/lifecycle.ts index 62511926a9..def9e97780 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/lifecycle.ts +++ b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/lifecycle.ts @@ -12,6 +12,7 @@ import { type PetChangeMeta, setChangeEventsAvailable } from '@/store/live-sync' +import { clearAllPrompts, clearClarifyRequest } from '@/store/prompts' import { markRuntimeGone } from '@/store/runtime-gone' import { dropSessionState, unbindTileRuntime } from '@/store/session-states' // Leaf import (not the `@/themes` barrel) to avoid pulling the ThemeProvider @@ -105,6 +106,13 @@ export function handleLifecycleEvent(ctx: GatewayEventContext): boolean { // Heal while the cached stored-id mapping is still intact, then drop. markRuntimeGone(reclaimedRuntimeId) dropSessionState(reclaimedRuntimeId) + // A prompt keyed to the dead runtime must not outlive it. The runtime id + // rotates on every resume (cold/lazy/eager all mint a fresh sid), so the + // new runtime's turn-end clears can never remove an entry keyed to THIS + // one — a stale approval would re-mount the floating "needs approval" + // bar whenever the reclaimed conversation is reopened (#86577). + clearAllPrompts(reclaimedRuntimeId) + clearClarifyRequest(undefined, reclaimedRuntimeId) // A tile bound to the reclaimed runtime would otherwise render an // empty transcript forever: its view reads $sessionStates[runtime] // (just dropped) and its resume effect is gated on !runtimeId, so a diff --git a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/session-info.ts b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/session-info.ts index 429f1b4cd8..257f05fad1 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/session-info.ts +++ b/apps/desktop/src/app/session/hooks/use-message-stream/gateway-event/session-info.ts @@ -4,6 +4,7 @@ import { reconcileApprovalModeForProfile } from '@/store/approval-mode' import { reconcileSessionCompacting } from '@/store/compaction' import { requestDesktopOnboardingForCredentialWarning } from '@/store/onboarding' import { followActiveSessionCwd } from '@/store/projects' +import { clearAllPrompts, clearClarifyRequest } from '@/store/prompts' import { $activeSessionId, $currentCwd, @@ -284,6 +285,18 @@ export function handleSessionInfoEvent(ctx: GatewayEventContext): boolean { // mutates the per-runtime cache entry, and syncSessionStateToView // guards the view publish to the active session, so this is safe. if (runningChanged && sessionId) { + // The agent loop's finally block emits running=false even when a + // reconnect gap or provider crash swallowed message.complete — and + // message.complete is where the turn-end prompt clear lives. An + // approval left parked by that miss re-mounts the floating "needs + // approval" bar on a session whose turn is already finished, so treat + // the end of a turn we knew was live as an authoritative clear edge + // too (#86577). Bystander sessions keep their prompts: the clear is + // scoped to this sessionId. + if (!payload!.running && (knownState?.busy || knownState?.awaitingResponse)) { + clearAllPrompts(sessionId) + clearClarifyRequest(undefined, sessionId) + } // Set when THIS event releases a confirmed live turn whose terminal // message never arrived. The updater is invoked exactly once, // synchronously, by updateSessionState. diff --git a/apps/desktop/src/app/session/hooks/use-message-stream/session-reclaimed.test.tsx b/apps/desktop/src/app/session/hooks/use-message-stream/session-reclaimed.test.tsx index 7058eadaa0..8947303414 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream/session-reclaimed.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-message-stream/session-reclaimed.test.tsx @@ -5,6 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { ClientSessionState } from '@/app/types' import { createClientSessionState } from '@/lib/chat-runtime' +import { clearAllPrompts, sessionApprovalRequest, setApprovalRequest } from '@/store/prompts' import { resetRuntimeGoneHealing } from '@/store/runtime-gone' import { $activeSessionId, $sessionResumeRequest } from '@/store/session' import { $sessionStates, $sessionTiles, publishSessionState } from '@/store/session-states' @@ -43,6 +44,7 @@ beforeEach(() => { $sessionTiles.set([]) $activeSessionId.set(null) $sessionResumeRequest.set(null) + clearAllPrompts() }) afterEach(() => { @@ -52,6 +54,7 @@ afterEach(() => { $sessionTiles.set([]) $activeSessionId.set(null) $sessionResumeRequest.set(null) + clearAllPrompts() vi.restoreAllMocks() }) @@ -69,6 +72,21 @@ describe('session.reclaimed', () => { expect($sessionStates.get()['live-kept']).toBeDefined() }) + // The runtime id rotates on every resume, so a prompt keyed to the reclaimed + // runtime can never be cleared by the NEW runtime's turn-end edges. Left + // behind, it re-mounts the floating "needs approval" bar on a finished + // conversation whenever it is reopened (#86577). + it('retires only the reclaimed runtime approval', () => { + mountStream() + setApprovalRequest({ command: 'rm stale', description: 'stale request', sessionId: 'live-gone' }) + setApprovalRequest({ command: 'rm kept', description: 'kept request', sessionId: 'live-kept' }) + + reclaim('live-gone') + + expect(sessionApprovalRequest('live-gone').get()).toBeNull() + expect(sessionApprovalRequest('live-kept').get()?.command).toBe('rm kept') + }) + it('ignores a payload with no runtime id instead of clearing everything', () => { mountStream() publishSessionState('live-a', createClientSessionState()) diff --git a/apps/desktop/src/app/session/hooks/use-message-stream/stale-pending-settle.test.tsx b/apps/desktop/src/app/session/hooks/use-message-stream/stale-pending-settle.test.tsx index ad0182177e..b0ddda2a82 100644 --- a/apps/desktop/src/app/session/hooks/use-message-stream/stale-pending-settle.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-message-stream/stale-pending-settle.test.tsx @@ -6,12 +6,15 @@ import type { GatewayEvent } from '@hermes/shared' // running=false is the turn's finally-block signal and the only settle edge // those paths still emit, so it must finalize the bubble. import { act, cleanup } from '@testing-library/react' -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { clearAllPrompts, sessionApprovalRequest, setApprovalRequest } from '@/store/prompts' import { type MessageStreamHarness, renderMessageStream } from './test-harness' import { STREAM_DELTA_FLUSH_MS } from './utils' const SID = 'stale-pending-session' +const OTHER_SID = 'other-session' let stream: MessageStreamHarness @@ -32,8 +35,13 @@ const flushDeltas = async () => { const emit = (event: GatewayEvent) => act(() => stream.handleEvent(event)) describe('turn end without message.complete (session.info running=false)', () => { + beforeEach(() => { + clearAllPrompts() + }) + afterEach(() => { cleanup() + clearAllPrompts() vi.useRealTimers() vi.restoreAllMocks() }) @@ -84,4 +92,49 @@ describe('turn end without message.complete (session.info running=false)', () => expect(state?.messages.every(message => !message.pending)).toBe(true) expect(state?.streamId).toBeNull() }) + + // A turn whose message.complete was swallowed (reconnect gap, provider + // crash) used to leave its approval entry parked: the floating "needs + // approval" bar kept reappearing on a session the sidebar already showed + // as finished. running=false is the agent loop's finally-block edge — it + // must clear the turn's prompts just like message.complete does (#86577). + it('retires the finished session approval when message.complete was missed', async () => { + await mountHarness() + + emit({ session_id: SID, type: 'message.start', payload: {} }) + await act(async () => { + stream.handleRequest('approval', { + command: 'rm -rf stale', + description: 'stale request', + session_id: SID + }) + }) + setApprovalRequest({ command: 'rm other', description: 'other request', sessionId: OTHER_SID }) + + expect(sessionApprovalRequest(SID).get()?.command).toBe('rm -rf stale') + + emit({ payload: { running: false }, session_id: SID, type: 'session.info' }) + + expect(sessionApprovalRequest(SID).get()).toBeNull() + // Bystander sessions keep their prompts: only the finished turn clears. + expect(sessionApprovalRequest(OTHER_SID).get()?.command).toBe('rm other') + }) + + // An idle session's running=false heartbeat carries no turn edge, so it + // must not retire a prompt another session's turn just raised. + it('does not clear prompts on a running=false heartbeat for a session that was never busy', async () => { + await mountHarness() + + await act(async () => { + stream.handleRequest('approval', { + command: 'tail -f log', + description: 'live request', + session_id: SID + }) + }) + + emit({ payload: { running: false }, session_id: SID, type: 'session.info' }) + + expect(sessionApprovalRequest(SID).get()?.command).toBe('tail -f log') + }) })