diff --git a/src/lib/actions/sandbox/rebuild.ts b/src/lib/actions/sandbox/rebuild.ts index 5fca56ce188..d95c2e5e301 100644 --- a/src/lib/actions/sandbox/rebuild.ts +++ b/src/lib/actions/sandbox/rebuild.ts @@ -23,9 +23,21 @@ const hermesProviderAuth = require("../../hermes-provider-auth") as { baseUrl?: string, ) => void; }; -const { LOCAL_INFERENCE_PROVIDERS, REMOTE_PROVIDER_CONFIG } = require("../../onboard/providers") as { +type ProviderLookup = + | { kind: "exists" } + | { kind: "missing" } + | { kind: "lookup_failed"; message: string }; + +const { + LOCAL_INFERENCE_PROVIDERS, + REMOTE_PROVIDER_CONFIG, + lookupProviderInGateway, + isMutableEndpointProvider, +} = require("../../onboard/providers") as { LOCAL_INFERENCE_PROVIDERS: string[]; REMOTE_PROVIDER_CONFIG: Record; + lookupProviderInGateway: (name: string, runOpenshellFn: typeof runOpenshell) => ProviderLookup; + isMutableEndpointProvider: (name: string) => boolean; }; import { @@ -363,20 +375,50 @@ export async function rebuildSandbox( `Preflight credential check: ${rebuildCredentialEnv} → ${credentialValue ? "present" : "MISSING"}`, ); if (!credentialValue) { - console.error(""); - console.error(` ${_RD}Rebuild preflight failed:${R} provider credential not found.`); - console.error(` The non-interactive recreate step requires ${rebuildCredentialEnv},`); - console.error(" but it is not set in the environment."); - console.error(""); - console.error(" To fix, do one of:"); - console.error(` export ${rebuildCredentialEnv}=`); - console.error(` ${CLI_NAME} onboard # re-enter the key interactively`); - console.error(""); - console.error(" Sandbox is untouched — no data was lost."); - bail(`Missing credential: ${rebuildCredentialEnv}`); - return; + const gatewayProviderName = rebuildProvider || null; + const skipGatewayFallback = + Boolean(gatewayProviderName) && isMutableEndpointProvider(gatewayProviderName as string); + const lookup: ProviderLookup = + gatewayProviderName && !skipGatewayFallback + ? lookupProviderInGateway(gatewayProviderName, runOpenshell) + : { kind: "missing" }; + log( + `Preflight credential check: gateway provider '${gatewayProviderName || "(none)"}' → ${lookup.kind}${skipGatewayFallback ? " (skipped, mutable endpoint)" : ""}`, + ); + if (lookup.kind === "exists") { + console.log( + ` ${D}Note. '${gatewayProviderName}' is already registered in the OpenShell gateway. ` + + `Skipping host env credential check, the gateway will reuse the stored credential. ` + + `To rotate, export ${rebuildCredentialEnv}= before rebuild.${R}`, + ); + rebuildCredentialEnv = null; + } else if (lookup.kind === "lookup_failed") { + console.error(""); + console.error(` ${_RD}Rebuild preflight failed.${R} OpenShell gateway lookup for provider '${gatewayProviderName}' returned an error.`); + console.error(` ${lookup.message}`); + console.error(" This is likely a gateway connectivity or RPC issue, not a missing credential."); + console.error(" Check OpenShell status with \`openshell status\` and retry."); + console.error(""); + console.error(" Sandbox is untouched, no data was lost."); + bail(`Gateway lookup failed for ${gatewayProviderName}`); + return; + } else { + console.error(""); + console.error(` ${_RD}Rebuild preflight failed.${R} provider credential not found.`); + console.error(` The non-interactive recreate step requires ${rebuildCredentialEnv},`); + console.error(" but it is not set in the environment."); + console.error(""); + console.error(" To fix, do one of:"); + console.error(` export ${rebuildCredentialEnv}=`); + console.error(` ${CLI_NAME} onboard # re-enter the key interactively`); + console.error(""); + console.error(" Sandbox is untouched, no data was lost."); + bail(`Missing credential: ${rebuildCredentialEnv}`); + return; + } } - } else { + } + if (!rebuildCredentialEnv) { // No credentialEnv in session — local inference (Ollama/vLLM) or // session was lost. Either way, skip the credential preflight; // onboard will handle it. diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index bc231df3a59..ad444b07759 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -16,6 +16,9 @@ const { setOnboardBrandingAgent, }: typeof import("./onboard/branding") = require("./onboard/branding"); const { cleanupTempDir }: typeof import("./onboard/temp-files") = require("./onboard/temp-files"); +const { + reuseGatewayOrUpsertInferenceProvider, +}: typeof import("./onboard/inference-provider-upsert") = require("./onboard/inference-provider-upsert"); const { stopStaleDashboardListenersForSandbox } = require("./onboard/stale-gateway-cleanup"); const { looksLikeForwardPortConflict, runBackgroundForwardStartWithPortReleaseRetries }: typeof import("./onboard/forward-start") = require("./onboard/forward-start"); const { @@ -7503,32 +7506,29 @@ async function setupInference( resolvedCredentialEnv && credentialValue ? { [resolvedCredentialEnv]: credentialValue } : {}; - const providerResult = upsertProvider( - provider, - config.providerType, - resolvedCredentialEnv, - resolvedEndpointUrl, - env, + const upsertOutcome = await reuseGatewayOrUpsertInferenceProvider( + { + provider, + providerType: config.providerType, + credentialEnv: resolvedCredentialEnv, + endpointUrl: resolvedEndpointUrl, + env, + credentialValue, + label: config.label, + helpUrl: config.helpUrl, + }, + { + upsertProvider, + providerExistsInGateway, + isMutableEndpointProvider: onboardProviders.isMutableEndpointProvider, + isNonInteractive, + promptValidationRecovery, + classifyApplyFailure, + }, ); - if (!providerResult.ok) { - console.error(` ${providerResult.message}`); - if (isNonInteractive()) { - process.exit(providerResult.status || 1); - } - const retry = await promptValidationRecovery( - config.label, - classifyApplyFailure(providerResult.message), - resolvedCredentialEnv, - config.helpUrl, - ); - if (retry === "credential" || retry === "retry") { - continue; - } - if (retry === "selection" || retry === "model") { - return { retry: "selection" }; - } - process.exit(providerResult.status || 1); - } + if (upsertOutcome.kind === "exit") process.exit(upsertOutcome.code); + if (upsertOutcome.kind === "retry") continue; + if (upsertOutcome.kind === "selection") return { retry: "selection" }; const args = ["inference", "set"]; if (config.skipVerify) { args.push("--no-verify"); diff --git a/src/lib/onboard/inference-provider-upsert.ts b/src/lib/onboard/inference-provider-upsert.ts new file mode 100644 index 00000000000..a769eb683db --- /dev/null +++ b/src/lib/onboard/inference-provider-upsert.ts @@ -0,0 +1,88 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +// Issue 3895. Sandbox rebuild calls back into onboard --resume after the +// host shell has been closed, so NVIDIA_API_KEY and friends are no longer +// in process.env even though the original onboard registered the provider +// in the OpenShell gateway. Re-running upsertProvider with an empty env +// would overwrite the gateway-stored credential, so this helper short +// circuits the upsert when the gateway already holds the credential and +// the host has nothing to contribute. + +import type { ProbeRecovery } from "../validation-recovery"; +import type { ValidationClassification } from "../validation"; + +export type UpsertResult = { ok: boolean; status?: number; message?: string }; + +export type UpsertOutcome = + | { kind: "ok" } + | { kind: "retry" } + | { kind: "selection" } + | { kind: "exit"; code: number }; + +export type RecoveryChoice = "credential" | "retry" | "selection" | "model" | string; + +export interface InferenceProviderUpsertSpec { + provider: string; + providerType: string; + credentialEnv: string; + endpointUrl: string | null; + env: Record; + credentialValue: string | null; + label: string; + helpUrl: string | null; +} + +export interface InferenceProviderUpsertHandlers { + upsertProvider: ( + name: string, + type: string, + credentialEnv: string, + baseUrl: string | null, + env: Record, + ) => UpsertResult; + providerExistsInGateway: (name: string) => boolean; + isMutableEndpointProvider: (name: string) => boolean; + isNonInteractive: () => boolean; + promptValidationRecovery: ( + label: string, + classification: ProbeRecovery, + credentialEnv: string, + helpUrl: string | null, + ) => Promise; + classifyApplyFailure: (message: string) => ValidationClassification; +} + +export async function reuseGatewayOrUpsertInferenceProvider( + spec: InferenceProviderUpsertSpec, + handlers: InferenceProviderUpsertHandlers, +): Promise { + if ( + !spec.credentialValue && + !handlers.isMutableEndpointProvider(spec.provider) && + handlers.providerExistsInGateway(spec.provider) + ) { + return { kind: "ok" }; + } + const result = handlers.upsertProvider( + spec.provider, + spec.providerType, + spec.credentialEnv, + spec.endpointUrl, + spec.env, + ); + if (result.ok) return { kind: "ok" }; + console.error(` ${result.message}`); + if (handlers.isNonInteractive()) { + return { kind: "exit", code: result.status || 1 }; + } + const retry = await handlers.promptValidationRecovery( + spec.label, + handlers.classifyApplyFailure(result.message ?? ""), + spec.credentialEnv, + spec.helpUrl, + ); + if (retry === "credential" || retry === "retry") return { kind: "retry" }; + if (retry === "selection" || retry === "model") return { kind: "selection" }; + return { kind: "exit", code: result.status || 1 }; +} diff --git a/src/lib/onboard/providers.ts b/src/lib/onboard/providers.ts index 46748382fa1..db51acea495 100644 --- a/src/lib/onboard/providers.ts +++ b/src/lib/onboard/providers.ts @@ -267,6 +267,39 @@ function providerExistsInGateway(name, _runOpenshell) { return result.status === 0; } +const MUTABLE_ENDPOINT_PROVIDERS = new Set([ + "compatible-endpoint", + "compatible-anthropic-endpoint", +]); + +function isMutableEndpointProvider(name) { + return MUTABLE_ENDPOINT_PROVIDERS.has(name); +} + +function lookupProviderInGateway(name, _runOpenshell) { + const result = _runOpenshell(["provider", "get", name], { + ignoreError: true, + stdio: ["ignore", "pipe", "pipe"], + }); + if (result.status === 0) { + return { kind: "exists" }; + } + const stderr = String(result.stderr || "").trim(); + const stdout = String(result.stdout || "").trim(); + const haystack = `${stderr} ${stdout}`.toLowerCase(); + const looksLikeNotFound = + /not found|does not exist|no such provider|unknown provider/.test(haystack) || + (result.status === 1 && !stderr && !stdout); + if (looksLikeNotFound) { + return { kind: "missing" }; + } + const message = + compactText(redact(stderr)) || + compactText(redact(stdout)) || + `openshell provider get exited with status ${result.status}`; + return { kind: "lookup_failed", message }; +} + /** * Create or update an OpenShell provider in the gateway. * @@ -337,6 +370,8 @@ module.exports = { buildProviderArgs, upsertProvider, providerExistsInGateway, + lookupProviderInGateway, + isMutableEndpointProvider, upsertMessagingProviders, getSandboxInferenceConfig, }; diff --git a/test/rebuild-credential-preflight.test.ts b/test/rebuild-credential-preflight.test.ts index b2a71747b07..1569716607d 100644 --- a/test/rebuild-credential-preflight.test.ts +++ b/test/rebuild-credential-preflight.test.ts @@ -54,6 +54,7 @@ function createFixture(opts: { providerCredentialHashes?: Record; dockerBuildExitCode?: number; providerRegistered?: boolean; + providerLookupErrors?: boolean; }) { const { sandboxName = "my-assistant", @@ -67,6 +68,7 @@ function createFixture(opts: { providerCredentialHashes, dockerBuildExitCode = 0, providerRegistered = true, + providerLookupErrors = false, } = opts; const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-2273-")); tmpFixtures.push(tmpDir); @@ -212,7 +214,7 @@ if (a[0]==="gateway" && a[1]==="info") { process.stdout.write("nemoclaw\\n if (a[0]==="gateway" && a[1]==="select") { process.exit(0); } if (a[0]==="inference" && a[1]==="get") { process.stdout.write('{"provider":"${provider}","model":"meta/llama-3.3-70b-instruct"}\\n'); process.exit(0); } if (a[0]==="inference" && a[1]==="set") { process.exit(0); } -if (a[0]==="provider" && a[1]==="get") { process.exit(${providerRegistered ? 0 : 1}); } +if (a[0]==="provider" && a[1]==="get") { ${providerLookupErrors ? "process.stderr.write('openshell: connection refused\\\\n'); process.exit(2);" : `process.exit(${providerRegistered ? 0 : 1});`} } if (a[0]==="provider") { process.exit(0); } if (a[0]==="forward") { process.exit(0); } process.exit(0); @@ -314,13 +316,15 @@ function registryHasSandbox(fixture: ReturnType): boolean describe("Issue #2273: atomic rebuild", () => { describe("Layer 2: preflight credential check", () => { it( - "aborts rebuild BEFORE destroying sandbox when credential is missing", + "aborts rebuild BEFORE destroying sandbox when credential is missing and gateway has no provider", { timeout: 60_000 }, () => { - // No credential in env or credentials.json + // No credential in env or credentials.json AND no gateway provider — + // the only configuration that should still bail preflight after #3895. const f = createFixture({ credentialEnv: "NVIDIA_API_KEY", // no savedCredential + providerRegistered: false, }); const result = runRebuild(f); @@ -336,6 +340,32 @@ describe("Issue #2273: atomic rebuild", () => { }, ); + it( + "proceeds when credential is missing in env but provider is already registered in OpenShell (#3895)", + { timeout: 60_000 }, + () => { + // Regression for #3895: a sandbox onboarded with NVIDIA Endpoints + // and later modified (e.g. channel add → auto-rebuild) used to fail + // preflight when NVIDIA_API_KEY was no longer in the operator's + // shell, even though the credential was already stored in the + // OpenShell gateway. Rebuild now reuses the gateway-stored + // credential and proceeds without re-prompting. + const f = createFixture({ + credentialEnv: "NVIDIA_API_KEY", + // no savedCredential, no env var + providerRegistered: true, + }); + + const result = runRebuild(f); + const output = (result.stderr || "") + (result.stdout || ""); + + expect(output).not.toContain("preflight failed"); + expect(output).not.toContain("Missing credential: NVIDIA_API_KEY"); + expect(output).toContain("already registered in the OpenShell gateway"); + expect(output).toContain("Backing up sandbox state"); + }, + ); + it( "proceeds when credential is saved in credentials.json (not in env)", { timeout: 60_000 }, @@ -471,14 +501,15 @@ describe("Issue #2273: atomic rebuild", () => { ); it( - "preflight works for non-NVIDIA providers (OpenAI, Anthropic, etc.)", + "preflight aborts for non-NVIDIA providers when both env var and gateway provider are missing", { timeout: 60_000 }, () => { - // OpenAI provider with no credential — should abort + // OpenAI provider with no credential anywhere — should abort. const f = createFixture({ provider: "openai-api", credentialEnv: "OPENAI_API_KEY", // no savedCredential + providerRegistered: false, }); const result = runRebuild(f); @@ -491,6 +522,33 @@ describe("Issue #2273: atomic rebuild", () => { }, ); + it.each([ + ["openai-api", "OPENAI_API_KEY"], + ["anthropic-prod", "ANTHROPIC_API_KEY"], + ["gemini-api", "GEMINI_API_KEY"], + ])( + "reuses the OpenShell gateway provider for %s when env var is missing (#3895)", + (provider, credentialEnv) => { + // #3895 fix extends gateway credential reuse to every remote provider + // that registers an OpenShell provider during onboard, not just NVIDIA. + const f = createFixture({ + provider, + credentialEnv, + providerRegistered: true, + // no savedCredential + }); + + const result = runRebuild(f); + const output = (result.stderr || "") + (result.stdout || ""); + + expect(output).not.toContain("preflight failed"); + expect(output).not.toContain(`Missing credential: ${credentialEnv}`); + expect(output).toContain("already registered in the OpenShell gateway"); + expect(output).toContain("Backing up sandbox state"); + }, + 60_000, + ); + it( "uses the registered Hermes Provider in OpenShell instead of requiring OPENAI_API_KEY", { timeout: 60_000 }, @@ -532,6 +590,47 @@ describe("Issue #2273: atomic rebuild", () => { }, ); + it( + "surfaces a gateway connectivity error when provider lookup fails (#3918 review)", + { timeout: 60_000 }, + () => { + const f = createFixture({ + credentialEnv: "NVIDIA_API_KEY", + providerLookupErrors: true, + }); + + const result = runRebuild(f); + const output = (result.stderr || "") + (result.stdout || ""); + + expect(result.status).not.toBe(0); + expect(output).toContain("gateway lookup"); + expect(output).toContain("openshell status"); + expect(output).not.toContain("Missing credential: NVIDIA_API_KEY"); + expect(registryHasSandbox(f)).toBe(true); + }, + ); + + it( + "does not reuse the gateway credential for mutable-endpoint providers (#3918 review)", + { timeout: 60_000 }, + () => { + const f = createFixture({ + provider: "compatible-endpoint", + credentialEnv: "COMPATIBLE_API_KEY", + providerRegistered: true, + }); + + const result = runRebuild(f); + const output = (result.stderr || "") + (result.stdout || ""); + + expect(result.status).not.toBe(0); + expect(output).toContain("preflight failed"); + expect(output).toContain("COMPATIBLE_API_KEY"); + expect(output).not.toContain("already registered in the OpenShell gateway"); + expect(registryHasSandbox(f)).toBe(true); + }, + ); + it( "aborts Hermes OAuth rebuild before backup when the OpenShell provider is missing", { timeout: 60_000 }, @@ -595,14 +694,17 @@ describe("Issue #2273: atomic rebuild", () => { ); it( - "preflight failure exits non-zero when credential is missing", + "preflight failure exits non-zero when credential is missing and gateway has no provider", { timeout: 60_000 }, () => { // Verifies that missing credentials cause rebuild to exit non-zero. // This is the observable CLI behavior — the preflight check fails // and bail() calls process.exit with a non-zero code. + // After #3895 the gateway-registered provider is reused if present, + // so this assertion only holds when the gateway also has nothing. const f = createFixture({ credentialEnv: "NVIDIA_API_KEY", + providerRegistered: false, // No credential — preflight will fail and exit non-zero });