-
Notifications
You must be signed in to change notification settings - Fork 3.1k
refactor(onboard): extract inference provider flows into modules (#767) #4774
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
24e9066
f3d6433
a645daf
aa85ddc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,138 @@ | ||
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
| // | ||
| // Hermes Provider inference setup flow. | ||
| // Extracted verbatim from onboard.setupInference (#767). | ||
|
|
||
| import type { HermesAuthMethod } from "../hermes-auth"; | ||
| import type { HermesDeps, SetupInferenceResult } from "./types"; | ||
|
|
||
| export async function setupHermesProviderInference( | ||
| args: { | ||
| sandboxName: string | null; | ||
| model: string; | ||
| provider: string; | ||
| endpointUrl: string | null; | ||
| credentialEnv: string | null; | ||
| hermesAuthMethod: HermesAuthMethod | string | null; | ||
| hermesToolGateways: string[]; | ||
| }, | ||
| deps: HermesDeps, | ||
| ): Promise<SetupInferenceResult> { | ||
| const { | ||
| sandboxName, | ||
| model, | ||
| provider, | ||
| endpointUrl, | ||
| credentialEnv, | ||
| hermesAuthMethod, | ||
| hermesToolGateways, | ||
| } = args; | ||
| const { | ||
| runOpenshell, | ||
| upsertProvider: _upsertProvider, // intentionally unused; matches inline branch | ||
| verifyInferenceRoute, | ||
| verifyOnboardInferenceSmoke, | ||
| isNonInteractive, | ||
| registry, | ||
| hermesProviderAuth, | ||
| getHermesToolGatewayBroker, | ||
| providerExistsInGateway, | ||
| normalizeHermesAuthMethod, | ||
| resolveHermesNousApiKey, | ||
| checkHermesProviderStoreReachable, | ||
| hermesAuthMethodLabel, | ||
| hermesConstants: { | ||
| HERMES_NOUS_API_KEY_CREDENTIAL_ENV, | ||
| HERMES_AUTH_METHOD_API_KEY, | ||
| HERMES_AUTH_METHOD_OAUTH, | ||
| }, | ||
| requireValue, | ||
| redact, | ||
| compactText, | ||
| } = deps; | ||
| void _upsertProvider; | ||
|
|
||
| const targetSandbox = requireValue(sandboxName, "Hermes Provider requires a sandbox name"); | ||
| const resolvedHermesAuthMethod = | ||
| normalizeHermesAuthMethod(hermesAuthMethod) || | ||
| (credentialEnv === HERMES_NOUS_API_KEY_CREDENTIAL_ENV | ||
| ? HERMES_AUTH_METHOD_API_KEY | ||
| : HERMES_AUTH_METHOD_OAUTH); | ||
| const providerStore = checkHermesProviderStoreReachable(runOpenshell); | ||
| if (!providerStore.ok) { | ||
| console.error(" ✗ OpenShell provider storage is unreachable."); | ||
| console.error(` ${providerStore.message}`); | ||
| console.error(" Restart or recreate the OpenShell gateway, then rerun onboarding."); | ||
| if (isNonInteractive()) process.exit(1); | ||
| return { retry: "selection" }; | ||
| } | ||
| const providerRegistered = hermesProviderAuth.isHermesProviderRegistered(runOpenshell); | ||
| const toolGatewayProviderRegistered = | ||
| hermesToolGateways.length === 0 | ||
| ? true | ||
| : providerExistsInGateway( | ||
| getHermesToolGatewayBroker().getHermesToolGatewayProviderName(targetSandbox), | ||
| ); | ||
| const hasFreshNousApiKey = | ||
| resolvedHermesAuthMethod === HERMES_AUTH_METHOD_API_KEY && !!resolveHermesNousApiKey(); | ||
| const shouldPrepareHermesCredentials = | ||
| !providerRegistered || | ||
| !toolGatewayProviderRegistered || | ||
| hasFreshNousApiKey || | ||
| (resolvedHermesAuthMethod === HERMES_AUTH_METHOD_OAUTH && !isNonInteractive()); | ||
| if (shouldPrepareHermesCredentials) { | ||
| try { | ||
| const state = | ||
| resolvedHermesAuthMethod === HERMES_AUTH_METHOD_API_KEY | ||
| ? await hermesProviderAuth.ensureHermesProviderApiKeyCredentials(targetSandbox, { | ||
| apiKey: resolveHermesNousApiKey(), | ||
| runOpenshell, | ||
| baseUrl: endpointUrl || undefined, | ||
| }) | ||
| : await hermesProviderAuth.ensureHermesProviderOAuthCredentials(targetSandbox, { | ||
| allowInteractiveLogin: !isNonInteractive(), | ||
| runOpenshell, | ||
| baseUrl: endpointUrl || undefined, | ||
| toolGatewayPresets: hermesToolGateways, | ||
| }); | ||
| if (!state) { | ||
| const authLabel = hermesAuthMethodLabel(resolvedHermesAuthMethod); | ||
| console.error(` ✗ Hermes Provider ${authLabel} is not available on the host.`); | ||
| console.error( | ||
| " Re-run `nemoclaw onboard --agent hermes` interactively to configure credentials.", | ||
| ); | ||
| process.exit(1); | ||
| } | ||
| } catch (err) { | ||
| console.error( | ||
| ` ✗ Failed to prepare Hermes Provider credentials: ${ | ||
| err instanceof Error ? err.message : String(err) | ||
| }`, | ||
| ); | ||
| if (isNonInteractive()) process.exit(1); | ||
| return { retry: "selection" }; | ||
| } | ||
| } | ||
|
|
||
| const applyResult = runOpenshell( | ||
| ["inference", "set", "--no-verify", "--provider", provider, "--model", model], | ||
| { ignoreError: true }, | ||
| ); | ||
| if (applyResult.status !== 0) { | ||
| const message = | ||
| compactText(redact(`${applyResult.stderr || ""} ${applyResult.stdout || ""}`)) || | ||
| `Failed to configure inference provider '${provider}'.`; | ||
| console.error(` ${message}`); | ||
| if (isNonInteractive()) process.exit(applyResult.status || 1); | ||
| return { retry: "selection" }; | ||
| } | ||
|
|
||
| verifyInferenceRoute(provider, model); | ||
| verifyOnboardInferenceSmoke({ provider, model, endpointUrl, credentialEnv }); | ||
| if (sandboxName) { | ||
| registry.updateSandbox(sandboxName, { model, provider }); | ||
| } | ||
| console.log(` ✓ Inference route set: ${provider} / ${model}`); | ||
| return { ok: true }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
| // | ||
| // Inference provider setup modules. | ||
| // | ||
| // `setupInference` in `src/lib/onboard.ts` is the orchestrator: it owns the | ||
| // step banner, the shared verify + registry-update finalization, and the | ||
| // "unsupported provider" error path. Each provider-specific branch lives in | ||
| // its own module here so the flows can be read, reviewed, and tested in | ||
| // isolation. See issue #767 for the broader provider extraction plan. | ||
|
|
||
| export { setupHermesProviderInference } from "./hermes"; | ||
| export { setupOllamaLocalInference } from "./ollama-local"; | ||
| export { setupRemoteProviderInference } from "./remote"; | ||
| export { setupRoutedInference } from "./routed"; | ||
| export { setupVllmLocalInference } from "./vllm-local"; | ||
| export { | ||
| isRemoteProviderName, | ||
| REMOTE_PROVIDER_NAMES, | ||
| } from "./types"; | ||
| export type { | ||
| CommonDeps, | ||
| HermesDeps, | ||
| OllamaDeps, | ||
| RemoteProviderDeps, | ||
| RemoteProviderName, | ||
| RoutedDeps, | ||
| SetupInferenceResult, | ||
| VllmDeps, | ||
| } from "./types"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
| // | ||
| // Ollama local inference provider setup flow. | ||
| // Extracted verbatim from onboard.setupInference (#767). | ||
|
|
||
| import type { | ||
| OllamaDeps, | ||
| SetupInferenceResult, | ||
| } from "./types"; | ||
|
|
||
| export async function setupOllamaLocalInference( | ||
| args: { model: string; provider: string; allowToolsIncompatible: boolean }, | ||
| deps: OllamaDeps, | ||
| ): Promise<{ done: true; result: SetupInferenceResult } | { done: false }> { | ||
| const { model, provider, allowToolsIncompatible } = args; | ||
| const { | ||
| upsertProvider, | ||
| validateLocalProvider, | ||
| getLocalProviderBaseUrl, | ||
| applyLocalInferenceRoute, | ||
| getOllamaWarmupCommand, | ||
| run, | ||
| shouldFrontOllamaWithProxy, | ||
| ensureOllamaAuthProxy, | ||
| isProxyHealthy, | ||
| getOllamaProxyToken, | ||
| persistAndProbeOllamaProxy, | ||
| localInference, | ||
| OLLAMA_PROXY_CREDENTIAL_ENV, | ||
| } = deps; | ||
|
|
||
| const validation = validateLocalProvider(provider); | ||
| let proxyReady = false; | ||
| const frontOllamaWithProxy = shouldFrontOllamaWithProxy(); | ||
| if (!validation.ok) { | ||
| // The container reachability check uses Docker's --add-host host-gateway, | ||
| // which may not work on all Docker configurations (e.g., Brev, rootless). | ||
| // The real sandbox uses k3s CoreDNS + NodeHosts — a different path. | ||
| // Try to start/restart the auth proxy before probing — this recovers | ||
| // from stale or missing proxy processes before we decide to abort. | ||
| if (frontOllamaWithProxy) { | ||
| ensureOllamaAuthProxy(); | ||
| proxyReady = isProxyHealthy(); | ||
| } | ||
| if (proxyReady) { | ||
| console.warn(` ⚠ ${validation.message}`); | ||
| if (validation.diagnostic) { | ||
| console.warn(` Diagnostic: ${validation.diagnostic}`); | ||
| } | ||
| console.warn( | ||
| " The auth proxy is healthy on the host — continuing. " + | ||
| "The sandbox uses a different network path and may work correctly.", | ||
| ); | ||
| } else { | ||
| console.error(` ${validation.message}`); | ||
| if (validation.diagnostic) { | ||
| console.error(` Diagnostic: ${validation.diagnostic}`); | ||
| } | ||
| if (process.platform === "darwin") { | ||
| console.error( | ||
| " On macOS, local inference also depends on OpenShell host routing support.", | ||
| ); | ||
| } | ||
| process.exit(1); | ||
| } | ||
| } | ||
| const baseUrl = getLocalProviderBaseUrl(provider); | ||
| let ollamaCredential = "ollama"; | ||
| if (frontOllamaWithProxy) { | ||
| // Skip if already started during the fallback recovery above. | ||
| if (!proxyReady) ensureOllamaAuthProxy(); | ||
| const proxyToken = getOllamaProxyToken(); | ||
| if (!proxyToken) { | ||
| console.error( | ||
| " Ollama auth proxy token is not set. Re-run onboard to initialize the proxy.", | ||
| ); | ||
| process.exit(1); | ||
| } | ||
| ollamaCredential = proxyToken; | ||
| // Persist token now that ollama-local is confirmed as the provider. | ||
| // Not persisted earlier in case the user backs out to a different provider. | ||
| await persistAndProbeOllamaProxy(proxyToken); | ||
|
Comment on lines
+81
to
+83
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move proxy-token persistence after the back-out/failure paths. This runs before both 💡 Suggested change- let ollamaCredential = "ollama";
+ let ollamaCredential = "ollama";
+ let proxyToken: string | null | undefined;
if (frontOllamaWithProxy) {
// Skip if already started during the fallback recovery above.
if (!proxyReady) ensureOllamaAuthProxy();
- const proxyToken = getOllamaProxyToken();
+ proxyToken = getOllamaProxyToken();
if (!proxyToken) {
console.error(
" Ollama auth proxy token is not set. Re-run onboard to initialize the proxy.",
);
process.exit(1);
}
ollamaCredential = proxyToken;
- // Persist token now that ollama-local is confirmed as the provider.
- // Not persisted earlier in case the user backs out to a different provider.
- await persistAndProbeOllamaProxy(proxyToken);
}
@@
if (await applyLocalInferenceRoute("ollama-local", model)) {
return { done: true, result: { retry: "selection" } };
}
+ if (proxyToken) {
+ await persistAndProbeOllamaProxy(proxyToken);
+ }🤖 Prompt for AI Agents |
||
| } | ||
| // Use a dedicated internal credential env (NEMOCLAW_OLLAMA_PROXY_TOKEN) | ||
| // so the gateway never reads the user's host OPENAI_API_KEY for local | ||
| // Ollama. GH #2519: a stale host OPENAI_API_KEY was leaking into the | ||
| // inference path and producing 401s. | ||
| const providerResult = upsertProvider( | ||
| "ollama-local", | ||
| "openai", | ||
| OLLAMA_PROXY_CREDENTIAL_ENV, | ||
| baseUrl, | ||
| { [OLLAMA_PROXY_CREDENTIAL_ENV]: ollamaCredential }, | ||
| ); | ||
| if (!providerResult.ok) { | ||
| console.error(` ${providerResult.message}`); | ||
| process.exit(providerResult.status || 1); | ||
| } | ||
| if (await applyLocalInferenceRoute("ollama-local", model)) { | ||
| return { done: true, result: { retry: "selection" } }; | ||
| } | ||
| console.log(` Priming Ollama model: ${model}`); | ||
| run(getOllamaWarmupCommand(model), { ignoreError: true }); | ||
| const probe = localInference.validateOllamaModelWithToolsOverride(model, allowToolsIncompatible); | ||
| if (!probe.ok) { | ||
| console.error(` ${probe.message}`); | ||
| process.exit(1); | ||
| } | ||
| // Do not mutate ~/.nemoclaw/credentials.json here: local Ollama now uses | ||
| // OLLAMA_PROXY_CREDENTIAL_ENV, so any saved OPENAI_API_KEY remains available | ||
| // to unrelated OpenAI-backed sandboxes. | ||
| return { done: false }; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Narrow the Hermes contract to require
sandboxName.This function can't actually handle
null: Line 56 immediately hard-fails ifsandboxNameis missing. Keeping the arg nullable weakens the provider interface and pushes a compile-time invariant into runtime. If Hermes always needs a sandbox, make that explicit in the type here (or via a provider-specific subtype before dispatch).♻️ Suggested shape
export async function setupHermesProviderInference( args: { - sandboxName: string | null; + sandboxName: string; model: string; provider: string; endpointUrl: string | null; credentialEnv: string | null; hermesAuthMethod: HermesAuthMethod | string | null; hermesToolGateways: string[]; }, deps: HermesDeps, ): Promise<SetupInferenceResult> { @@ - const targetSandbox = requireValue(sandboxName, "Hermes Provider requires a sandbox name"); + const targetSandbox = sandboxName;Also applies to: 56-56
🤖 Prompt for AI Agents