diff --git a/apps/desktop/src/components/assistant-ui/thread/status.test.tsx b/apps/desktop/src/components/assistant-ui/thread/status.test.tsx new file mode 100644 index 000000000000..bd98e47a8b3f --- /dev/null +++ b/apps/desktop/src/components/assistant-ui/thread/status.test.tsx @@ -0,0 +1,255 @@ +import { act, cleanup, render } from '@testing-library/react' +import { AssistantRuntimeProvider, type ThreadMessage, useExternalStoreRuntime } from '@assistant-ui/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { I18nProvider } from '@/i18n' +import { ResponseLoadingIndicator } from './status' + +// Integration coverage for the elapsed-timer navigation fix (hermes-agent#62158): +// `ResponseLoadingIndicator` must resolve a stable `run:${messageId}` key from +// the running assistant message so the timer survives a view navigation +// (remount) instead of restarting from zero, and must restart when a new run +// (new assistant message id) begins. The component under test is the real +// `ResponseLoadingIndicator`; keying flows through the shared keyed +// `useElapsedSeconds` in activity-timer.ts. +function Harness({ messageId }: { messageId: string }) { + const createdAt = new Date('2026-01-01T00:00:00.000Z') + const messages: ThreadMessage[] = [ + { + id: 'user-1', + role: 'user', + content: [{ type: 'text', text: 'hi' }], + createdAt, + metadata: { + unstable_state: null, + unstable_annotations: [], + unstable_data: [], + steps: [], + custom: {} + }, + status: { type: 'complete', reason: 'stop' } + }, + { + id: messageId, + role: 'assistant', + content: [{ type: 'text', text: '' }], + createdAt, + metadata: { + unstable_state: 'running', + unstable_annotations: [], + unstable_data: [], + steps: [], + custom: {} + }, + status: { type: 'running' } + } + ] as ThreadMessage[] + + const runtime = useExternalStoreRuntime({ + messages, + isRunning: true, + onNew: async () => {} + }) + + return ( + + + + + + ) +} + +// Simulates the transition right after a new prompt is sent but before the +// runtime appends the run's assistant message: the last message is a user +// prompt (or a prior turn's completed assistant), with no in-flight assistant +// message yet. The indicator must NOT key on a stale prior-turn id. +function TransitionHarness() { + const createdAt = new Date('2026-01-01T00:00:00.000Z') + const messages: ThreadMessage[] = [ + { + id: 'user-1', + role: 'user', + content: [{ type: 'text', text: 'hi' }], + createdAt, + metadata: { + unstable_state: null, + unstable_annotations: [], + unstable_data: [], + steps: [], + custom: {} + }, + status: { type: 'complete', reason: 'stop' } + }, + { + id: 'assistant-prior', + role: 'assistant', + content: [{ type: 'text', text: 'done' }], + createdAt, + metadata: { + unstable_state: null, + unstable_annotations: [], + unstable_data: [], + steps: [], + custom: {} + }, + status: { type: 'complete', reason: 'stop' } + }, + { + id: 'user-2', + role: 'user', + content: [{ type: 'text', text: 'new prompt' }], + createdAt, + metadata: { + unstable_state: null, + unstable_annotations: [], + unstable_data: [], + steps: [], + custom: {} + }, + status: { type: 'complete', reason: 'stop' } + } + ] as ThreadMessage[] + + const runtime = useExternalStoreRuntime({ + messages, + isRunning: true, + onNew: async () => {} + }) + + return ( + + + + + + ) +} + +// Last message is a PRIOR turn's completed assistant (no running message +// appended yet) — the other transition shape Flash flagged. Must show a fresh +// timer, not a stale timestamp from the completed turn. +function CompletedLastHarness() { + const createdAt = new Date('2026-01-01T00:00:00.000Z') + const messages: ThreadMessage[] = [ + { + id: 'user-1', + role: 'user', + content: [{ type: 'text', text: 'hi' }], + createdAt, + metadata: { + unstable_state: null, + unstable_annotations: [], + unstable_data: [], + steps: [], + custom: {} + }, + status: { type: 'complete', reason: 'stop' } + }, + { + id: 'assistant-prior', + role: 'assistant', + content: [{ type: 'text', text: 'done' }], + createdAt, + metadata: { + unstable_state: null, + unstable_annotations: [], + unstable_data: [], + steps: [], + custom: {} + }, + status: { type: 'complete', reason: 'stop' } + } + ] as ThreadMessage[] + + const runtime = useExternalStoreRuntime({ + messages, + isRunning: true, + onNew: async () => {} + }) + + return ( + + + + + + ) +} + +describe('ResponseLoadingIndicator elapsed timer', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.setSystemTime(new Date('2026-01-01T00:00:00.000Z')) + }) + + afterEach(() => { + cleanup() + vi.useRealTimers() + }) + + it('keeps counting across a remount while the same run is active', () => { + const first = render() + + act(() => { + vi.advanceTimersByTime(9_000) + }) + + expect(first.container.textContent).toMatch(/9s/) + + first.unmount() + + act(() => { + vi.advanceTimersByTime(4_000) + }) + + const second = render() + expect(second.container.textContent).toMatch(/13s/) + }) + + it('resets when a new run (new assistant message id) starts', () => { + const first = render() + + act(() => { + vi.advanceTimersByTime(10_000) + }) + + expect(first.container.textContent).toMatch(/10s/) + + cleanup() + + const second = render() + expect(second.container.textContent).toMatch(/0s/) + }) + + it('does not reuse a stale prior-turn timestamp during the pre-append transition', () => { + // First, let a prior turn accumulate elapsed time under its own key. + const prior = render() + act(() => { + vi.advanceTimersByTime(30_000) + }) + expect(prior.container.textContent).toMatch(/30s/) + prior.unmount() + + // New prompt sent, but the runtime has not appended the run's assistant + // message yet: the last message is the user prompt. The indicator must + // start a FRESH timer (0s), not reuse the prior turn's 30s timestamp. + const transition = render() + expect(transition.container.textContent).toMatch(/0s/) + }) + + it('does not reuse a stale timestamp when the last message is a completed prior-turn assistant', () => { + // Let a prior turn accumulate elapsed time under its own key. + const prior = render() + act(() => { + vi.advanceTimersByTime(45_000) + }) + expect(prior.container.textContent).toMatch(/45s/) + prior.unmount() + + // The runtime has not appended the new run's assistant message yet, so the + // last message is the previous completed assistant. Must start fresh (0s). + const completed = render() + expect(completed.container.textContent).toMatch(/0s/) + }) +}) diff --git a/apps/desktop/src/components/assistant-ui/thread/status.tsx b/apps/desktop/src/components/assistant-ui/thread/status.tsx index 53c6f415a85f..ab31fc570463 100644 --- a/apps/desktop/src/components/assistant-ui/thread/status.tsx +++ b/apps/desktop/src/components/assistant-ui/thread/status.tsx @@ -57,9 +57,40 @@ export const CenteredThreadSpinner: FC = () => { ) } +// Key the chat-level elapsed timer on the running assistant message id so it +// survives view navigation (chat switch, Settings open/close) instead of +// restarting from zero on every remount. `ResponseLoadingIndicator` renders at +// the thread-list scope (no message context), so it reads the id of the last +// running assistant message from `s.thread.messages`; `StreamStallIndicator` +// renders inside the message and reads `s.message.id` directly. Both key on the +// same `run:` prefix, so the pre-first-token -> streaming -> stall phases of one +// turn count continuously, and a new prompt (new message id) resets the timer. +const RUN_KEY_PREFIX = 'run:' + +function runningAssistantMessageIdFromThread(): string | undefined { + const thread = useAuiState(s => s.thread) + const messages = thread?.messages + if (!messages || messages.length === 0) { + return undefined + } + // Only the running assistant message counts. The runtime appends the run's + // assistant message (status `running`) as the last message once a turn + // starts, but during the transition before that append — or if a prior + // turn's completed assistant message is still last — the last message is not + // the in-flight one. Keying on either would reuse a stale timestamp and show + // a huge bogus elapsed time, so require the last message to be an assistant + // message that is currently running. + const last = messages[messages.length - 1] + if (last.role !== 'assistant' || last.status?.type !== 'running') { + return undefined + } + return last.id +} + export const ResponseLoadingIndicator: FC = () => { const { t } = useI18n() - const elapsed = useElapsedSeconds() + const runId = runningAssistantMessageIdFromThread() + const elapsed = useElapsedSeconds(true, runId ? `${RUN_KEY_PREFIX}${runId}` : undefined) const compacting = useStore($compactionActive) return ( @@ -147,7 +178,8 @@ export const StreamStallIndicator: FC = () => { }, [activity]) const active = (stalled || compacting) && !awaitingInput - const elapsed = useElapsedSeconds(active) + const messageId = useAuiState(s => s.message.id) + const elapsed = useElapsedSeconds(active, messageId ? `${RUN_KEY_PREFIX}${messageId}` : undefined) if (!active) { return null diff --git a/apps/desktop/src/components/chat/activity-timer.test.tsx b/apps/desktop/src/components/chat/activity-timer.test.tsx index acc70a99ed06..0cde361b5c14 100644 --- a/apps/desktop/src/components/chat/activity-timer.test.tsx +++ b/apps/desktop/src/components/chat/activity-timer.test.tsx @@ -1,4 +1,4 @@ -import { act, render, screen } from '@testing-library/react' +import { act, cleanup, render, screen } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { __resetElapsedTimerRegistryForTests, useElapsedSeconds } from './activity-timer' @@ -17,6 +17,7 @@ describe('useElapsedSeconds', () => { }) afterEach(() => { + cleanup() vi.useRealTimers() __resetElapsedTimerRegistryForTests() }) @@ -40,4 +41,44 @@ describe('useElapsedSeconds', () => { expect(screen.getByTestId('elapsed').textContent).toBe('8') }) + + it('resets elapsed time when the key changes (new run)', () => { + const first = render() + + act(() => { + vi.advanceTimersByTime(10_000) + }) + + expect(screen.getByTestId('elapsed').textContent).toBe('10') + + first.unmount() + + // A new prompt produces a new message id -> the timer must restart from 0. + render() + + expect(screen.getByTestId('elapsed').textContent).toBe('0') + }) + + it('survives a navigation away/back (remount with the same run key)', () => { + // Simulates the chat view unmounting (chat switch / Settings) and remounting + // while the same agent turn is still running. The run: key must persist. + const first = render() + + act(() => { + vi.advanceTimersByTime(12_000) + }) + + expect(screen.getByTestId('elapsed').textContent).toBe('12') + + first.unmount() + + // Time passes while the view is away; the run keeps counting in the registry. + act(() => { + vi.advanceTimersByTime(5_000) + }) + + render() + + expect(screen.getByTestId('elapsed').textContent).toBe('17') + }) }) diff --git a/apps/desktop/src/components/chat/activity-timer.ts b/apps/desktop/src/components/chat/activity-timer.ts index 533dc5b373cf..1f27432d10e4 100644 --- a/apps/desktop/src/components/chat/activity-timer.ts +++ b/apps/desktop/src/components/chat/activity-timer.ts @@ -1,9 +1,15 @@ import { useEffect, useRef, useState } from 'react' // Module-level registry so timers survive component unmount/remount (e.g. -// when a tool row scrolls out and back). Keyed by caller-supplied timerKey; -// anonymous timers (no key) start fresh each mount. +// when a tool row scrolls out and back, or the chat view is navigated away +// from and back). Keyed by caller-supplied timerKey; anonymous timers (no +// key) start fresh each mount. +// +// Bounded by an LRU cap: each run leaves one entry behind, and a long-lived +// desktop session would otherwise accumulate entries forever. Map preserves +// insertion order, so `.keys().next().value` is the oldest entry. const startedAtByKey = new Map() +const MAX_ENTRIES = 1000 function startedAt(key?: string): number { if (!key) { @@ -19,6 +25,13 @@ function startedAt(key?: string): number { const now = Date.now() startedAtByKey.set(key, now) + if (startedAtByKey.size > MAX_ENTRIES) { + const oldest = startedAtByKey.keys().next().value + if (oldest !== undefined) { + startedAtByKey.delete(oldest) + } + } + return now }