From 05c48b2a97882d6d87164834d60bc941134f2dfc Mon Sep 17 00:00:00 2001 From: ethernet Date: Wed, 22 Jul 2026 17:58:33 -0400 Subject: [PATCH] fix(desktop): avoid duplicate inflight user rows on resume Keep a live session projection from adding its user turn when the latest persisted row already represents that same inflight prompt. Add real Electron coverage for fast and cold resume with idle and background-inference sessions. --- apps/desktop/e2e/fixtures.ts | 10 +- apps/desktop/e2e/large-session-resume.spec.ts | 230 ++++++++++++++++++ apps/desktop/e2e/mock-server.ts | 50 +++- .../desktop/e2e/scripts/seed_large_session.py | 52 ++++ .../hooks/use-session-actions/index.ts | 49 ++-- .../hooks/use-session-actions/utils.test.ts | 26 ++ .../hooks/use-session-actions/utils.ts | 11 +- 7 files changed, 393 insertions(+), 35 deletions(-) create mode 100644 apps/desktop/e2e/large-session-resume.spec.ts create mode 100644 apps/desktop/e2e/scripts/seed_large_session.py diff --git a/apps/desktop/e2e/fixtures.ts b/apps/desktop/e2e/fixtures.ts index 7d0ea6be25c9..b03321f051c1 100644 --- a/apps/desktop/e2e/fixtures.ts +++ b/apps/desktop/e2e/fixtures.ts @@ -27,7 +27,7 @@ import * as path from 'node:path' import { _electron, type ElectronApplication, type Page } from '@playwright/test' -import { startMockServer } from './mock-server' +import { startMockServer, type MockServerOptions } from './mock-server' import { installErrorBannerGuard } from './test' const DESKTOP_ROOT = path.resolve(import.meta.dirname, '..') @@ -331,9 +331,13 @@ export interface MockBackendFixture { * 3. Launch the desktop app * 4. Return handles for test interaction */ -export async function setupMockBackend(): Promise { +export interface MockBackendOptions { + mockServer?: MockServerOptions +} + +export async function setupMockBackend(options: MockBackendOptions = {}): Promise { // 1. Start mock server - const mock = await startMockServer() + const mock = await startMockServer(options.mockServer) // 2. Create sandbox + write config const sandbox = createSandbox('mock') diff --git a/apps/desktop/e2e/large-session-resume.spec.ts b/apps/desktop/e2e/large-session-resume.spec.ts new file mode 100644 index 000000000000..7ea72795a8ae --- /dev/null +++ b/apps/desktop/e2e/large-session-resume.spec.ts @@ -0,0 +1,230 @@ +import { spawnSync } from 'node:child_process' +import * as path from 'node:path' + +import { type TestInfo } from '@playwright/test' + +import { expect, test, type ElectronApplication, type Page } from './test' + +import { + buildAppEnv, + createSandbox, + launchDesktop, + type Sandbox, + waitForAppReady, + writeEnvFile, + writeMockProviderConfig, +} from './fixtures' +import { MOCK_REPLY, startMockServer, type MockServer, type MockServerOptions } from './mock-server' + +const DESKTOP_ROOT = path.resolve(import.meta.dirname, '..') +const REPO_ROOT = path.resolve(DESKTOP_ROOT, '..', '..') +const SEED_SCRIPT = path.resolve(import.meta.dirname, 'scripts', 'seed_large_session.py') +const SESSION_TITLE = 'E2E large persisted session' +const EXPECTED_TEXT = 'E2E persisted user message 52' +const BACKGROUND_PROMPT = 'E2E background inference must remain attached across resume' + +interface SeededFixture { + app: ElectronApplication + mock: MockServer + mockUrl: string + page: Page + sandbox: Sandbox + cleanup: () => Promise +} + +interface PaintState { + bursts: number + timeline: Array<{ mutations: number; time: number }> +} + +async function setupSeededDesktop(mockServer?: MockServerOptions): Promise { + const mock = await startMockServer(mockServer) + const sandbox = createSandbox('large-session') + writeMockProviderConfig(sandbox.hermesHome, mock.url) + writeEnvFile(sandbox.hermesHome) + + const seeded = spawnSync('python3', [SEED_SCRIPT, path.join(sandbox.hermesHome, 'state.db')], { + cwd: REPO_ROOT, + encoding: 'utf8', + env: { ...process.env, PYTHONPATH: REPO_ROOT }, + }) + if (seeded.status !== 0) { + throw new Error(`large-session seed failed:\n${seeded.stdout}\n${seeded.stderr}`) + } + + const { app, page } = await launchDesktop(buildAppEnv(sandbox)) + + return { + app, + mock, + mockUrl: mock.url, + page, + sandbox, + cleanup: async () => { + await app.close().catch(() => undefined) + await mock.close() + sandbox.cleanup() + }, + } +} + +function sessionRow(page: Page) { + return page.locator('[data-slot="sidebar"] button').filter({ hasText: SESSION_TITLE }).first() +} + +async function openSeededSession(page: Page): Promise { + const row = sessionRow(page) + await row.waitFor({ state: 'visible', timeout: 60_000 }) + await row.click() + await page.waitForFunction( + expected => (document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '').includes(expected), + EXPECTED_TEXT, + { timeout: 30_000 }, + ) +} + +async function openNewSession(page: Page): Promise { + const button = page.locator('[data-slot="sidebar"] button').filter({ hasText: 'New session' }).first() + await button.waitFor({ state: 'visible', timeout: 10_000 }) + await button.click() + await page.waitForFunction( + expected => !(document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '').includes(expected), + EXPECTED_TEXT, + { timeout: 15_000 }, + ) +} + +async function submitPrompt(page: Page, prompt: string): Promise { + const composer = page.locator('[contenteditable="true"]').first() + await composer.waitFor({ state: 'visible', timeout: 15_000 }) + await composer.click() + await composer.type(prompt, { delay: 2 }) + await page.keyboard.press('Enter') + await page.waitForFunction( + expected => (document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '').includes(expected), + prompt, + { timeout: 15_000 }, + ) +} + +async function startPaintObserver(page: Page): Promise { + await page.evaluate(() => { + const viewport = document.querySelector('[data-slot="aui_thread-viewport"]') + const state = { bursts: 0, timeline: [] as Array<{ mutations: number; time: number }> } + ;(window as Window & { __largeSessionPaints?: typeof state }).__largeSessionPaints = state + if (!viewport) return + + let additions = 0 + let flushTimer: ReturnType | undefined + new MutationObserver(records => { + additions += records.reduce( + (count, record) => count + (record.type === 'childList' && record.addedNodes.length > 0 ? 1 : 0), + 0, + ) + if (additions === 0) return + if (flushTimer) clearTimeout(flushTimer) + flushTimer = setTimeout(() => { + state.bursts += 1 + state.timeline.push({ mutations: additions, time: Date.now() }) + additions = 0 + }, 30) + }).observe(viewport, { childList: true, subtree: true }) + }) +} + +async function paintState(page: Page): Promise { + const state = await page.evaluate(() => (window as Window & { __largeSessionPaints?: PaintState }).__largeSessionPaints) + expect(state, 'paint observer should attach to the thread viewport').toBeDefined() + return state! +} + +async function textNodeOccurrences(page: Page, expected: string): Promise { + return page.evaluate(text => { + const viewport = document.querySelector('[data-slot="aui_thread-viewport"]') + if (!viewport) return 0 + + const walker = document.createTreeWalker(viewport, NodeFilter.SHOW_TEXT) + let count = 0 + while (walker.nextNode()) { + if (walker.currentNode.textContent?.includes(text)) { + count += 1 + } + } + return count + }, expected) +} + +async function reloadIntoColdRenderer(fixture: SeededFixture): Promise { + await fixture.page.reload() + await waitForAppReady(fixture, 120_000) + await openNewSession(fixture.page) +} + +async function assertUnchangedResume(page: Page, testInfo: TestInfo): Promise { + await openSeededSession(page) + await page.waitForTimeout(1_000) + await page.screenshot({ path: testInfo.outputPath('unchanged-session-resume.png'), fullPage: false }) + + const paints = await paintState(page) + expect(await textNodeOccurrences(page, EXPECTED_TEXT), 'the resumed user message should appear once').toBe(1) + // A warm session first restores its retained view, then reconciles it with the + // authoritative transcript. That is bounded at two builds; a third paint was + // the old eager-prefetch + runtime-rebuild regression. A cold restore has one. + expect(paints.bursts, `unexpected transcript paint count: ${JSON.stringify(paints.timeline)}`).toBeLessThanOrEqual(2) +} + +test.describe('large session resume', () => { + let fixture: SeededFixture | null = null + + test.afterEach(async () => { + await fixture?.cleanup() + fixture = null + }) + + test('cold resume of an unchanged session has one user row and bounded transcript paints', async ({}, testInfo) => { + fixture = await setupSeededDesktop() + await waitForAppReady(fixture, 120_000) + + await startPaintObserver(fixture.page) + await assertUnchangedResume(fixture.page, testInfo) + }) + + test('fast resume of an unchanged session has one user row and bounded transcript paints', async ({}, testInfo) => { + fixture = await setupSeededDesktop() + await waitForAppReady(fixture, 120_000) + + await openSeededSession(fixture.page) + await openNewSession(fixture.page) + await startPaintObserver(fixture.page) + await assertUnchangedResume(fixture.page, testInfo) + }) + + for (const resumeKind of ['fast', 'cold'] as const) { + test(`${resumeKind} resume keeps background inference attached without duplicate messages`, async ({}, testInfo) => { + fixture = await setupSeededDesktop({ holdFirstStreamForPrompt: BACKGROUND_PROMPT }) + await waitForAppReady(fixture, 120_000) + + await openSeededSession(fixture.page) + await submitPrompt(fixture.page, BACKGROUND_PROMPT) + await fixture.mock.waitForHeldStream() + await openNewSession(fixture.page) + + if (resumeKind === 'cold') { + await reloadIntoColdRenderer(fixture) + } + + await openSeededSession(fixture.page) + fixture.mock.releaseHeldStream() + await fixture.page.waitForFunction( + expected => (document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '').includes(expected), + MOCK_REPLY, + { timeout: 60_000 }, + ) + await fixture.page.waitForTimeout(300) + await fixture.page.screenshot({ path: testInfo.outputPath(`${resumeKind}-background-inference-resume.png`), fullPage: false }) + + expect(await textNodeOccurrences(fixture.page, BACKGROUND_PROMPT), 'the running user prompt should appear once').toBe(1) + expect(await textNodeOccurrences(fixture.page, MOCK_REPLY), 'the completed assistant reply should appear once').toBe(1) + }) + } +}) diff --git a/apps/desktop/e2e/mock-server.ts b/apps/desktop/e2e/mock-server.ts index f7a4a74b338b..8ff1c0d9548d 100644 --- a/apps/desktop/e2e/mock-server.ts +++ b/apps/desktop/e2e/mock-server.ts @@ -17,21 +17,42 @@ import http from 'node:http' /** A canned assistant reply used for every chat completion request. */ -const CANNED_REPLY = 'Hello from the mock inference server! The full boot chain is working.' +export const MOCK_REPLY = 'Hello from the mock inference server! The full boot chain is working.' + +export interface MockServerOptions { + /** + * Pause a streaming response just after its first token when the latest user + * prompt contains this text. E2E tests use this to switch away from a real, + * still-running inference turn before resuming that session. + */ + holdFirstStreamForPrompt?: string +} + +export interface MockServer { + port: number + url: string + receivedPrompts: string[] + waitForHeldStream: () => Promise + releaseHeldStream: () => void + close: () => Promise +} /** * Start the mock server on an ephemeral port. * * @returns a handle with `port`, `url`, received user prompts, and `close()`. */ -export function startMockServer(): Promise<{ - port: number - url: string - receivedPrompts: string[] - close: () => Promise -}> { +export function startMockServer(options: MockServerOptions = {}): Promise { return new Promise((resolve, reject) => { const receivedPrompts: string[] = [] + let resolveHeldStreamStarted: (() => void) | null = null + let releaseHeldStream: (() => void) | null = null + const heldStreamStarted = new Promise(resolveHeld => { + resolveHeldStreamStarted = resolveHeld + }) + const heldStreamReleased = new Promise(resolveRelease => { + releaseHeldStream = resolveRelease + }) const server = http.createServer((req, res) => { // CORS headers — the Electron renderer doesn't need them, but they // don't hurt and make the server usable from a browser context too. @@ -100,7 +121,11 @@ export function startMockServer(): Promise<{ }) // Send the content in a few chunks to simulate streaming. - const words = CANNED_REPLY.split(' ') + const words = MOCK_REPLY.split(' ') + const holdThisStream = Boolean( + options.holdFirstStreamForPrompt && typeof lastUserMessage?.content === 'string' && + lastUserMessage.content.includes(options.holdFirstStreamForPrompt), + ) let i = 0 const sendChunk = () => { @@ -143,6 +168,11 @@ export function startMockServer(): Promise<{ })}\n\n`, ) i++ + if (holdThisStream && i === 1) { + resolveHeldStreamStarted?.() + heldStreamReleased.then(() => setTimeout(sendChunk, 20)) + return + } // Small delay between chunks to simulate real streaming. setTimeout(sendChunk, 20) } @@ -160,7 +190,7 @@ export function startMockServer(): Promise<{ choices: [ { index: 0, - message: { role: 'assistant', content: CANNED_REPLY }, + message: { role: 'assistant', content: MOCK_REPLY }, finish_reason: 'stop', }, ], @@ -202,6 +232,8 @@ export function startMockServer(): Promise<{ port, url, receivedPrompts, + waitForHeldStream: () => heldStreamStarted, + releaseHeldStream: () => releaseHeldStream?.(), close: () => new Promise((resolveClose, rejectClose) => { server.close((err) => { diff --git a/apps/desktop/e2e/scripts/seed_large_session.py b/apps/desktop/e2e/scripts/seed_large_session.py new file mode 100644 index 000000000000..dc0ef750a691 --- /dev/null +++ b/apps/desktop/e2e/scripts/seed_large_session.py @@ -0,0 +1,52 @@ +#!/usr/bin/env python3 +"""Seed a deterministic, tool-free large session into an isolated state.db.""" + +import sys +from pathlib import Path + +repo_root = Path(__file__).resolve().parents[3] +sys.path.insert(0, str(repo_root)) + +from hermes_state import SessionDB # noqa: E402 + +SESSION_ID = "e2e-large-session" +SESSION_TITLE = "E2E large persisted session" + + +def main() -> None: + if len(sys.argv) != 2: + raise SystemExit(f"usage: {sys.argv[0]} ") + + messages = [] + for index in range(53): + role = "user" if index % 2 == 0 else "assistant" + content = ( + f"E2E persisted user message {index}: audit the compatibility matrix" + if role == "user" + else f"E2E persisted assistant reply {index}: recorded the audit result" + ) + messages.append({"role": role, "content": content, "timestamp": 1_700_000_000 + index}) + + database = SessionDB(db_path=Path(sys.argv[1])) + result = database.import_sessions( + [ + { + "id": SESSION_ID, + "source": "desktop", + "model": "mock-model", + "started_at": 1_700_000_000, + "title": SESSION_TITLE, + "cwd": str(repo_root), + "system_prompt": "", + "messages": messages, + } + ] + ) + database.close() + + if not result.get("ok") or result.get("imported") != 1: + raise SystemExit(f"failed to seed large session: {result}") + + +if __name__ == "__main__": + main() diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts index 3e6736244040..34cfe1f95559 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts @@ -60,7 +60,7 @@ import { } from '@/store/session-states' import { broadcastSessionsChanged } from '@/store/session-sync' import { isWatchWindow } from '@/store/windows' -import type { SessionCreateResponse, SessionResumeResponse, UsageStats } from '@/types/hermes' +import type { SessionCreateResponse, SessionMessage, SessionResumeResponse, UsageStats } from '@/types/hermes' import { NEW_CHAT_ROUTE, sessionRoute, SETTINGS_ROUTE } from '../../../routes' import type { ClientSessionState, SidebarNavItem } from '../../../types' @@ -778,8 +778,9 @@ export function useSessionActions({ setMessages([]) } + // A history load is not a live turn. Toggling busy here and again in the + // finally block re-renders the thread viewport after it has loaded. busyRef.current = true - setBusy(true) setAwaitingResponse(false) clearNotifications() setSelectedStoredSessionId(storedSessionId) @@ -805,7 +806,6 @@ export function useSessionActions({ : $messages.get() let prefetchApplied = false - let prefetchedMessageCount = 0 let prefetchedStoredSessionId: string | null = null // REST transcript prefetch and the gateway resume RPC are independent @@ -832,24 +832,14 @@ export function useSessionActions({ // keeps it from surfacing as unhandled while the prefetch settles. resumePromise.catch(() => undefined) + // Keep both requests concurrent, but do not paint the REST result until + // the runtime resume has also settled. An eager prefetch paint followed + // by the runtime projection rebuilds large transcripts during resume. + let prefetchedResult: { messages: SessionMessage[]; session_id?: string } | null = null + try { if (prefetchPromise) { - const storedMessages = await prefetchPromise - - if (isCurrentResume()) { - const previousMessages = resumedSameSelectedSession - ? preserveLocalPendingTurnMessages($messages.get(), resumeStartMessages) - : $messages.get() - - localSnapshot = reconcileAuthoritativeMessages(storedMessages.messages, previousMessages) - prefetchApplied = true - prefetchedMessageCount = storedMessages.messages.length - prefetchedStoredSessionId = storedMessages.session_id || storedSessionId - - if (!chatMessageArraysEquivalent($messages.get(), localSnapshot)) { - setMessages(localSnapshot) - } - } + prefetchedResult = await prefetchPromise } } catch { // Non-fatal: gateway resume below can still hydrate the session. @@ -861,6 +851,16 @@ export function useSessionActions({ return } + if (prefetchedResult) { + const previousMessages = resumedSameSelectedSession + ? preserveLocalPendingTurnMessages($messages.get(), resumeStartMessages) + : $messages.get() + + localSnapshot = reconcileAuthoritativeMessages(prefetchedResult.messages, previousMessages) + prefetchApplied = true + prefetchedStoredSessionId = prefetchedResult.session_id || storedSessionId + } + const currentMessages = $messages.get() // Keep the local snapshot when resume would only reshuffle runtime @@ -878,8 +878,7 @@ export function useSessionActions({ const preferredMessages = prefetchApplied && prefetchMatchesResumedSession && - !hasLiveProjection && - resumed.messages.length <= prefetchedMessageCount + !hasLiveProjection ? localSnapshot : (() => { const previousMessages = resumedSameSelectedSession @@ -927,6 +926,14 @@ export function useSessionActions({ }), storedSessionId ) + + // updateSessionState stages its view sync through requestAnimationFrame. + // Commit the final, already-reconciled transcript now so resume has one + // additive DOM build instead of an eager prefetch build plus a later + // runtime projection build. + if (!chatMessageArraysEquivalent($messages.get(), messagesForView)) { + setMessages(messagesForView) + } } catch (err) { if (!isCurrentResume()) { return diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/utils.test.ts b/apps/desktop/src/app/session/hooks/use-session-actions/utils.test.ts index 84e0d232fe2b..652cd0af3717 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/utils.test.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/utils.test.ts @@ -354,6 +354,32 @@ describe('appendLiveSessionProjection', () => { expect(restored[3]).toMatchObject({ id: 'assistant-stream-runtime-1', pending: true }) }) + it('does not duplicate a persisted inflight user after consecutive canceled user turns', () => { + const stored = [ + msg('stored-user-1', 'user', 'canceled prompt one'), + msg('stored-user-2', 'user', 'canceled prompt two'), + msg('stored-user-3', 'user', 'current running prompt') + ] + + const restored = appendLiveSessionProjection(stored, { + session_id: 'runtime-1', + inflight: { + user: 'current running prompt', + assistant: 'partial answer', + streaming: true + } + }) + + expect(restored.map(message => message.role)).toEqual(['user', 'user', 'user', 'assistant']) + expect(restored.map(message => message.parts.map(part => ('text' in part ? part.text : '')).join(''))).toEqual([ + 'canceled prompt one', + 'canceled prompt two', + 'current running prompt', + 'partial answer' + ]) + expect(restored[3]).toMatchObject({ id: 'assistant-stream-runtime-1', pending: true }) + }) + it('preserves the original array when no live projection exists', () => { const stored = [msg('stored-user', 'user', 'earlier')] diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts b/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts index feb22b9e658a..2604a68841df 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts @@ -314,8 +314,15 @@ export function appendLiveSessionProjection( const sessionId = projection.session_id || 'session' const projected: ChatMessage[] = [] - - if (inflightUser) { + // A turn normally persists its user row before inference begins. session.resume + // then returns that stored row *and* the still-live inflight projection; adding + // both makes a backgrounded prompt appear twice when its session is reopened. + // Only suppress the projection when the latest authoritative user row is the + // same turn — older identical prompts must not hide a newly accepted repeat. + const latestUser = [...messages].reverse().find(message => message.role === 'user') + const inflightUserAlreadyPersisted = latestUser && chatMessageText(latestUser).trim() === inflightUser + + if (inflightUser && !inflightUserAlreadyPersisted) { projected.push({ id: `user-inflight-${sessionId}`, role: 'user',