mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 16:02:20 +00:00
Observe daemon health failures and fix e2e test races (#10595)
- Add E2E_FORCE_DAEMON_HEALTH_UNREACHABLE env to simulate failed health checks - Log when replacing a failed daemon, but stay silent on cold starts - Simplify daemon-slow-health-check-preservation: use forced-unreachable health instead of SIGSTOP/SIGCONT - Add --no-sandbox flag to electron launch args for Ubuntu CI - Support extraEnv option in restart session launches
This commit is contained in:
@@ -7,6 +7,7 @@ import { DaemonServer } from './daemon-server'
|
||||
import { getDaemonPidPath, serializeDaemonPidFile } from './daemon-spawner'
|
||||
import {
|
||||
checkDaemonHealth,
|
||||
E2E_FORCE_DAEMON_HEALTH_UNREACHABLE_ENV,
|
||||
getProcessStartedAtMs,
|
||||
healthCheckDaemon,
|
||||
killStaleDaemon,
|
||||
@@ -127,6 +128,25 @@ describe('daemon health', () => {
|
||||
await expect(healthCheckDaemon(socketPath, tokenPath)).resolves.toBe(false)
|
||||
})
|
||||
|
||||
it('returns unreachable when the e2e force-health-failure env is set', async () => {
|
||||
// Why: prove the e2e seam short-circuits even when a real daemon would
|
||||
// otherwise pass — not the already-covered missing-socket path.
|
||||
const server = new DaemonServer({
|
||||
socketPath,
|
||||
tokenPath,
|
||||
spawnSubprocess: () => createMockSubprocess()
|
||||
})
|
||||
await server.start()
|
||||
vi.stubEnv(E2E_FORCE_DAEMON_HEALTH_UNREACHABLE_ENV, '1')
|
||||
try {
|
||||
await expect(checkDaemonHealth(socketPath, tokenPath)).resolves.toBe('unreachable')
|
||||
await expect(healthCheckDaemon(socketPath, tokenPath)).resolves.toBe(false)
|
||||
} finally {
|
||||
vi.unstubAllEnvs()
|
||||
await server.shutdown()
|
||||
}
|
||||
})
|
||||
|
||||
it('classifies a hello-rejected daemon as rejected, not unreachable', async () => {
|
||||
// Why: 'rejected' means the daemon answered and refused adoption — the
|
||||
// launcher may replace it. 'unreachable' also covers a wedged-but-live
|
||||
|
||||
@@ -24,6 +24,10 @@ const RESOLVER_HEALTH_CHECK_TIMEOUT_MS = 3_000
|
||||
const KILL_WAIT_MS = 3_000
|
||||
const KILL_POLL_MS = 100
|
||||
const START_TIME_TOLERANCE_MS = 1_500
|
||||
// Why: e2e forces the failed-health preserve path without SIGSTOP races —
|
||||
// a stopped daemon also blocks listSessions, so the unhealthy guard cannot
|
||||
// verify live sessions until SIGCONT, which is flaky under CI load.
|
||||
export const E2E_FORCE_DAEMON_HEALTH_UNREACHABLE_ENV = 'ORCA_E2E_FORCE_DAEMON_HEALTH_UNREACHABLE'
|
||||
// Why: on Windows the pid file's startedAtMs is the daemon's self-reported
|
||||
// Node start time, while verification reads the OS process creation time —
|
||||
// the gap between them is the exe bootstrap, which AV/disk pressure can
|
||||
@@ -82,6 +86,11 @@ function canConnectSocket(socketPath: string): Promise<boolean> {
|
||||
|
||||
export function checkDaemonHealth(socketPath: string, tokenPath: string): Promise<DaemonHealth> {
|
||||
return new Promise((resolve) => {
|
||||
if (process.env[E2E_FORCE_DAEMON_HEALTH_UNREACHABLE_ENV] === '1') {
|
||||
resolve('unreachable')
|
||||
return
|
||||
}
|
||||
|
||||
if (process.platform !== 'win32' && !existsSync(socketPath)) {
|
||||
resolve('unreachable')
|
||||
return
|
||||
|
||||
@@ -2160,6 +2160,64 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
||||
expect(forkMock).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('stays silent about replacing a daemon on a cold start, where there is none', async () => {
|
||||
// Why: a first launch reaches the same replace fall-through (unreachable health,
|
||||
// no socket, nothing to probe); announcing a replacement there reports killing a
|
||||
// daemon that never existed, on the most common path there is.
|
||||
const mod = await importFresh()
|
||||
await mod.initDaemonPtyProvider()
|
||||
|
||||
// Both pre-spawn probes fail: nothing ever answers, so no session count is observed.
|
||||
const unreachableClient = function MockDaemonClient() {
|
||||
return {
|
||||
ensureConnected: vi.fn(async () => {
|
||||
throw new Error('connect ENOENT')
|
||||
}),
|
||||
request: vi.fn(),
|
||||
disconnect: vi.fn()
|
||||
}
|
||||
}
|
||||
daemonClientMock
|
||||
.mockImplementationOnce(unreachableClient)
|
||||
.mockImplementationOnce(unreachableClient)
|
||||
|
||||
const launcher = spawnerInstances[0].launcher as (
|
||||
socketPath: string,
|
||||
tokenPath: string
|
||||
) => Promise<{ shutdown(): Promise<void> }>
|
||||
checkDaemonHealthMock.mockResolvedValueOnce('unreachable')
|
||||
probeSocketExistsMock.mockReturnValue(false)
|
||||
forkMock.mockImplementationOnce(() => ({
|
||||
pid: 12345,
|
||||
on(event: string, cb: (arg?: unknown) => void) {
|
||||
if (event === 'message') {
|
||||
queueMicrotask(() => cb({ type: 'ready', startedAtMs: 1_000_000 }))
|
||||
}
|
||||
return this
|
||||
},
|
||||
once() {
|
||||
return this
|
||||
},
|
||||
off() {
|
||||
return this
|
||||
},
|
||||
disconnect: vi.fn(),
|
||||
unref: vi.fn()
|
||||
}))
|
||||
|
||||
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
try {
|
||||
await launcher('/fake/socket', '/fake/token')
|
||||
|
||||
expect(forkMock).toHaveBeenCalled()
|
||||
expect(warnSpy).not.toHaveBeenCalledWith(
|
||||
expect.stringContaining('Replacing daemon that failed the health check')
|
||||
)
|
||||
} finally {
|
||||
warnSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
// Why: net.connect stub whose 'connect' fires, so probeSocket() reports the pipe alive on every grace re-check.
|
||||
function stubAliveSocketConnect() {
|
||||
const handlers: Record<string, (() => void)[]> = { connect: [], error: [] }
|
||||
@@ -2267,6 +2325,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
||||
|
||||
// Count only the launcher's own session-count probes.
|
||||
daemonClientMock.mockClear()
|
||||
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
|
||||
try {
|
||||
await launcher('/fake/socket', '/fake/token')
|
||||
@@ -2279,7 +2338,16 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
||||
expect(forkMock).toHaveBeenCalled()
|
||||
// The launcher probes the full grace budget: 1 initial probe + WEDGED_DAEMON_GRACE_RETRIES retries.
|
||||
expect(daemonClientMock).toHaveBeenCalledTimes(3 + WEDGED_DAEMON_GRACE_RETRIES)
|
||||
// Why: this replace path used to kill the daemon with no log, so a post-hoc
|
||||
// reader could not tell it apart from an adoption; the verdict must be recorded.
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Replacing daemon that failed the health check')
|
||||
)
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining(`graceRetries=${WEDGED_DAEMON_GRACE_RETRIES}`)
|
||||
)
|
||||
} finally {
|
||||
warnSpy.mockRestore()
|
||||
// Restore the answering default: clearAllMocks clears calls not impls, so the throwing impl would leak into later tests.
|
||||
daemonClientMock.mockImplementation(answeringDefault)
|
||||
}
|
||||
|
||||
@@ -428,6 +428,17 @@ function createOutOfProcessLauncher(
|
||||
)
|
||||
return preserveDaemon()
|
||||
}
|
||||
// Why: the sibling replace branches announce themselves, but this one used
|
||||
// to kill a daemon silently — leaving no way to tell a replacement apart
|
||||
// from an adoption after the fact. A cold start also lands here with
|
||||
// nothing to replace, so only speak up once something actually answered:
|
||||
// a probe that returned a count, a socket that survived a grace retry, or
|
||||
// a refused hello.
|
||||
if (liveSessionCount !== null || graceRetry > 0 || health === 'rejected') {
|
||||
console.warn(
|
||||
`[daemon] Replacing daemon that failed the health check (health=${health}, liveSessions=${liveSessionCount ?? 'unverifiable'}, graceRetries=${graceRetry})`
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// Why: a raw socket can outlive a broken daemon; kill by PID before respawn so the new daemon doesn't race the stale one.
|
||||
|
||||
@@ -13,13 +13,13 @@ import {
|
||||
} from './helpers/terminal'
|
||||
import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store'
|
||||
import { attachRepoAndOpenTerminal, createRestartSession } from './helpers/orca-restart'
|
||||
import { E2E_FORCE_DAEMON_HEALTH_UNREACHABLE_ENV } from '../../src/main/daemon/daemon-health'
|
||||
import { PROTOCOL_VERSION } from '../../src/main/daemon/types'
|
||||
import { PTY_SESSION_ID_SEPARATOR } from '../../src/shared/pty-session-id-format'
|
||||
|
||||
// Why: must land after the relaunched app's 3s daemon health check has timed
|
||||
// out (so the unhealthy guard runs) but before the guard's 5s client hello
|
||||
// budget expires. Daemon init starts within the first ~2s of main startup.
|
||||
const RESUME_DAEMON_AFTER_MS = 6_500
|
||||
// Why: holds the daemon guard's decision until well past the ~600ms it takes
|
||||
// electron.launch to resolve, which is the earliest the stderr listener can attach.
|
||||
const GUARD_DECISION_DELAY_MS = 3_000
|
||||
|
||||
function readDaemonPid(userDataDir: string): number {
|
||||
const raw = readFileSync(
|
||||
@@ -42,12 +42,10 @@ test('preserves a live daemon PTY when the daemon is too slow for the startup he
|
||||
test.skip(true, 'Global setup did not produce a seeded test repo')
|
||||
return
|
||||
}
|
||||
test.skip(process.platform === 'win32', 'SIGSTOP/SIGCONT are POSIX-only')
|
||||
|
||||
const session = createRestartSession(testInfo)
|
||||
let firstApp: ElectronApplication | null = null
|
||||
let secondApp: ElectronApplication | null = null
|
||||
let daemonPid: number | null = null
|
||||
|
||||
try {
|
||||
const firstLaunch = await session.launch()
|
||||
@@ -66,70 +64,58 @@ test('preserves a live daemon PTY when the daemon is too slow for the startup he
|
||||
await execInTerminal(firstLaunch.page, ptyId, `echo ${marker}`)
|
||||
await waitForTerminalOutput(firstLaunch.page, marker)
|
||||
|
||||
daemonPid = readDaemonPid(session.userDataDir)
|
||||
const daemonPid = readDaemonPid(session.userDataDir)
|
||||
|
||||
await session.close(firstApp)
|
||||
firstApp = null
|
||||
|
||||
// Why: a stopped daemon still accepts socket connections at the kernel
|
||||
// level but answers nothing — the same observable behavior as a daemon
|
||||
// that is too busy to respond within the health-check budget.
|
||||
process.kill(daemonPid, 'SIGSTOP')
|
||||
|
||||
const stderrLines: string[] = []
|
||||
const resumeTimer = setTimeout(() => {
|
||||
if (daemonPid !== null) {
|
||||
process.kill(daemonPid, 'SIGCONT')
|
||||
}
|
||||
}, RESUME_DAEMON_AFTER_MS)
|
||||
try {
|
||||
// Why: capture stderr from process start — the daemon guard logs its
|
||||
// preservation decision during main-process startup, which can complete
|
||||
// before firstWindow resolves, so a post-launch listener would miss it.
|
||||
const secondLaunch = await session.launch({
|
||||
onStderr: (chunk) => stderrLines.push(chunk)
|
||||
})
|
||||
secondApp = secondLaunch.app
|
||||
// Why: force the failed-health branch without SIGSTOP. Stopping the daemon
|
||||
// also blocks listSessions, so the preserve guard races a fixed SIGCONT
|
||||
// timer under CI load and often takes the healthy path (or misses logs).
|
||||
// With health forced unreachable, listSessions still succeeds and the only
|
||||
// way to keep the same daemon PID is the failed-health preserve path.
|
||||
//
|
||||
// The init delay is what makes the guard's log observable: Playwright owns
|
||||
// the child's stderr from spawn and this listener can only attach once
|
||||
// electron.launch resolves (~600ms in CI). Forced-unreachable health returns
|
||||
// with no timeout, so an undelayed guard decides at ~500ms and its line is
|
||||
// lost before the test is listening.
|
||||
const secondLaunch = await session.launch({
|
||||
extraEnv: {
|
||||
[E2E_FORCE_DAEMON_HEALTH_UNREACHABLE_ENV]: '1',
|
||||
ORCA_E2E_DAEMON_INIT_DELAY_MS: String(GUARD_DECISION_DELAY_MS)
|
||||
},
|
||||
onStderr: (chunk) => stderrLines.push(chunk)
|
||||
})
|
||||
secondApp = secondLaunch.app
|
||||
|
||||
await waitForSessionReady(secondLaunch.page)
|
||||
await expect
|
||||
.poll(
|
||||
async () => secondLaunch.page.evaluate(() => window.__store?.getState().activeWorktreeId),
|
||||
{ timeout: 15_000 }
|
||||
)
|
||||
.toBe(worktreeId)
|
||||
await ensureTerminalVisible(secondLaunch.page)
|
||||
await waitForActiveTerminalManager(secondLaunch.page, 30_000)
|
||||
await waitForPaneCount(secondLaunch.page, 1, 30_000)
|
||||
await waitForTerminalOutput(secondLaunch.page, marker, 20_000)
|
||||
await waitForSessionReady(secondLaunch.page)
|
||||
await expect
|
||||
.poll(
|
||||
async () => secondLaunch.page.evaluate(() => window.__store?.getState().activeWorktreeId),
|
||||
{ timeout: 15_000 }
|
||||
)
|
||||
.toBe(worktreeId)
|
||||
await ensureTerminalVisible(secondLaunch.page)
|
||||
await waitForActiveTerminalManager(secondLaunch.page, 30_000)
|
||||
await waitForPaneCount(secondLaunch.page, 1, 30_000)
|
||||
await waitForTerminalOutput(secondLaunch.page, marker, 20_000)
|
||||
|
||||
// The guard path must actually have run and chosen preserve over replace:
|
||||
// the daemon failed the health check yet was kept because its live session
|
||||
// was verified. Match the stable "preserve…daemon…health check" concepts
|
||||
// (not the exact wording) so a benign log reword doesn't flake, and
|
||||
// confirm the replace path stayed off.
|
||||
await expect
|
||||
.poll(() => stderrLines.join(''), { timeout: 10_000 })
|
||||
.toMatch(/preserv\w*\s+daemon[^\n]*health check/i)
|
||||
expect(stderrLines.join('')).not.toMatch(/\breplacing daemon\b/i)
|
||||
expect(readDaemonPid(session.userDataDir)).toBe(daemonPid)
|
||||
// Why: a killed daemon cold-restores scrollback from history, so the
|
||||
// marker text alone cannot distinguish a live session from a dead one.
|
||||
// The restore banner only appears for cold-restored (dead) sessions.
|
||||
expect(await getTerminalContent(secondLaunch.page)).not.toContain('--- session restored ---')
|
||||
} finally {
|
||||
clearTimeout(resumeTimer)
|
||||
}
|
||||
// Why: the same PID alone also holds on the healthy-adoption path, so assert
|
||||
// the guard's own decision line — otherwise a seam that silently stopped
|
||||
// working would leave this test passing for the wrong reason. Match the
|
||||
// stable "preserve…daemon…health check" concepts so a reword doesn't flake.
|
||||
await expect
|
||||
.poll(() => stderrLines.join(''), { timeout: 10_000 })
|
||||
.toMatch(/preserv\w*\s+daemon[^\n]*health check/i)
|
||||
expect(readDaemonPid(session.userDataDir)).toBe(daemonPid)
|
||||
expect(stderrLines.join('')).not.toMatch(/\breplacing daemon\b/i)
|
||||
// Why: a killed daemon cold-restores scrollback from history, so the
|
||||
// marker text alone cannot distinguish a live session from a dead one.
|
||||
// The restore banner only appears for cold-restored (dead) sessions.
|
||||
expect(await getTerminalContent(secondLaunch.page)).not.toContain('--- session restored ---')
|
||||
} finally {
|
||||
if (daemonPid !== null) {
|
||||
try {
|
||||
// Idempotent: ensures the daemon is resumable for harness cleanup even
|
||||
// if the test failed before the resume timer fired.
|
||||
process.kill(daemonPid, 'SIGCONT')
|
||||
} catch {
|
||||
// Daemon already gone
|
||||
}
|
||||
}
|
||||
if (secondApp) {
|
||||
await session.close(secondApp)
|
||||
}
|
||||
|
||||
@@ -3,9 +3,14 @@ export function getOrcaElectronLaunchArgs(mainPath: string, headful: boolean): s
|
||||
return [mainPath]
|
||||
}
|
||||
|
||||
// Why: Ubuntu CI can fail headless Electron when Chromium's GPU subprocess
|
||||
// cannot initialize; keep E2E on a low-process software path under Xvfb.
|
||||
// Why: Ubuntu CI cannot run Electron's setuid chrome-sandbox (not root-owned
|
||||
// mode 4755 in node_modules). Playwright's electron.launch injects
|
||||
// --no-sandbox automatically; raw spawn() paths (e.g. second-instance
|
||||
// activation) must match or Chromium aborts with SIGTRAP before handshake.
|
||||
// GPU flags keep headless under Xvfb on a software path when the GPU
|
||||
// subprocess cannot initialize.
|
||||
return [
|
||||
'--no-sandbox',
|
||||
'--disable-gpu',
|
||||
'--disable-gpu-compositing',
|
||||
'--disable-gpu-sandbox',
|
||||
|
||||
@@ -42,6 +42,8 @@ type LaunchOptions = {
|
||||
* the test starts capturing.
|
||||
*/
|
||||
onStderr?: (chunk: string) => void
|
||||
/** Merged into this launch only (not baked into the session's shared env). */
|
||||
extraEnv?: Record<string, string>
|
||||
}
|
||||
|
||||
type RestartSession = {
|
||||
@@ -153,6 +155,7 @@ export function createRestartSession(
|
||||
args: getOrcaElectronLaunchArgs(mainPath, headful),
|
||||
env: {
|
||||
...homeIsolation.env,
|
||||
...options?.extraEnv,
|
||||
ORCA_E2E_RUNTIME_WS_PORT: String(runtimeWsPort)
|
||||
}
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user