mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(desktop-e2e): end the sidebar background-dot wall-clock race
The cross-session sidebar specs asserted a state that could expire before they looked at it, making them the flakiest tests in the suite — two reds on unrelated PRs within three minutes on 2026-07-26. Root cause, from the failing run's trace: the tests need a background process that is still RUNNING after the agent turn finishes, but the process was a fixed `sleep 5` racing two other clocks — the turn itself (two model round trips plus a real subagent delegation) and the 4s success linger before a finished task auto-dismisses. On a loaded runner the "dot should appear" poll took 7.5s to see the dot; by then `sleep 5` had already exited, `waitForFunction(finalText)` returned in 0.08s because the turn was long done, and the next line — a bare synchronous `.count()`, not a wait — sampled 0. The process lifetime is now test-controlled: `createBackgroundReleaseHandle()` mints a sentinel path, the scripted command blocks until that file appears, and the test releases it exactly when it wants the dot to clear. One clock instead of three, and the "turn done, process still running" state is stable rather than a window to catch. The wait is bounded (60s) so a forgotten release can't hang a worker, and `sleep 5` stays as the default for callers that pass no handle. No product code touched — E2E harness only.
This commit is contained in:
Hermes Agent
Teknium
parent
6deb92df52
commit
3a3bc41c7e
3 changed files with 187 additions and 44 deletions
|
|
@ -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<MockSe
|
|||
}
|
||||
|
||||
if (isSidebarCrossTrigger) {
|
||||
const turn = SIDEBAR_CROSS_SCRIPT[_sidebarCrossIndex] ?? SIDEBAR_CROSS_SCRIPT[SIDEBAR_CROSS_SCRIPT.length - 1]
|
||||
const script = sidebarCrossScript(options.backgroundReleasePath)
|
||||
const turn = script[_sidebarCrossIndex] ?? script[script.length - 1]
|
||||
_sidebarCrossIndex++
|
||||
|
||||
if (stream) {
|
||||
|
|
@ -722,6 +765,65 @@ export function restartMockServer(): void {
|
|||
resetScriptIndex()
|
||||
}
|
||||
|
||||
/** Test-controlled lifetime for the E2E_SIDEBAR_CROSS background process. */
|
||||
export interface BackgroundReleaseHandle {
|
||||
/** Sentinel path — pass as `backgroundReleasePath` to `startMockServer`. */
|
||||
path: string
|
||||
/** End the background process now (creates the sentinel). */
|
||||
release: () => 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
|
||||
|
|
|
|||
|
|
@ -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(),
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue