Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
255 changes: 255 additions & 0 deletions apps/desktop/src/components/assistant-ui/thread/status.test.tsx
Original file line number Diff line number Diff line change
@@ -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<ThreadMessage>({
messages,
isRunning: true,
onNew: async () => {}
})

return (
<I18nProvider>
<AssistantRuntimeProvider runtime={runtime}>
<ResponseLoadingIndicator />
</AssistantRuntimeProvider>
</I18nProvider>
)
}

// 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<ThreadMessage>({
messages,
isRunning: true,
onNew: async () => {}
})

return (
<I18nProvider>
<AssistantRuntimeProvider runtime={runtime}>
<ResponseLoadingIndicator />
</AssistantRuntimeProvider>
</I18nProvider>
)
}

// 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<ThreadMessage>({
messages,
isRunning: true,
onNew: async () => {}
})

return (
<I18nProvider>
<AssistantRuntimeProvider runtime={runtime}>
<ResponseLoadingIndicator />
</AssistantRuntimeProvider>
</I18nProvider>
)
}

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(<Harness messageId="run-a" />)

act(() => {
vi.advanceTimersByTime(9_000)
})

expect(first.container.textContent).toMatch(/9s/)

first.unmount()

act(() => {
vi.advanceTimersByTime(4_000)
})

const second = render(<Harness messageId="run-a" />)
expect(second.container.textContent).toMatch(/13s/)
})

it('resets when a new run (new assistant message id) starts', () => {
const first = render(<Harness messageId="run-a" />)

act(() => {
vi.advanceTimersByTime(10_000)
})

expect(first.container.textContent).toMatch(/10s/)

cleanup()

const second = render(<Harness messageId="run-b" />)
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(<Harness messageId="assistant-prior" />)
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(<TransitionHarness />)
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(<Harness messageId="assistant-prior" />)
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(<CompletedLastHarness />)
expect(completed.container.textContent).toMatch(/0s/)
})
})
36 changes: 34 additions & 2 deletions apps/desktop/src/components/assistant-ui/thread/status.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useAuiState is called inside this function (line 71), but this lowercase name is neither a component nor a custom hook. apps/desktop/eslint.config.mjs:99-100 enables react-hooks/rules-of-hooks as an error. Rename this to useRunningAssistantMessageIdFromThread or move the selector into ResponseLoadingIndicator.

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 (
Expand Down Expand Up @@ -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
Expand Down
43 changes: 42 additions & 1 deletion apps/desktop/src/components/chat/activity-timer.test.tsx
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -17,6 +17,7 @@ describe('useElapsedSeconds', () => {
})

afterEach(() => {
cleanup()
vi.useRealTimers()
__resetElapsedTimerRegistryForTests()
})
Expand All @@ -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(<Probe active timerKey="run:msg-1" />)

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(<Probe active timerKey="run:msg-2" />)

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(<Probe active timerKey="run:msg-1" />)

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(<Probe active timerKey="run:msg-1" />)

expect(screen.getByTestId('elapsed').textContent).toBe('17')
})
})
17 changes: 15 additions & 2 deletions apps/desktop/src/components/chat/activity-timer.ts
Original file line number Diff line number Diff line change
@@ -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<string, number>()
const MAX_ENTRIES = 1000

function startedAt(key?: string): number {
if (!key) {
Expand All @@ -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
}

Expand Down
Loading