fix false missing Hermes CLI detection after deployment
This commit is contained in:
@@ -0,0 +1,34 @@
|
||||
// @vitest-environment jsdom
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
const request = vi.hoisted(() => vi.fn())
|
||||
vi.mock('@/api/client', () => ({ request }))
|
||||
|
||||
beforeEach(() => { vi.resetModules(); request.mockReset() })
|
||||
|
||||
describe('CLI installation guard', () => {
|
||||
it('does not send a slow, installed CLI to the installation page', async () => {
|
||||
request.mockResolvedValue({ available: false, reason: 'timeout' })
|
||||
const api = await import('@/api/hermes/system')
|
||||
expect(await api.checkHermesCliAvailable()).toBe(true)
|
||||
expect(await api.getHermesCliStatus()).toMatchObject({ reason: 'timeout' })
|
||||
expect(request).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it.each(['missing', 'permission_denied', 'failed'])('retains a genuine %s error and bypasses server cache on retry', async reason => {
|
||||
request.mockResolvedValueOnce({ available: false, reason }).mockResolvedValueOnce({ available: true, reason: 'ready' })
|
||||
const api = await import('@/api/hermes/system')
|
||||
expect(await api.checkHermesCliAvailable()).toBe(false)
|
||||
api.resetHermesCliProbe()
|
||||
expect(await api.checkHermesCliAvailable()).toBe(true)
|
||||
expect(request).toHaveBeenLastCalledWith('/api/hermes/cli-status?refresh=1')
|
||||
})
|
||||
|
||||
it('continues to work with older servers and transport errors', async () => {
|
||||
const api = await import('@/api/hermes/system')
|
||||
request.mockResolvedValueOnce({ available: false })
|
||||
expect(await api.checkHermesCliAvailable()).toBe(false)
|
||||
api.resetHermesCliProbe()
|
||||
request.mockRejectedValueOnce(new Error('server restarting'))
|
||||
expect(await api.checkHermesCliAvailable()).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,33 @@
|
||||
import { expect, test } from '@playwright/test'
|
||||
import { mockHermesApi } from './fixtures'
|
||||
|
||||
test('a CLI timeout does not redirect to the installation guide', async ({ page }) => {
|
||||
await mockHermesApi(page)
|
||||
await page.route(url => url.pathname === '/api/hermes/cli-status', route => route.fulfill({
|
||||
json: { available: false, reason: 'timeout', bin: '/opt/hermes/bin/hermes' },
|
||||
}))
|
||||
await page.goto('/')
|
||||
await expect(page.getByPlaceholder('Username')).toBeVisible()
|
||||
await expect(page.getByText('pip install hermes-agent')).toHaveCount(0)
|
||||
})
|
||||
|
||||
for (const [reason, title] of [
|
||||
['permission_denied', 'Hermes CLI cannot be executed'],
|
||||
['failed', 'Hermes CLI failed to start'],
|
||||
['missing', 'Hermes CLI not detected'],
|
||||
]) {
|
||||
test(`explains ${reason} and retries without waiting for the server cache`, async ({ page }) => {
|
||||
await mockHermesApi(page)
|
||||
let forced = false
|
||||
await page.route(url => url.pathname === '/api/hermes/cli-status', async route => {
|
||||
forced = new URL(route.request().url()).searchParams.get('refresh') === '1'
|
||||
await route.fulfill({ json: { available: forced, reason: forced ? 'ready' : reason, bin: '/opt/hermes/bin/hermes' } })
|
||||
})
|
||||
await page.goto('/')
|
||||
await expect(page.getByRole('heading', { name: title, exact: true })).toBeVisible()
|
||||
if (reason !== 'missing') await expect(page.getByText('pip install hermes-agent')).toHaveCount(0)
|
||||
await page.getByRole('button', { name: 'Re-check' }).click()
|
||||
await expect(page.getByPlaceholder('Username')).toBeVisible()
|
||||
expect(forced).toBe(true)
|
||||
})
|
||||
}
|
||||
@@ -0,0 +1,29 @@
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from 'fs'
|
||||
import { tmpdir } from 'os'
|
||||
import { dirname, join, resolve } from 'path'
|
||||
import { spawnSync } from 'child_process'
|
||||
|
||||
const dirs: string[] = []
|
||||
afterEach(() => dirs.splice(0).forEach(dir => rmSync(dir, { recursive: true, force: true })))
|
||||
|
||||
describe.skipIf(process.platform === 'win32')('deployment CLI readiness', () => {
|
||||
it.each([true, false])('requires executable CLI readiness even when HTTP checks pass: %s', available => {
|
||||
const root = mkdtempSync(join(tmpdir(), 'hermes-deploy-test-'))
|
||||
dirs.push(root)
|
||||
const bin = join(root, 'bin')
|
||||
mkdirSync(bin)
|
||||
const shim = (name: string, body: string) => writeFileSync(join(bin, name), `#!/bin/sh\n${body}\n`, { mode: 0o755 })
|
||||
shim('npm', 'if [ "$1" = "-v" ]; then echo 11; fi\nexit 0')
|
||||
shim('systemctl', 'exit 0')
|
||||
shim('curl', `case "$*" in\n *cli-status*) echo '${JSON.stringify({ available, reason: available ? 'ready' : 'permission_denied' })}' ;;\n *) printf 200 ;;\nesac`)
|
||||
const result = spawnSync('bash', [resolve('deploy.sh'), '--skip-pull', '--skip-deps', '--skip-build'], {
|
||||
encoding: 'utf8',
|
||||
timeout: 15000,
|
||||
env: { ...process.env, PATH: [bin, dirname(process.execPath), process.env.PATH].join(':'), HERMES_UI_STATE_DIR: join(root, 'state'), HERMES_UI_DEPLOY_SCOPED: '1' },
|
||||
})
|
||||
expect(result.error).toBeUndefined()
|
||||
expect(result.status).toBe(available ? 0 : 1)
|
||||
expect(result.stdout + result.stderr).toContain(available ? 'Hermes CLI is executable by the service' : 'permission_denied')
|
||||
})
|
||||
})
|
||||
@@ -16,6 +16,7 @@ type LoadHealthControllerOptions = {
|
||||
bridgeReadinessError?: Error
|
||||
managerError?: Error
|
||||
runtimeStateError?: Error
|
||||
cliProbe?: ReturnType<typeof vi.fn>
|
||||
}
|
||||
|
||||
const defaultBridgeReadiness = {
|
||||
@@ -65,6 +66,12 @@ async function loadHealthController(options: LoadHealthControllerOptions = {}) {
|
||||
isDockerContainer: () => options.isDocker === true,
|
||||
}))
|
||||
|
||||
const cliProbe = options.cliProbe || vi.fn().mockResolvedValue({ available: true, reason: 'ready' })
|
||||
vi.doMock('../../packages/server/src/services/hermes/hermes-process', () => ({
|
||||
probeHermesCli: cliProbe,
|
||||
resolveHermesBin: () => '/opt/hermes/bin/hermes',
|
||||
}))
|
||||
|
||||
const health = await import('../../packages/server/src/controllers/health')
|
||||
|
||||
return {
|
||||
@@ -303,3 +310,43 @@ describe('health controller version metadata', () => {
|
||||
})
|
||||
|
||||
})
|
||||
|
||||
describe('CLI status cache', () => {
|
||||
afterEach(() => { vi.useRealTimers(); vi.resetModules() })
|
||||
|
||||
it('coalesces concurrent requests and allows an explicit recheck after failure', async () => {
|
||||
let finish!: (value: any) => void
|
||||
const probe = vi.fn().mockReturnValueOnce(new Promise(resolve => { finish = resolve }))
|
||||
.mockResolvedValue({ available: true, reason: 'ready' })
|
||||
const { hermesCliStatus } = await loadHealthController({ cliProbe: probe })
|
||||
const first = createMockCtx()
|
||||
const second = createMockCtx()
|
||||
const pending = [hermesCliStatus(first), hermesCliStatus(second)]
|
||||
expect(probe).toHaveBeenCalledOnce()
|
||||
finish({ available: false, reason: 'timeout' })
|
||||
await Promise.all(pending)
|
||||
expect(first.body).toMatchObject({ available: false, reason: 'timeout' })
|
||||
expect(second.body).toEqual(first.body)
|
||||
await hermesCliStatus(createMockCtx())
|
||||
expect(probe).toHaveBeenCalledOnce()
|
||||
const retry = { ...createMockCtx(), query: { refresh: '1' } }
|
||||
await hermesCliStatus(retry)
|
||||
expect(retry.body).toMatchObject({ available: true, reason: 'ready' })
|
||||
expect(probe).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('expires failures quickly and starts successful cache lifetime after the probe completes', async () => {
|
||||
vi.useFakeTimers()
|
||||
const probe = vi.fn().mockResolvedValueOnce({ available: false, reason: 'permission_denied' })
|
||||
.mockImplementationOnce(() => new Promise(resolve => setTimeout(() => resolve({ available: true, reason: 'ready' }), 8000)))
|
||||
const { hermesCliStatus } = await loadHealthController({ cliProbe: probe })
|
||||
await hermesCliStatus(createMockCtx())
|
||||
await vi.advanceTimersByTimeAsync(5001)
|
||||
const pending = hermesCliStatus(createMockCtx())
|
||||
await vi.advanceTimersByTimeAsync(8000)
|
||||
await pending
|
||||
await vi.advanceTimersByTimeAsync(29000)
|
||||
await hermesCliStatus(createMockCtx())
|
||||
expect(probe).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -4,12 +4,14 @@ import { tmpdir } from 'os'
|
||||
import { join } from 'path'
|
||||
|
||||
const execFileCalls = vi.hoisted(() => [] as Array<{ command: string; args: string[]; options: any }>)
|
||||
const execResult = vi.hoisted(() => ({ error: null as any, delayMs: 0 }))
|
||||
const spawnCalls = vi.hoisted(() => [] as Array<{ command: string; args: string[]; options: any }>)
|
||||
|
||||
vi.mock('child_process', () => ({
|
||||
execFile: vi.fn((command: string, args: string[], options: any, callback: (error: Error | null, stdout: string, stderr: string) => void) => {
|
||||
execFileCalls.push({ command, args, options })
|
||||
callback(null, 'ok\n', '')
|
||||
if (execResult.delayMs) setTimeout(() => callback(execResult.error, 'ok\n', ''), execResult.delayMs)
|
||||
else callback(execResult.error, 'ok\n', '')
|
||||
}),
|
||||
spawn: vi.fn((command: string, args: string[], options: any) => {
|
||||
spawnCalls.push({ command, args, options })
|
||||
@@ -24,6 +26,9 @@ function setPlatform(platform: NodeJS.Platform): void {
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
execResult.error = null
|
||||
execResult.delayMs = 0
|
||||
vi.useRealTimers()
|
||||
execFileCalls.length = 0
|
||||
spawnCalls.length = 0
|
||||
delete process.env.HERMES_AGENT_BRIDGE_PYTHON
|
||||
@@ -122,3 +127,31 @@ describe('Hermes process invocation', () => {
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('Hermes installation probe', () => {
|
||||
it('uses local help instead of the network-dependent version check and accepts slow startup', async () => {
|
||||
vi.useFakeTimers()
|
||||
execResult.delayMs = 7800
|
||||
const { probeHermesCli } = await import('../../packages/server/src/services/hermes/hermes-process')
|
||||
const pending = probeHermesCli('/opt/hermes/bin/hermes')
|
||||
await vi.advanceTimersByTimeAsync(7800)
|
||||
await expect(pending).resolves.toEqual({ available: true, reason: 'ready' })
|
||||
expect(execFileCalls[0]).toMatchObject({ args: ['--help'], options: { timeout: 15000 } })
|
||||
})
|
||||
|
||||
it.each([
|
||||
[{ code: 'ENOENT' }, 'missing'],
|
||||
[{ code: 'ENOTDIR' }, 'missing'],
|
||||
[{ code: 127 }, 'missing'],
|
||||
[{ code: 'EACCES' }, 'permission_denied'],
|
||||
[{ code: 'EPERM' }, 'permission_denied'],
|
||||
[{ code: 126 }, 'permission_denied'],
|
||||
[{ killed: true, signal: 'SIGTERM' }, 'timeout'],
|
||||
[{ code: 'ETIMEDOUT' }, 'timeout'],
|
||||
[{ code: 1 }, 'failed'],
|
||||
])('distinguishes %j from a missing installation', async (error, reason) => {
|
||||
execResult.error = error
|
||||
const { probeHermesCli } = await import('../../packages/server/src/services/hermes/hermes-process')
|
||||
await expect(probeHermesCli('hermes')).resolves.toEqual({ available: false, reason })
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user