mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
Merge pull request #69884 from NousResearch/bb/salvage-55707-onboarding
fix(desktop): stop spurious onboarding for keyless custom providers (supersedes #55707, #45224)
This commit is contained in:
commit
0721d2ea80
9 changed files with 152 additions and 25 deletions
|
|
@ -23,7 +23,7 @@ import { refreshBackgroundProcesses } from '@/store/composer-status'
|
|||
import { $gateway } from '@/store/gateway'
|
||||
import { dispatchNativeNotification } from '@/store/native-notifications'
|
||||
import { notify } from '@/store/notifications'
|
||||
import { requestDesktopOnboarding } from '@/store/onboarding'
|
||||
import { requestDesktopOnboarding, requestDesktopOnboardingForCredentialWarning } from '@/store/onboarding'
|
||||
import { revealDesktopPane } from '@/store/pane-focus'
|
||||
import { flashPetActivity, markPetUnread, setPetActivity } from '@/store/pet'
|
||||
import { $activeGatewayProfile, normalizeProfileKey } from '@/store/profile'
|
||||
|
|
@ -407,9 +407,7 @@ export function useGatewayEventHandler(deps: GatewayEventDeps) {
|
|||
setCurrentUsage(current => ({ ...current, ...payload.usage }))
|
||||
}
|
||||
|
||||
if (typeof payload?.credential_warning === 'string' && payload.credential_warning) {
|
||||
requestDesktopOnboarding(payload.credential_warning)
|
||||
}
|
||||
requestDesktopOnboardingForCredentialWarning(payload?.credential_warning)
|
||||
|
||||
if (apply) {
|
||||
reportInstallMethodWarning(payload?.install_warning)
|
||||
|
|
|
|||
|
|
@ -1,7 +1,8 @@
|
|||
import { beforeEach, describe, expect, it } from 'vitest'
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
|
||||
|
||||
import type { ChatMessage } from '@/lib/chat-messages'
|
||||
import { $approvalModes, approvalModeForProfile } from '@/store/approval-mode'
|
||||
import { $desktopOnboarding } from '@/store/onboarding'
|
||||
import { $activeGatewayProfile } from '@/store/profile'
|
||||
import type { SessionInfo } from '@/types/hermes'
|
||||
|
||||
|
|
@ -38,6 +39,32 @@ describe('applyRuntimeInfo approval mode', () => {
|
|||
})
|
||||
})
|
||||
|
||||
const initialOnboardingState = $desktopOnboarding.get()
|
||||
|
||||
describe('applyRuntimeInfo credential warnings', () => {
|
||||
beforeEach(() => {
|
||||
$desktopOnboarding.set({ ...initialOnboardingState, reason: null, requested: false })
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
$desktopOnboarding.set(initialOnboardingState)
|
||||
})
|
||||
|
||||
it('requests setup for the exact empty-key warning returned by the server', () => {
|
||||
const warning = "No API key configured for provider 'openrouter'. First message will fail."
|
||||
|
||||
applyRuntimeInfo({ credential_warning: warning })
|
||||
|
||||
expect($desktopOnboarding.get()).toMatchObject({ reason: warning, requested: true })
|
||||
})
|
||||
|
||||
it('ignores an auxiliary-provider warning', () => {
|
||||
applyRuntimeInfo({ credential_warning: 'OPENROUTER_API_KEY not set' })
|
||||
|
||||
expect($desktopOnboarding.get()).toMatchObject({ reason: null, requested: false })
|
||||
})
|
||||
})
|
||||
|
||||
describe('isSessionGoneError', () => {
|
||||
it('is true for 404 / session-not-found, false otherwise', () => {
|
||||
expect(isSessionGoneError(new Error('Request failed 404'))).toBe(true)
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ import { assistantTextPart, type ChatMessage, chatMessageText, textPart } from '
|
|||
import { normalizePersonalityValue } from '@/lib/chat-runtime'
|
||||
import { embeddedImageUrls, textWithoutEmbeddedImages } from '@/lib/embedded-images'
|
||||
import { reconcileApprovalModeForProfile } from '@/store/approval-mode'
|
||||
import { requestDesktopOnboarding } from '@/store/onboarding'
|
||||
import { requestDesktopOnboardingForCredentialWarning } from '@/store/onboarding'
|
||||
import { $activeGatewayProfile, $profiles, normalizeProfileKey } from '@/store/profile'
|
||||
import {
|
||||
$currentCwd,
|
||||
|
|
@ -557,9 +557,7 @@ export function applyRuntimeInfo(info: SessionRuntimeInfo | undefined): SessionR
|
|||
reconcileApprovalModeForProfile($activeGatewayProfile.get(), info.approval_mode)
|
||||
}
|
||||
|
||||
if (info.credential_warning) {
|
||||
requestDesktopOnboarding(info.credential_warning)
|
||||
}
|
||||
requestDesktopOnboardingForCredentialWarning(info.credential_warning)
|
||||
|
||||
reportInstallMethodWarning(info.install_warning)
|
||||
|
||||
|
|
|
|||
|
|
@ -12,6 +12,24 @@ describe('isProviderSetupErrorMessage', () => {
|
|||
expect(isProviderSetupErrorMessage('set an API key (OPENROUTER_API_KEY) in ~/.hermes/.env')).toBe(true)
|
||||
})
|
||||
|
||||
it('matches the exact empty-key warning emitted in session.info', () => {
|
||||
expect(
|
||||
isProviderSetupErrorMessage("No API key configured for provider 'openrouter'. First message will fail.")
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('does not match bare env var mentions from auxiliary warnings', () => {
|
||||
expect(isProviderSetupErrorMessage('OPENROUTER_API_KEY not set')).toBe(false)
|
||||
expect(isProviderSetupErrorMessage('Run `hermes setup` or set OPENROUTER_API_KEY.')).toBe(false)
|
||||
expect(
|
||||
isProviderSetupErrorMessage(
|
||||
'⚠ No auxiliary LLM provider configured — context compression will drop middle turns without a summary. Run `hermes setup` or set OPENROUTER_API_KEY.'
|
||||
)
|
||||
).toBe(false)
|
||||
expect(isProviderSetupErrorMessage('OPENAI_API_KEY missing')).toBe(false)
|
||||
expect(isProviderSetupErrorMessage('ANTHROPIC_API_KEY not found')).toBe(false)
|
||||
})
|
||||
|
||||
it('does not match non-provider runtime failures', () => {
|
||||
expect(
|
||||
isProviderSetupErrorMessage('Selected runtime is not available. setup.status reports configured credentials.')
|
||||
|
|
|
|||
|
|
@ -1,5 +1,7 @@
|
|||
const PROVIDER_SETUP_ERROR_RE =
|
||||
/No (?:inference|Hermes) provider(?: is)? configured|no_provider_configured|OPENROUTER_API_KEY|OPENAI_API_KEY|ANTHROPIC_API_KEY|set an API key/i
|
||||
/No (?:inference|Hermes) provider(?: is)? configured|no_provider_configured|set an API key/i
|
||||
|
||||
const SESSION_INFO_CREDENTIAL_WARNING_RE = /^No API key configured for provider '[^']*'\. First message will fail\.$/
|
||||
|
||||
export function isProviderSetupErrorMessage(message: null | string | undefined): boolean {
|
||||
const text = message?.trim()
|
||||
|
|
@ -8,5 +10,5 @@ export function isProviderSetupErrorMessage(message: null | string | undefined):
|
|||
return false
|
||||
}
|
||||
|
||||
return PROVIDER_SETUP_ERROR_RE.test(text)
|
||||
return PROVIDER_SETUP_ERROR_RE.test(text) || SESSION_INFO_CREDENTIAL_WARNING_RE.test(text)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -46,14 +46,28 @@ function installApiMock(api: (request: { path: string }) => Promise<unknown>) {
|
|||
})
|
||||
}
|
||||
|
||||
function runtimeMismatchGateway(): OnboardingContext['requestGateway'] {
|
||||
function emptyOpenRouterGateway(): OnboardingContext['requestGateway'] {
|
||||
return async method => {
|
||||
if (method === 'setup.status') {
|
||||
return { provider_configured: true } as never
|
||||
}
|
||||
|
||||
if (method === 'setup.runtime_check') {
|
||||
return { error: 'Selected runtime is not available.', ok: false } as never
|
||||
return { error: 'No usable credentials found for openrouter.', ok: false, provider: 'openrouter' } as never
|
||||
}
|
||||
|
||||
throw new Error(`unexpected gateway method: ${method}`)
|
||||
}
|
||||
}
|
||||
|
||||
function keylessCustomGateway(): OnboardingContext['requestGateway'] {
|
||||
return async method => {
|
||||
if (method === 'setup.status') {
|
||||
return { provider_configured: true } as never
|
||||
}
|
||||
|
||||
if (method === 'setup.runtime_check') {
|
||||
return { ok: true, provider: 'custom' } as never
|
||||
}
|
||||
|
||||
throw new Error(`unexpected gateway method: ${method}`)
|
||||
|
|
@ -99,12 +113,12 @@ describe('refreshOnboarding', () => {
|
|||
$desktopOnboarding.set(baseState({ providers: [provider('cached')] }))
|
||||
requestDesktopOnboarding('Need provider setup')
|
||||
|
||||
const ready = await refreshOnboarding(onboardingContext(runtimeMismatchGateway()))
|
||||
const ready = await refreshOnboarding(onboardingContext(emptyOpenRouterGateway()))
|
||||
|
||||
expect(ready).toBe(false)
|
||||
expect(api).toHaveBeenCalledTimes(1)
|
||||
expect($desktopOnboarding.get().providers?.map(p => p.id)).toEqual(['fresh'])
|
||||
expect($desktopOnboarding.get().reason).toContain('Selected runtime is not available.')
|
||||
expect($desktopOnboarding.get().reason).toContain('No usable credentials found for openrouter.')
|
||||
expect($desktopOnboarding.get().reason).toContain('setup.status reports configured credentials')
|
||||
})
|
||||
|
||||
|
|
@ -120,7 +134,7 @@ describe('refreshOnboarding', () => {
|
|||
installApiMock(api)
|
||||
$desktopOnboarding.set(baseState({ providers: [provider('cached')] }))
|
||||
|
||||
const ready = await refreshOnboarding(onboardingContext(runtimeMismatchGateway()))
|
||||
const ready = await refreshOnboarding(onboardingContext(emptyOpenRouterGateway()))
|
||||
|
||||
expect(ready).toBe(false)
|
||||
expect(api).not.toHaveBeenCalled()
|
||||
|
|
@ -182,6 +196,43 @@ describe('refreshOnboarding', () => {
|
|||
expect($desktopOnboarding.get().configured).toBe(true)
|
||||
})
|
||||
|
||||
it('enters setup when the selected OpenRouter credential is genuinely empty', async () => {
|
||||
installApiMock(vi.fn())
|
||||
window.localStorage.setItem('hermes-desktop-onboarded-v1', '1')
|
||||
$desktopOnboarding.set(
|
||||
baseState({
|
||||
configured: true,
|
||||
providers: [provider('cached')],
|
||||
reason: null,
|
||||
requested: false
|
||||
})
|
||||
)
|
||||
|
||||
const ready = await refreshOnboarding(onboardingContext(emptyOpenRouterGateway()))
|
||||
|
||||
expect(ready).toBe(false)
|
||||
expect($desktopOnboarding.get().configured).toBe(false)
|
||||
expect($desktopOnboarding.get().reason).toContain('No usable credentials found for openrouter.')
|
||||
expect(window.localStorage.getItem('hermes-desktop-onboarded-v1')).toBeNull()
|
||||
})
|
||||
|
||||
it('keeps a keyless custom runtime out of setup', async () => {
|
||||
const api = vi.fn()
|
||||
|
||||
installApiMock(api)
|
||||
$desktopOnboarding.set(baseState({ configured: false, reason: 'stale setup error', requested: true }))
|
||||
|
||||
const ready = await refreshOnboarding(onboardingContext(keylessCustomGateway()))
|
||||
|
||||
expect(ready).toBe(true)
|
||||
expect(api).not.toHaveBeenCalled()
|
||||
expect($desktopOnboarding.get()).toMatchObject({
|
||||
configured: true,
|
||||
reason: null,
|
||||
requested: false
|
||||
})
|
||||
})
|
||||
|
||||
it('does not preserve configured when onboarding was explicitly requested', async () => {
|
||||
const api = vi.fn(async ({ path }: { path: string }) => {
|
||||
if (path === '/api/providers/oauth') {
|
||||
|
|
@ -249,8 +300,8 @@ describe('refreshOnboarding', () => {
|
|||
installApiMock(api)
|
||||
$desktopOnboarding.set(baseState({ requested: true }))
|
||||
|
||||
const first = refreshOnboarding(onboardingContext(runtimeMismatchGateway()))
|
||||
const second = refreshOnboarding(onboardingContext(runtimeMismatchGateway()))
|
||||
const first = refreshOnboarding(onboardingContext(emptyOpenRouterGateway()))
|
||||
const second = refreshOnboarding(onboardingContext(emptyOpenRouterGateway()))
|
||||
|
||||
await vi.waitFor(() => expect(api).toHaveBeenCalledTimes(1))
|
||||
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import {
|
|||
submitOAuthCode,
|
||||
validateProviderCredential
|
||||
} from '@/hermes'
|
||||
import { isProviderSetupErrorMessage } from '@/lib/provider-setup-errors'
|
||||
import { evaluateRuntimeReadiness, type RuntimeReadinessResult } from '@/lib/runtime-readiness'
|
||||
import { notify, notifyError } from '@/store/notifications'
|
||||
import type { ModelOptionProvider, OAuthProvider, OAuthStartResponse } from '@/types/hermes'
|
||||
|
|
@ -185,9 +186,8 @@ async function checkRuntime(ctx: OnboardingContext, requestedProvider?: string):
|
|||
}
|
||||
|
||||
function shouldPreserveConfiguredOnFallback(runtime: RuntimeReadinessResult, state: DesktopOnboardingState): boolean {
|
||||
// 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.
|
||||
// Non-authoritative transport fallback only — keep a previously verified
|
||||
// configured state instead of forcing the blocking onboarding overlay.
|
||||
return runtime.source === 'fallback' && state.configured === true && !state.requested
|
||||
}
|
||||
|
||||
|
|
@ -390,6 +390,16 @@ export function requestDesktopOnboarding(reason = DEFAULT_ONBOARDING_REASON) {
|
|||
patch({ reason: reason.trim() || DEFAULT_ONBOARDING_REASON, requested: true })
|
||||
}
|
||||
|
||||
export function requestDesktopOnboardingForCredentialWarning(reason: null | string | undefined) {
|
||||
const warning = reason?.trim()
|
||||
|
||||
if (!warning || !isProviderSetupErrorMessage(warning)) {
|
||||
return
|
||||
}
|
||||
|
||||
requestDesktopOnboarding(warning)
|
||||
}
|
||||
|
||||
// Open the onboarding provider selector on demand from an already-configured
|
||||
// app — e.g. the model picker's "Add provider" button. Reuses the entire
|
||||
// onboarding flow (OAuth rows, API-key form, model-confirm) instead of
|
||||
|
|
|
|||
|
|
@ -4901,6 +4901,20 @@ def test_setup_status_reports_provider_config(monkeypatch):
|
|||
assert resp["result"]["provider_configured"] is False
|
||||
|
||||
|
||||
def test_probe_credentials_emits_exact_empty_key_warning():
|
||||
agent = types.SimpleNamespace(api_key="", provider="openrouter")
|
||||
|
||||
assert server._probe_credentials(agent) == (
|
||||
"No API key configured for provider 'openrouter'. First message will fail."
|
||||
)
|
||||
|
||||
|
||||
def test_probe_credentials_allows_keyless_custom_runtime():
|
||||
agent = types.SimpleNamespace(api_key="no-key-required", provider="custom")
|
||||
|
||||
assert server._probe_credentials(agent) == ""
|
||||
|
||||
|
||||
def test_setup_runtime_check_rejects_empty_runtime_key(monkeypatch):
|
||||
monkeypatch.setattr("hermes_cli.main._has_any_provider_configured", lambda: True)
|
||||
monkeypatch.setattr(
|
||||
|
|
@ -4914,8 +4928,13 @@ def test_setup_runtime_check_rejects_empty_runtime_key(monkeypatch):
|
|||
|
||||
resp = server.handle_request({"id": "1", "method": "setup.runtime_check", "params": {}})
|
||||
|
||||
assert resp["result"]["ok"] is False
|
||||
assert resp["result"]["provider"] == "openrouter"
|
||||
assert resp["result"] == {
|
||||
"ok": False,
|
||||
"provider": "openrouter",
|
||||
"model": None,
|
||||
"source": "env/config",
|
||||
"error": "No usable credentials found for openrouter.",
|
||||
}
|
||||
|
||||
|
||||
def test_setup_runtime_check_allows_no_key_custom_runtime(monkeypatch):
|
||||
|
|
|
|||
|
|
@ -3866,11 +3866,15 @@ def _get_usage(agent) -> dict:
|
|||
|
||||
|
||||
def _probe_credentials(agent) -> str:
|
||||
"""Light credential check at session creation — returns warning or ''."""
|
||||
"""Light credential check at session creation — returns warning or ''.
|
||||
|
||||
``no-key-required`` is a valid sentinel for keyless custom providers; only
|
||||
warn when the key is genuinely missing.
|
||||
"""
|
||||
try:
|
||||
key = getattr(agent, "api_key", "") or ""
|
||||
provider = getattr(agent, "provider", "") or ""
|
||||
if not key or key == "no-key-required":
|
||||
if not key:
|
||||
return f"No API key configured for provider '{provider}'. First message will fail."
|
||||
except Exception:
|
||||
pass
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue