mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-27 11:22:03 +00:00
Merge pull request #52008 from infinitycrew39/fix/desktop-nous-onboarding-stale-provider
fix(desktop): stop Nous Portal onboarding from validating stale Anthropic config
This commit is contained in:
commit
35e9c63d89
6 changed files with 134 additions and 33 deletions
|
|
@ -1,6 +1,6 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { interpretRuntimeReadiness } from './runtime-readiness'
|
||||
import { evaluateRuntimeReadiness, fetchRuntimeReadinessSignals, interpretRuntimeReadiness } from './runtime-readiness'
|
||||
|
||||
describe('interpretRuntimeReadiness', () => {
|
||||
it('prefers runtime_check when both signals exist', () => {
|
||||
|
|
@ -63,3 +63,51 @@ describe('interpretRuntimeReadiness', () => {
|
|||
expect(result.reason).toBe('setup.runtime_check timeout')
|
||||
})
|
||||
})
|
||||
|
||||
describe('fetchRuntimeReadinessSignals', () => {
|
||||
it('scopes setup.runtime_check to the requested provider', async () => {
|
||||
const calls: Array<{ method: string; params?: Record<string, unknown> }> = []
|
||||
const requestGateway = async <T = unknown>(method: string, params?: Record<string, unknown>) => {
|
||||
calls.push({ method, params })
|
||||
|
||||
if (method === 'setup.status') {
|
||||
return { provider_configured: true } as T
|
||||
}
|
||||
|
||||
if (method === 'setup.runtime_check') {
|
||||
return { ok: true } as T
|
||||
}
|
||||
|
||||
throw new Error(`unexpected method: ${method}`)
|
||||
}
|
||||
|
||||
await fetchRuntimeReadinessSignals(requestGateway, 'nous')
|
||||
|
||||
expect(calls).toEqual([
|
||||
{ method: 'setup.status' },
|
||||
{ method: 'setup.runtime_check', params: { provider: 'nous' } }
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
describe('evaluateRuntimeReadiness', () => {
|
||||
it('forwards requestedProvider to setup.runtime_check', async () => {
|
||||
const requestGateway = async <T = unknown>(method: string, params?: Record<string, unknown>) => {
|
||||
if (method === 'setup.status') {
|
||||
return { provider_configured: true } as T
|
||||
}
|
||||
|
||||
if (method === 'setup.runtime_check') {
|
||||
expect(params).toEqual({ provider: 'nous' })
|
||||
|
||||
return { ok: true } as T
|
||||
}
|
||||
|
||||
throw new Error(`unexpected method: ${method}`)
|
||||
}
|
||||
|
||||
const result = await evaluateRuntimeReadiness(requestGateway, { requestedProvider: 'nous' })
|
||||
|
||||
expect(result.ready).toBe(true)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ export interface RuntimeReadinessSignals {
|
|||
|
||||
export interface RuntimeReadinessOptions {
|
||||
defaultReason?: string
|
||||
requestedProvider?: string
|
||||
unknownReady?: boolean
|
||||
}
|
||||
|
||||
|
|
@ -54,21 +55,27 @@ function normalizeMessage(value: null | string | undefined): null | string {
|
|||
|
||||
async function requestWithFallback<T>(
|
||||
requestGateway: RuntimeReadinessRequester,
|
||||
method: string
|
||||
method: string,
|
||||
params?: Record<string, unknown>
|
||||
): Promise<{ error: null | string; value: null | T }> {
|
||||
try {
|
||||
return { error: null, value: await requestGateway<T>(method) }
|
||||
return { error: null, value: await requestGateway<T>(method, params) }
|
||||
} catch (error) {
|
||||
return { error: toErrorMessage(error), value: null }
|
||||
}
|
||||
}
|
||||
|
||||
export async function fetchRuntimeReadinessSignals(
|
||||
requestGateway: RuntimeReadinessRequester
|
||||
requestGateway: RuntimeReadinessRequester,
|
||||
requestedProvider?: string
|
||||
): Promise<RuntimeReadinessSignals> {
|
||||
const runtimeParams = requestedProvider?.trim()
|
||||
? { provider: requestedProvider.trim() }
|
||||
: undefined
|
||||
|
||||
const [setup, runtime] = await Promise.all([
|
||||
requestWithFallback<SetupStatusSnapshot>(requestGateway, 'setup.status'),
|
||||
requestWithFallback<RuntimeCheckSnapshot>(requestGateway, 'setup.runtime_check')
|
||||
requestWithFallback<RuntimeCheckSnapshot>(requestGateway, 'setup.runtime_check', runtimeParams)
|
||||
])
|
||||
|
||||
return {
|
||||
|
|
@ -141,7 +148,7 @@ export async function evaluateRuntimeReadiness(
|
|||
requestGateway: RuntimeReadinessRequester,
|
||||
options: RuntimeReadinessOptions = {}
|
||||
): Promise<RuntimeReadinessResult> {
|
||||
const signals = await fetchRuntimeReadinessSignals(requestGateway)
|
||||
const signals = await fetchRuntimeReadinessSignals(requestGateway, options.requestedProvider)
|
||||
|
||||
return interpretRuntimeReadiness(signals, options)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -308,7 +308,7 @@ describe('OAuth onboarding', () => {
|
|||
throw new Error(`unexpected api path: ${path}`)
|
||||
})
|
||||
|
||||
const requestGateway: OnboardingContext['requestGateway'] = async method => {
|
||||
const requestGateway: OnboardingContext['requestGateway'] = async (method, params) => {
|
||||
if (method === 'reload.env') {
|
||||
return {} as never
|
||||
}
|
||||
|
|
@ -318,6 +318,8 @@ describe('OAuth onboarding', () => {
|
|||
}
|
||||
|
||||
if (method === 'setup.runtime_check') {
|
||||
expect(params).toEqual({ provider: 'nous' })
|
||||
|
||||
return { ok: true } as never
|
||||
}
|
||||
|
||||
|
|
@ -355,6 +357,14 @@ describe('OAuth onboarding', () => {
|
|||
}
|
||||
|
||||
expect(calls.some(c => c.path === '/api/model/set')).toBe(true)
|
||||
|
||||
const optionsIndex = calls.findIndex(c => c.path === '/api/model/options')
|
||||
const recommendedIndex = calls.findIndex(c => c.path.startsWith('/api/model/recommended-default'))
|
||||
const setIndex = calls.findIndex(c => c.path === '/api/model/set')
|
||||
|
||||
expect(optionsIndex).toBeGreaterThanOrEqual(0)
|
||||
expect(recommendedIndex).toBeGreaterThan(optionsIndex)
|
||||
expect(setIndex).toBeGreaterThan(recommendedIndex)
|
||||
})
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -181,9 +181,13 @@ function clearPoll() {
|
|||
}
|
||||
}
|
||||
|
||||
async function checkRuntime(ctx: OnboardingContext): Promise<RuntimeReadinessResult> {
|
||||
async function checkRuntime(
|
||||
ctx: OnboardingContext,
|
||||
requestedProvider?: string
|
||||
): Promise<RuntimeReadinessResult> {
|
||||
return evaluateRuntimeReadiness(ctx.requestGateway, {
|
||||
defaultReason: DEFAULT_ONBOARDING_REASON,
|
||||
requestedProvider,
|
||||
unknownReady: false
|
||||
})
|
||||
}
|
||||
|
|
@ -317,7 +321,28 @@ async function completeWithModelConfirm(
|
|||
ignoreRuntimeGate = false
|
||||
) {
|
||||
await ctx.requestGateway('reload.env').catch(() => undefined)
|
||||
const runtime = await checkRuntime(ctx)
|
||||
|
||||
const defaults = await fetchProviderDefaultModel(preferredSlugs)
|
||||
|
||||
if (defaults) {
|
||||
// Persist the chosen provider/model before the runtime gate so a stale
|
||||
// config provider (e.g. anthropic from a prior failed setup) cannot make
|
||||
// setup.runtime_check validate the wrong backend after a fresh OAuth login.
|
||||
try {
|
||||
const res = await setModelAssignment({
|
||||
scope: 'main',
|
||||
provider: defaults.providerSlug,
|
||||
model: defaults.defaultModel
|
||||
})
|
||||
|
||||
notifyGatewayTools(res.gateway_tools)
|
||||
} catch {
|
||||
// Persistence failed — still run the scoped runtime check below and
|
||||
// show the confirm card so the user can pick something explicitly.
|
||||
}
|
||||
}
|
||||
|
||||
const runtime = await checkRuntime(ctx, preferredSlugs[0])
|
||||
|
||||
if (!runtime.ready && !ignoreRuntimeGate) {
|
||||
onFail(runtime.reason)
|
||||
|
|
@ -325,8 +350,6 @@ async function completeWithModelConfirm(
|
|||
return
|
||||
}
|
||||
|
||||
const defaults = await fetchProviderDefaultModel(preferredSlugs)
|
||||
|
||||
if (!defaults) {
|
||||
// Couldn't get a sensible default — proceed without confirm step.
|
||||
notifyReady(providerLabel)
|
||||
|
|
@ -336,27 +359,6 @@ async function completeWithModelConfirm(
|
|||
return
|
||||
}
|
||||
|
||||
// Persist the default model BEFORE showing the confirm card so that:
|
||||
// (1) "current default: X" shown in the UI is what's actually written
|
||||
// to config — no lying.
|
||||
// (2) If the user clicks "Start chatting" without changing anything,
|
||||
// no extra write is needed.
|
||||
// (3) If they bail out (e.g., refresh the page), they still end up
|
||||
// with a working config, not an empty-model fallback.
|
||||
try {
|
||||
const res = await setModelAssignment({
|
||||
scope: 'main',
|
||||
provider: defaults.providerSlug,
|
||||
model: defaults.defaultModel
|
||||
})
|
||||
|
||||
notifyGatewayTools(res.gateway_tools)
|
||||
} catch {
|
||||
// Persistence failed — still show the confirm card so the user can
|
||||
// pick something explicitly. The backend will pick its own default
|
||||
// at chat time if we end up never persisting.
|
||||
}
|
||||
|
||||
setFlow({
|
||||
status: 'confirming_model',
|
||||
providerSlug: defaults.providerSlug,
|
||||
|
|
|
|||
|
|
@ -2957,6 +2957,39 @@ def test_setup_runtime_check_rejects_implicit_bedrock_when_unconfigured(monkeypa
|
|||
assert resp["result"]["provider"] == "bedrock"
|
||||
|
||||
|
||||
def test_setup_runtime_check_honors_requested_provider(monkeypatch):
|
||||
"""Onboarding must be able to validate the provider the user just connected."""
|
||||
monkeypatch.setattr("hermes_cli.main._has_any_provider_configured", lambda: True)
|
||||
|
||||
def fake_resolve(requested=None, **kwargs):
|
||||
if requested == "nous":
|
||||
return {
|
||||
"provider": "nous",
|
||||
"api_key": "invoke-jwt",
|
||||
"source": "portal",
|
||||
}
|
||||
return {
|
||||
"provider": "anthropic",
|
||||
"api_key": "",
|
||||
"source": "config",
|
||||
}
|
||||
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.runtime_provider.resolve_runtime_provider",
|
||||
fake_resolve,
|
||||
)
|
||||
|
||||
scoped = server.handle_request(
|
||||
{"id": "1", "method": "setup.runtime_check", "params": {"provider": "nous"}}
|
||||
)
|
||||
assert scoped["result"]["ok"] is True
|
||||
assert scoped["result"]["provider"] == "nous"
|
||||
|
||||
default = server.handle_request({"id": "1", "method": "setup.runtime_check", "params": {}})
|
||||
assert default["result"]["ok"] is False
|
||||
assert default["result"]["provider"] == "anthropic"
|
||||
|
||||
|
||||
def test_complete_slash_drops_removed_provider_alias():
|
||||
# `/provider` was folded into a single `/model` command, so autocomplete
|
||||
# must no longer offer the dead alias...
|
||||
|
|
|
|||
|
|
@ -9028,7 +9028,8 @@ def _(rid, params: dict) -> dict:
|
|||
from hermes_cli.auth import has_usable_secret
|
||||
from hermes_cli.main import _has_any_provider_configured
|
||||
|
||||
runtime = resolve_runtime_provider(requested=None)
|
||||
requested = str(params.get("provider") or "").strip() or None
|
||||
runtime = resolve_runtime_provider(requested=requested)
|
||||
provider_configured = bool(_has_any_provider_configured())
|
||||
provider = runtime.get("provider") or "provider"
|
||||
source = str(runtime.get("source") or "")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue