mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(desktop): read a readiness-probe 401 by whether it was credentialed
The boot readiness probe calls the credential-free /api/health, and only a 404 flips it to the /api/status fallback. Both halves are wrong against a gated gateway. /api/health landed inccab46ca4(2026-07-24), after the v2026.7.20 tag, so every container pinned to a release lacks the route. On those backends the dashboard auth gate runs ahead of the SPA catch-all, so an unknown /api/* path is rejected as unauthenticated rather than 404 — a credential-free probe can never observe the 404 that the fallback keys on, and boot loops until the 45s deadline reporting a healthy backend as "did not become ready". Upgrading the backend is not a workaround for release-pinned deployments. Simulating a 0.19.0 backend (both the route and its PUBLIC_API_PATHS entry removed, sinceccab46ca4added the two together) shows the probe's 401 means two different things depending on whether credentials were sent: credential-free: /api/health -> 401 no_cookie, /api/status -> 200 credentialed: /api/health -> 404, /api/sessions -> 200 So the fix is not "fall back on any 401". Uncredentialed, a gate-shaped 401 identifies a missing route and must fall back. Credentialed, a 401/403 is a rejected session and must fail fast — falling back would hit the public /api/status, get a 200, and report a dead session as ready, deferring the no_cookie to the first real API call. Split the two cases and tag the credentialed rejection as a terminal reauth error. A generic 401 without the gate shape, plus 429 and 5xx, keep polling as before.
This commit is contained in:
Brooklyn Nicholson
parent
a606d24cf2
commit
8d025489cf
2 changed files with 284 additions and 5 deletions
|
|
@ -2,7 +2,17 @@ import assert from 'node:assert/strict'
|
|||
|
||||
import { test } from 'vitest'
|
||||
|
||||
import { DEFAULT_HEALTH_PROBE_TIMEOUT_MS, isMissingHealthEndpointError, waitForHermesReady } from './backend-health'
|
||||
import {
|
||||
DEFAULT_HEALTH_PROBE_TIMEOUT_MS,
|
||||
isAuthRejectionError,
|
||||
isGatedMissingHealthError,
|
||||
isMissingHealthEndpointError,
|
||||
isReauthRequiredError,
|
||||
waitForHermesReady
|
||||
} from './backend-health'
|
||||
|
||||
const GATE_401 =
|
||||
'401: {"error":"unauthenticated","detail":"Unauthorized","reason":"no_cookie","login_url":"/login"}'
|
||||
|
||||
test('uses lightweight /api/health for current backends', async () => {
|
||||
const calls: string[][] = []
|
||||
|
|
@ -134,3 +144,198 @@ test('recognizes missing-route shapes only', () => {
|
|||
assert.equal(isMissingHealthEndpointError(new Error('Timed out connecting to Hermes backend after 15000ms')), false)
|
||||
assert.equal(isMissingHealthEndpointError(new Error('500: boom')), false)
|
||||
})
|
||||
|
||||
// --- Gated backends that predate /api/health (release 0.19.0 and earlier) ---
|
||||
//
|
||||
// The dashboard auth gate runs ahead of the SPA catch-all, so on a backend
|
||||
// without the route an ANONYMOUS probe is rejected as unauthenticated rather
|
||||
// than 404 — verified against a simulated 0.19.0 backend:
|
||||
// credential-free: /api/health -> 401 no_cookie, /api/status -> 200
|
||||
// credentialed: /api/health -> 404, /api/sessions -> 200
|
||||
|
||||
test('anonymous gate-shaped 401 falls back to /api/status (backend predates /api/health)', async () => {
|
||||
const calls: string[][] = []
|
||||
|
||||
await waitForHermesReady('http://192.168.1.132:9119', {
|
||||
token: null,
|
||||
fetchPublicJson: async url => {
|
||||
calls.push(['public', url])
|
||||
throw new Error(GATE_401)
|
||||
},
|
||||
fetchJson: async (url, token) => {
|
||||
calls.push(['token', url, token == null ? 'null' : token])
|
||||
|
||||
return { version: '0.19.0', auth_required: true }
|
||||
},
|
||||
sleep: async () => {},
|
||||
timeoutMs: 100,
|
||||
pollMs: 1
|
||||
})
|
||||
|
||||
assert.deepEqual(calls, [
|
||||
['public', 'http://192.168.1.132:9119/api/health'],
|
||||
['token', 'http://192.168.1.132:9119/api/status', 'null']
|
||||
])
|
||||
})
|
||||
|
||||
test('a credentialed 401 fails fast for reauth instead of reporting a dead session ready', async () => {
|
||||
// The regression a blanket 401->fallback introduces: /api/status is public,
|
||||
// so an expired session would answer 200 and boot would report "ready",
|
||||
// deferring the no_cookie to the first real API call.
|
||||
const calls: string[][] = []
|
||||
|
||||
await assert.rejects(
|
||||
waitForHermesReady('https://gateway.example', {
|
||||
token: 'session-token',
|
||||
fetchPublicJson: async () => {
|
||||
throw new Error('public probe must not be used when credentialed')
|
||||
},
|
||||
fetchJson: async url => {
|
||||
calls.push(['status', url])
|
||||
|
||||
return { version: '0.19.0' }
|
||||
},
|
||||
probeHealth: async url => {
|
||||
calls.push(['probe', url])
|
||||
throw new Error(GATE_401)
|
||||
},
|
||||
probeIsCredentialed: true,
|
||||
sleep: async () => {},
|
||||
timeoutMs: 100,
|
||||
pollMs: 1
|
||||
}),
|
||||
(error: any) => {
|
||||
assert.equal(isReauthRequiredError(error), true)
|
||||
assert.equal(error.needsOauthLogin, true)
|
||||
assert.match(error.message, /remote gateway session has expired/i)
|
||||
|
||||
return true
|
||||
}
|
||||
)
|
||||
|
||||
// Fail fast: never reached the public /api/status leg.
|
||||
assert.deepEqual(calls, [['probe', 'https://gateway.example/api/health']])
|
||||
})
|
||||
|
||||
test('a credentialed 403 is also a terminal reauth failure', async () => {
|
||||
await assert.rejects(
|
||||
waitForHermesReady('https://gateway.example', {
|
||||
fetchPublicJson: async () => ({}),
|
||||
fetchJson: async () => ({}),
|
||||
probeHealth: async () => {
|
||||
throw new Error('403: {"detail":"Forbidden"}')
|
||||
},
|
||||
probeIsCredentialed: true,
|
||||
sleep: async () => {},
|
||||
timeoutMs: 100,
|
||||
pollMs: 1
|
||||
}),
|
||||
(error: any) => isReauthRequiredError(error)
|
||||
)
|
||||
})
|
||||
|
||||
test('a credentialed probe still uses the 404 fallback for a genuinely missing route', async () => {
|
||||
// With credentials the gate lets the request through to the SPA catch-all,
|
||||
// so an old backend answers a real 404 — that must still fall back, not be
|
||||
// mistaken for a rejected session.
|
||||
const calls: string[][] = []
|
||||
|
||||
await waitForHermesReady('https://gateway.example', {
|
||||
token: 'session-token',
|
||||
fetchPublicJson: async () => {
|
||||
throw new Error('public probe must not be used when credentialed')
|
||||
},
|
||||
fetchJson: async url => {
|
||||
calls.push(['status', url])
|
||||
|
||||
return { version: '0.19.0' }
|
||||
},
|
||||
probeHealth: async url => {
|
||||
calls.push(['probe', url])
|
||||
throw new Error('404: {"detail":"Not Found"}')
|
||||
},
|
||||
probeIsCredentialed: true,
|
||||
sleep: async () => {},
|
||||
timeoutMs: 100,
|
||||
pollMs: 1
|
||||
})
|
||||
|
||||
assert.deepEqual(calls, [
|
||||
['probe', 'https://gateway.example/api/health'],
|
||||
['status', 'https://gateway.example/api/status']
|
||||
])
|
||||
})
|
||||
|
||||
test('a non-gate 401 keeps polling rather than skipping a misconfigured health route', async () => {
|
||||
const calls: string[][] = []
|
||||
let currentTime = 0
|
||||
|
||||
await assert.rejects(
|
||||
waitForHermesReady('http://127.0.0.1:9000', {
|
||||
fetchPublicJson: async url => {
|
||||
calls.push(['public', url])
|
||||
throw new Error('401: {"detail":"Unauthorized"}')
|
||||
},
|
||||
fetchJson: async url => {
|
||||
calls.push(['token', url])
|
||||
},
|
||||
sleep: async () => {},
|
||||
now: () => {
|
||||
currentTime += 20
|
||||
|
||||
return currentTime
|
||||
},
|
||||
timeoutMs: 50,
|
||||
pollMs: 1
|
||||
}),
|
||||
/401: \{"detail":"Unauthorized"\}/
|
||||
)
|
||||
|
||||
assert.ok(calls.length > 0)
|
||||
assert.ok(calls.every(call => call[0] === 'public' && call[1].endsWith('/api/health')))
|
||||
})
|
||||
|
||||
test('credentialed 5xx and 429 keep polling — only 401/403 are terminal', async () => {
|
||||
for (const transient of ['500: boom', '429: {"detail":"Too Many Requests"}']) {
|
||||
let attempts = 0
|
||||
let currentTime = 0
|
||||
|
||||
await assert.rejects(
|
||||
waitForHermesReady('https://gateway.example', {
|
||||
fetchPublicJson: async () => ({}),
|
||||
fetchJson: async () => ({}),
|
||||
probeHealth: async () => {
|
||||
attempts += 1
|
||||
throw new Error(transient)
|
||||
},
|
||||
probeIsCredentialed: true,
|
||||
sleep: async () => {},
|
||||
now: () => {
|
||||
currentTime += 20
|
||||
|
||||
return currentTime
|
||||
},
|
||||
timeoutMs: 100,
|
||||
pollMs: 1
|
||||
}),
|
||||
(error: any) => isReauthRequiredError(error) === false
|
||||
)
|
||||
|
||||
assert.ok(attempts > 1, `${transient} should have retried, got ${attempts} attempt(s)`)
|
||||
}
|
||||
})
|
||||
|
||||
test('error-shape predicates', () => {
|
||||
assert.equal(isGatedMissingHealthError(new Error(GATE_401)), true)
|
||||
assert.equal(isGatedMissingHealthError(new Error('401: {"detail":"Unauthorized"}')), false)
|
||||
assert.equal(isGatedMissingHealthError(new Error('404: {"detail":"Not Found"}')), false)
|
||||
|
||||
assert.equal(isAuthRejectionError(new Error(GATE_401)), true)
|
||||
assert.equal(isAuthRejectionError(new Error('403: {"detail":"Forbidden"}')), true)
|
||||
assert.equal(isAuthRejectionError(new Error('404: {"detail":"Not Found"}')), false)
|
||||
assert.equal(isAuthRejectionError(new Error('429: slow down')), false)
|
||||
assert.equal(isAuthRejectionError(new Error('500: boom')), false)
|
||||
|
||||
// A gated 401 must NOT be conflated with a missing route by the 404 predicate.
|
||||
assert.equal(isMissingHealthEndpointError(new Error(GATE_401)), false)
|
||||
})
|
||||
|
|
|
|||
|
|
@ -20,14 +20,72 @@ export interface HermesReadyOptions {
|
|||
healthProbeTimeoutMs?: number
|
||||
sleep?: (ms: number) => Promise<void>
|
||||
now?: () => number
|
||||
/**
|
||||
* Credentialed health probe. When supplied, readiness is probed with the
|
||||
* connection's own credentials instead of anonymously — which is what lets
|
||||
* a gated backend answer 404 for a genuinely missing /api/health, and what
|
||||
* makes a 401 from this probe mean "session rejected" rather than "route
|
||||
* behind a gate". Defaults to the credential-free `fetchPublicJson`.
|
||||
*/
|
||||
probeHealth?: (url: string, options?: { timeoutMs?: number }) => Promise<unknown>
|
||||
/**
|
||||
* Whether `probeHealth` actually presents credentials. Distinguishes the
|
||||
* two very different meanings of a 401 (see `waitForHermesReady`).
|
||||
*/
|
||||
probeIsCredentialed?: boolean
|
||||
}
|
||||
|
||||
export const REMOTE_SESSION_EXPIRED_MESSAGE =
|
||||
'Your remote gateway session has expired. Open Settings → Gateway and click "Sign in" again.'
|
||||
|
||||
export function isMissingHealthEndpointError(error: unknown): boolean {
|
||||
const message = error instanceof Error ? error.message : String(error ?? '')
|
||||
|
||||
return /^404:/.test(message) || message.includes('endpoint is likely missing')
|
||||
}
|
||||
|
||||
/**
|
||||
* True for a hard auth rejection (401/403) as opposed to a transient failure.
|
||||
* Deliberately shape-based: 429 is a throttle and 5xx is a server fault, and
|
||||
* both must keep polling.
|
||||
*/
|
||||
export function isAuthRejectionError(error: unknown): boolean {
|
||||
const message = error instanceof Error ? error.message : String(error ?? '')
|
||||
|
||||
return /^40[13]:/.test(message)
|
||||
}
|
||||
|
||||
/**
|
||||
* True for an auth rejection carrying the dashboard gate's "no session at all"
|
||||
* shape. On a backend that predates `/api/health`, the gate runs ahead of the
|
||||
* SPA catch-all, so an unknown `/api/*` path is rejected as unauthenticated
|
||||
* instead of 404 — this is the signal that an ANONYMOUS probe cannot reach the
|
||||
* route, and the reason a credential-free 401 must fall back to `/api/status`
|
||||
* rather than be reported as a boot failure.
|
||||
*/
|
||||
export function isGatedMissingHealthError(error: unknown): boolean {
|
||||
const message = error instanceof Error ? error.message : String(error ?? '')
|
||||
|
||||
return isAuthRejectionError(error) && message.includes('no_cookie')
|
||||
}
|
||||
|
||||
/** Tag a terminal reauth failure the main process latches and the overlay keys on. */
|
||||
export function makeReauthRequiredError(detail?: string): Error {
|
||||
const error = new Error(REMOTE_SESSION_EXPIRED_MESSAGE) as any
|
||||
error.needsOauthLogin = true
|
||||
error.isReauthRequired = true
|
||||
|
||||
if (detail) {
|
||||
error.detail = detail
|
||||
}
|
||||
|
||||
return error
|
||||
}
|
||||
|
||||
export function isReauthRequiredError(error: unknown): boolean {
|
||||
return Boolean((error as any)?.isReauthRequired)
|
||||
}
|
||||
|
||||
function supersededError() {
|
||||
const error: any = new Error('SSH bootstrap was superseded by newer connection settings.')
|
||||
error.kind = 'superseded'
|
||||
|
|
@ -59,6 +117,8 @@ export async function waitForHermesReady(baseUrl: string, options: HermesReadyOp
|
|||
|
||||
const base = baseUrl.replace(/\/+$/, '')
|
||||
const deadline = now() + timeoutMs
|
||||
const probeHealth = options.probeHealth ?? options.fetchPublicJson
|
||||
const probeIsCredentialed = Boolean(options.probeIsCredentialed)
|
||||
let lastError: unknown = null
|
||||
let useStatusFallback = false
|
||||
|
||||
|
|
@ -71,16 +131,30 @@ export async function waitForHermesReady(baseUrl: string, options: HermesReadyOp
|
|||
if (useStatusFallback) {
|
||||
await options.fetchJson(`${base}/api/status`, options.token)
|
||||
} else {
|
||||
await options.fetchPublicJson(`${base}/api/health`, { timeoutMs: healthProbeTimeoutMs })
|
||||
await probeHealth(`${base}/api/health`, { timeoutMs: healthProbeTimeoutMs })
|
||||
}
|
||||
|
||||
return
|
||||
} catch (error) {
|
||||
lastError = error
|
||||
|
||||
// Only an explicitly missing route means the backend predates
|
||||
// /api/health; timeouts and server errors keep polling health.
|
||||
if (!useStatusFallback && isMissingHealthEndpointError(error)) {
|
||||
// A confirmed 401/403 from a CREDENTIALED probe means the session was
|
||||
// rejected, not that the route is missing. Fail fast into a reauth
|
||||
// state: falling back to the public /api/status would answer 200 and
|
||||
// report a dead session as "ready", deferring the failure to the first
|
||||
// real API call. Applies to the /api/status leg too — it is routed
|
||||
// through the same credentials.
|
||||
if (probeIsCredentialed && isAuthRejectionError(error)) {
|
||||
throw makeReauthRequiredError(error instanceof Error ? error.message : String(error))
|
||||
}
|
||||
|
||||
// An explicitly missing route means the backend predates /api/health.
|
||||
// So does a gate-shaped 401 on an ANONYMOUS probe: the dashboard auth
|
||||
// gate runs ahead of the SPA catch-all, so a pre-/api/health backend
|
||||
// rejects the unknown path as unauthenticated instead of 404 and a
|
||||
// credential-free probe can never observe the 404. Timeouts, 5xx, 429,
|
||||
// and non-gate 401s keep polling health.
|
||||
if (!useStatusFallback && (isMissingHealthEndpointError(error) || isGatedMissingHealthError(error))) {
|
||||
useStatusFallback = true
|
||||
|
||||
continue
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue