mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-27 11:22:03 +00:00
fix(desktop): use notify() with stable id for fallback notification
hermes-pr-review findings:
- notifyError('runtime-not-ready', msg) misused the (error, fallback) API:
the key became the notification body and the message became the title.
Switch to notify({ id, kind, title, message }) which puts content in the
right slots.
- The stable id 'runtime-not-ready' deduplicates: notify() replaces by id,
so repeated refreshOnboarding calls during an outage no longer stack
up to 4 persistent error toasts.
- Remove dead !state.manual guard from shouldPreserveConfiguredOnFallback:
refreshOnboarding already short-circuits on manual before the helper.
- Test: seed localStorage with '1' before asserting it survives (was testing
the wrong invariant — null in, null out).
- Test: use static import for spy instead of fragile await import.
- Test: add negative case for requested=true + configured=true (should
still downgrade — requested overrides preservation).
This commit is contained in:
parent
d398076c21
commit
a4a74ca9e9
2 changed files with 48 additions and 14 deletions
|
|
@ -2,6 +2,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
|||
|
||||
import type { OAuthProvider } from '@/types/hermes'
|
||||
|
||||
import * as notifications from '@/store/notifications'
|
||||
|
||||
import {
|
||||
$desktopOnboarding,
|
||||
type DesktopOnboardingState,
|
||||
|
|
@ -136,6 +138,8 @@ describe('refreshOnboarding', () => {
|
|||
})
|
||||
|
||||
installApiMock(api)
|
||||
// Simulate a returning user: cache is set and store is configured.
|
||||
window.localStorage.setItem('hermes-desktop-onboarded-v1', '1')
|
||||
$desktopOnboarding.set(
|
||||
baseState({
|
||||
configured: true,
|
||||
|
|
@ -151,14 +155,12 @@ describe('refreshOnboarding', () => {
|
|||
expect(api).not.toHaveBeenCalled()
|
||||
expect($desktopOnboarding.get().configured).toBe(true)
|
||||
expect($desktopOnboarding.get().reason).toBeNull()
|
||||
expect(window.localStorage.getItem('hermes-desktop-onboarded-v1')).toBeNull()
|
||||
// The cache must survive the refresh — proving we didn't downgrade.
|
||||
expect(window.localStorage.getItem('hermes-desktop-onboarded-v1')).toBe('1')
|
||||
})
|
||||
|
||||
it('shows a non-blocking notification when preserving configured on fallback', async () => {
|
||||
const notifyErrorSpy = vi.spyOn(
|
||||
await import('@/store/notifications'),
|
||||
'notifyError'
|
||||
)
|
||||
const notifySpy = vi.spyOn(notifications, 'notify')
|
||||
|
||||
installApiMock(vi.fn())
|
||||
$desktopOnboarding.set(
|
||||
|
|
@ -172,13 +174,42 @@ describe('refreshOnboarding', () => {
|
|||
|
||||
await refreshOnboarding(onboardingContext(fallbackTimeoutGateway()))
|
||||
|
||||
expect(notifyErrorSpy).toHaveBeenCalledWith(
|
||||
'runtime-not-ready',
|
||||
expect.stringContaining('could not verify the running backend')
|
||||
expect(notifySpy).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
id: 'runtime-not-ready',
|
||||
kind: 'error'
|
||||
})
|
||||
)
|
||||
expect($desktopOnboarding.get().configured).toBe(true)
|
||||
})
|
||||
|
||||
it('does not preserve configured when onboarding was explicitly requested', async () => {
|
||||
const api = vi.fn(async ({ path }: { path: string }) => {
|
||||
if (path === '/api/providers/oauth') {
|
||||
return { providers: [provider('fresh')] }
|
||||
}
|
||||
|
||||
throw new Error(`unexpected api path: ${path}`)
|
||||
})
|
||||
|
||||
installApiMock(api)
|
||||
$desktopOnboarding.set(
|
||||
baseState({
|
||||
configured: true,
|
||||
providers: [provider('cached')],
|
||||
reason: null,
|
||||
requested: true
|
||||
})
|
||||
)
|
||||
|
||||
const ready = await refreshOnboarding(onboardingContext(fallbackTimeoutGateway()))
|
||||
|
||||
expect(ready).toBe(false)
|
||||
// requested overrides preservation — should downgrade.
|
||||
expect($desktopOnboarding.get().configured).toBe(false)
|
||||
expect(api).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('still surfaces onboarding when fallback failure happens before configured state', async () => {
|
||||
const api = vi.fn(async ({ path }: { path: string }) => {
|
||||
if (path === '/api/providers/oauth') {
|
||||
|
|
|
|||
|
|
@ -195,7 +195,7 @@ function shouldPreserveConfiguredOnFallback(
|
|||
// A fallback result means both runtime probes were non-authoritative
|
||||
// (transport timeout/disconnect). Keep a previously verified configured
|
||||
// state instead of forcing the blocking onboarding overlay.
|
||||
return runtime.source === 'fallback' && state.configured === true && !state.requested && !state.manual
|
||||
return runtime.source === 'fallback' && state.configured === true && !state.requested
|
||||
}
|
||||
|
||||
function notifyReady(provider: string) {
|
||||
|
|
@ -528,11 +528,14 @@ export async function refreshOnboarding(ctx: OnboardingContext) {
|
|||
if (shouldPreserveConfiguredOnFallback(runtime, state)) {
|
||||
// Gateway probes timed out but the user was already configured — don't
|
||||
// downgrade to the blocking onboarding overlay. Surface a non-blocking
|
||||
// notification so the user knows the backend wasn't verified.
|
||||
notifyError(
|
||||
'runtime-not-ready',
|
||||
'Hermes Desktop could not verify the running backend on startup. Some features may be unavailable until the gateway is reachable.'
|
||||
)
|
||||
// notification with a stable id so repeated calls during an outage dedup
|
||||
// instead of stacking toasts.
|
||||
notify({
|
||||
id: 'runtime-not-ready',
|
||||
kind: 'error',
|
||||
title: 'Runtime not ready',
|
||||
message: 'Hermes Desktop could not verify the running backend on startup. Some features may be unavailable until the gateway is reachable.'
|
||||
})
|
||||
|
||||
return false
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue