diff --git a/apps/desktop/e2e/mock-server.ts b/apps/desktop/e2e/mock-server.ts index a198ff8989c..ce4665d1775 100644 --- a/apps/desktop/e2e/mock-server.ts +++ b/apps/desktop/e2e/mock-server.ts @@ -14,8 +14,11 @@ * prove the full boot → gateway → inference → renderer chain works. */ +import fs from 'node:fs' import http from 'node:http' import type { ServerResponse } from 'node:http' +import os from 'node:os' +import nodePath from 'node:path' /** A canned assistant reply used for every chat completion request. */ export const MOCK_REPLY = 'Hello from the mock inference server! The full boot chain is working.' @@ -27,6 +30,14 @@ export interface MockServerOptions { holdFirstCompletionContaining?: string /** Absolute sandbox path written by the verify-on-stop scripted tool call. */ verificationWritePath?: string +/** + * Sentinel path that ends the E2E_SIDEBAR_CROSS background process. + * + * Without it that process is a bare `sleep 5`, which races the agent turn and + * the 4s auto-dismiss linger — see `createBackgroundReleaseHandle`. Pass a + * handle's `path` to let the test decide when the process exits. + */ +backgroundReleasePath?: string } export interface MockServer { @@ -167,37 +178,68 @@ const SIDEBAR_SCRIPT: ScriptedTurn[] = [ // ─── Sidebar cross-session script ────────────────────────────────────── // -// E2E_SIDEBAR_CROSS trigger uses a longer background process (sleep 5) so -// the "background running" dot is visible long enough for the test to: +// E2E_SIDEBAR_CROSS starts a long background process plus a subagent so the +// tests can: // 1. See the background dot while the subagent runs. // 2. Open a different session and see session A's dot transition to // "finished unread" when the background process completes. +// +// The background process must outlive the agent turn — the whole point is a +// dot that is still "running" after the final answer lands. A fixed `sleep` +// cannot guarantee that: on a loaded CI runner the turn (two model round +// trips + a real subagent delegation) can take longer than the sleep, the +// process exits early, the 4s success linger elapses, and the dot is gone +// before the test looks. That is a wall-clock race between three independent +// timers, and it made this the flakiest spec in the suite. +// +// When `backgroundReleasePath` is set the process instead blocks until the +// test creates that sentinel file, so the test — not the clock — decides when +// the dot clears. `sleep 5` remains the fallback for callers that don't pass +// a handle. +function sidebarCrossBgCommand(releasePath?: string): string { + if (!releasePath) { + return 'echo "long bg output" && sleep 5 && echo "finished"' + } + // Bounded wait (60s): if a test forgets to release (or crashes mid-way), + // the process still exits instead of hanging the worker until the suite + // times out. + const quoted = JSON.stringify(releasePath) + return [ + 'echo "long bg output"', + `for _ in $(seq 1 600); do [ -e ${quoted} ] && break; sleep 0.1; done`, + 'echo "finished"', + ].join(' && ') +} -const SIDEBAR_CROSS_SCRIPT: ScriptedTurn[] = [ - { - text: 'Starting a long background task and delegating work.', - toolCalls: [ - { - name: 'terminal', - args: { - command: 'echo "long bg output" && sleep 5 && echo "finished"', - background: true, - notify_on_complete: true, +function sidebarCrossScript(releasePath?: string): ScriptedTurn[] { + return [ + { + text: 'Starting a long background task and delegating work.', + toolCalls: [ + { + name: 'terminal', + args: { + command: sidebarCrossBgCommand(releasePath), + background: true, + notify_on_complete: true, + }, }, - }, - { - name: 'delegate_task', - args: { - goal: 'Analyze cross-session state', - context: 'Testing that the background dot updates across sessions.', + { + name: 'delegate_task', + args: { + goal: 'Analyze cross-session state', + context: 'Testing that the background dot updates across sessions.', + }, }, - }, - ], - }, - { - text: 'Both tasks are running in the background now.', - }, -] + ], + }, + { + text: 'Both tasks are running in the background now.', + }, + ] +} + +const SIDEBAR_CROSS_SCRIPT: ScriptedTurn[] = sidebarCrossScript() const QUEUE_STOP_SCRIPT: ScriptedTurn[] = [ { @@ -423,7 +465,8 @@ export function startMockServer(options: MockServerOptions = {}): Promise void + /** Remove the sentinel if it still exists. Safe to call twice. */ + cleanup: () => void +} + +/** + * Create a sentinel that keeps the E2E_SIDEBAR_CROSS background process alive + * until the test explicitly releases it. + * + * The cross-session sidebar tests need a background process that is still + * RUNNING after the agent turn finishes — that is the state under test (a + * session whose turn is done but whose background work is not). With a fixed + * `sleep`, three independent clocks race: the sleep, the agent turn (two model + * round trips plus a real subagent delegation), and the 4s success linger + * before a finished task auto-dismisses. When a loaded CI runner makes the + * turn slower than the sleep, the process is already gone and the assertion + * samples an empty sidebar. Observed on CI 2026-07-26 across two unrelated + * PRs: the "should appear" poll needed 7.5s to see the dot, by which point + * `sleep 5` had exited. + * + * With a sentinel there is one clock and the test owns it: + * + * ```ts + * const release = createBackgroundReleaseHandle() + * const mock = await startMockServer({ backgroundReleasePath: release.path }) + * // ... assert the dot is visible; it cannot vanish on its own ... + * release.release() // now, and only now, the process exits + * ``` + */ +export function createBackgroundReleaseHandle(): BackgroundReleaseHandle { + const path = nodePath.join( + os.tmpdir(), + `hermes-e2e-bg-release-${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`, + ) + return { + path, + release: () => { + try { + fs.writeFileSync(path, 'release') + } catch { + // The process also has a bounded fallback wait; a failed write must + // not crash the test before its real assertions run. + } + }, + cleanup: () => { + try { + fs.rmSync(path, { force: true }) + } catch { + // Best-effort — the sentinel lives in the OS temp dir. + } + }, + } +} + /** * The interim script's text constants, exported for test assertions. * Each entry is the visible text of one turn. Turns with empty text @@ -756,8 +858,12 @@ export const SIDEBAR_CROSS_TEXTS = { interimText: SIDEBAR_CROSS_SCRIPT[0].text, /** The final answer text. */ finalText: SIDEBAR_CROSS_SCRIPT[SIDEBAR_CROSS_SCRIPT.length - 1].text, - /** The longer background process command (sleep 5). */ - bgCommand: 'echo "long bg output" && sleep 5 && echo "finished"', + /** + * The default (unheld) background process command. Tests that pass a + * `backgroundReleasePath` get a sentinel-waiting command instead — see + * `createBackgroundReleaseHandle`. + */ + bgCommand: sidebarCrossBgCommand(), /** The subagent's goal. */ subagentGoal: 'Analyze cross-session state', } as const diff --git a/apps/desktop/e2e/sidebar-states.spec.ts b/apps/desktop/e2e/sidebar-states.spec.ts index 051efda3543..6d8c0c2c9c9 100644 --- a/apps/desktop/e2e/sidebar-states.spec.ts +++ b/apps/desktop/e2e/sidebar-states.spec.ts @@ -16,7 +16,12 @@ import { setupMockBackend, waitForAppReady, } from './fixtures' -import { SIDEBAR_CROSS_TEXTS, SIDEBAR_TEXTS, restartMockServer } from './mock-server' +import { + createBackgroundReleaseHandle, + restartMockServer, + SIDEBAR_CROSS_TEXTS, + SIDEBAR_TEXTS, +} from './mock-server' /** Background-running dot aria-label (from i18n en.ts). */ const BG_DOT_LABEL = 'Background task running' @@ -176,21 +181,30 @@ test.describe('sidebar states — cross-session dot transition', () => { test.describe.configure({ mode: 'serial' }) let fixture: MockBackendFixture + // Keeps the background process alive until this test releases it, so the + // "still running after the turn finished" state can't expire on its own. + const bgRelease = createBackgroundReleaseHandle() test.beforeAll(async () => { restartMockServer() - fixture = await setupMockBackend() + fixture = await setupMockBackend({ + mockServer: { backgroundReleasePath: bgRelease.path }, + }) await waitForAppReady(fixture, 120_000) }) test.afterAll(async () => { + // Release first so the process exits even if the test failed early, + // then drop the sentinel file. + bgRelease.release() await fixture?.cleanup() + bgRelease.cleanup() }) test('background dot transitions to finished when viewing another session', async () => { const page = fixture.page - // Start a turn with a long background process (sleep 5). + // Start a turn whose background process runs until we release it. const composer = page.locator('[contenteditable="true"]').first() await composer.waitFor({ state: 'visible', timeout: 10_000 }) await composer.click() @@ -212,8 +226,9 @@ test.describe('sidebar states — cross-session dot transition', () => { { timeout: 90_000 }, ) - // The background dot should still be visible (sleep 5 hasn't finished yet, - // or auto-dismiss hasn't fired). + // The background dot must still be visible: the turn is done but the + // process is held open by the sentinel, so this is a stable state rather + // than a window we have to catch in time. const bgDuringTurn = await page.locator(`[aria-label="${BG_DOT_LABEL}"]`).count() expect(bgDuringTurn, 'background dot should still be visible after turn completes').toBeGreaterThan(0) @@ -225,8 +240,9 @@ test.describe('sidebar states — cross-session dot transition', () => { await page.locator('button:has-text("New session")').first().click() await page.waitForTimeout(2000) - // Now wait for the background process to finish (sleep 5 + auto-dismiss). - // The session A dot should transition away from "background running". + // Now let the background process finish. The session A dot should + // transition away from "background running". + bgRelease.release() await expect .poll( () => page.locator(`[aria-label="${BG_DOT_LABEL}"]`).count(), diff --git a/apps/desktop/e2e/tile-unread-bug.spec.ts b/apps/desktop/e2e/tile-unread-bug.spec.ts index cd87a2eb681..fe87b63f389 100644 --- a/apps/desktop/e2e/tile-unread-bug.spec.ts +++ b/apps/desktop/e2e/tile-unread-bug.spec.ts @@ -22,7 +22,12 @@ import { setupMockBackend, waitForAppReady, } from './fixtures' -import { SIDEBAR_CROSS_TEXTS, restartMockServer } from './mock-server' +import { + type BackgroundReleaseHandle, + createBackgroundReleaseHandle, + restartMockServer, + SIDEBAR_CROSS_TEXTS, +} from './mock-server' /** Finished-unread dot aria-label. */ const UNREAD_DOT_LABEL = 'Finished — unread' @@ -34,7 +39,7 @@ function sessionRow(page: import('@playwright/test').Page, text: string) { return page.locator('[data-slot="sidebar"] button').filter({ hasText: text }).first() } -/** Common setup: start a turn with a sleep 5 bg process + subagent, wait for +/** Common setup: start a turn with a held bg process + subagent, wait for * the turn to complete, then switch to a new session so the first session is * no longer $selectedStoredSessionId (required before opening a tile). */ async function startTurnAndSwitchAway(page: import('@playwright/test').Page) { @@ -67,7 +72,9 @@ async function startTurnAndSwitchAway(page: import('@playwright/test').Page) { { timeout: 90_000 }, ) - // The background dot should still be visible (sleep 5 hasn't finished). + // The background dot must still be visible: the turn is done but the + // process is held open by the sentinel, so this is a stable state rather + // than a window we have to catch in time. const bgDuringTurn = await page.locator(`[aria-label="${BG_DOT_LABEL}"]`).count() expect(bgDuringTurn, 'background dot should still be visible after turn completes').toBeGreaterThan(0) @@ -77,8 +84,12 @@ async function startTurnAndSwitchAway(page: import('@playwright/test').Page) { await page.waitForTimeout(2000) } -/** Wait for the background process to finish (sleep 5 + auto-dismiss). */ -async function waitForBgProcessToFinish(page: import('@playwright/test').Page) { +/** Release the held background process, then wait for its dot to clear. */ +async function waitForBgProcessToFinish( + page: import('@playwright/test').Page, + release?: BackgroundReleaseHandle, +) { + release?.release() await expect .poll( () => page.locator(`[aria-label="${BG_DOT_LABEL}"]`).count(), @@ -95,15 +106,20 @@ test.describe('sidebar states — tab (hidden) unread is correct', () => { test.describe.configure({ mode: 'serial' }) let fixture: MockBackendFixture + const bgRelease = createBackgroundReleaseHandle() test.beforeAll(async () => { restartMockServer() - fixture = await setupMockBackend() + fixture = await setupMockBackend({ + mockServer: { backgroundReleasePath: bgRelease.path }, + }) await waitForAppReady(fixture, 120_000) }) test.afterAll(async () => { + bgRelease.release() await fixture?.cleanup() + bgRelease.cleanup() }) test('session opened as a tab (not visible) correctly gets unread dot', async () => { @@ -123,7 +139,7 @@ test.describe('sidebar states — tab (hidden) unread is correct', () => { // Evidence: the tab is open but the session is not visible on screen. await page.screenshot({ path: 'test-results/tile-bug-tab-opened.png' }) - await waitForBgProcessToFinish(page) + await waitForBgProcessToFinish(page, bgRelease) // A tab that's not the active tab IS hidden — the unread dot is correct. // The user is NOT looking at it, so marking it "unread" is right. @@ -142,15 +158,20 @@ test.describe.skip('sidebar states — split (visible) unread bug (RED)', () => test.describe.configure({ mode: 'serial' }) let fixture: MockBackendFixture + const bgRelease = createBackgroundReleaseHandle() test.beforeAll(async () => { restartMockServer() - fixture = await setupMockBackend() + fixture = await setupMockBackend({ + mockServer: { backgroundReleasePath: bgRelease.path }, + }) await waitForAppReady(fixture, 120_000) }) test.afterAll(async () => { + bgRelease.release() await fixture?.cleanup() + bgRelease.cleanup() }) test('session visible in a split tile does NOT get unread dot when it finishes', async () => { @@ -196,7 +217,7 @@ test.describe.skip('sidebar states — split (visible) unread bug (RED)', () => // Evidence: the split tile is now open side-by-side — both sessions visible. await page.screenshot({ path: 'test-results/tile-bug-split-opened.png' }) - await waitForBgProcessToFinish(page) + await waitForBgProcessToFinish(page, bgRelease) // THE BUG: the session visible in the split tile should NOT have the green // "finished unread" dot — the user is looking right at it. This assertion