From 0adb132f8c2aba8ae3eb184a4f956530327a6119 Mon Sep 17 00:00:00 2001 From: Charan Jagwani Date: Fri, 22 May 2026 12:26:55 -0700 Subject: [PATCH 1/5] fix(onboard): re-prompt for credentials on resume after provider reset (#3278) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `nemoclaw onboard --resume` short-circuits the inference setup step on the recorded `provider`/`model`. After `nemoclaw credentials reset ` deletes the provider from the gateway, that short-circuit left the sandbox rebuild failing with an authentication error because the credential was missing and never re-prompted. Adds `ensureResumeProviderReady` (in `src/lib/onboard/resume-provider-recovery.ts`) which checks `providerExistsInGateway` during resume and, when the provider is gone: - If the credential env is still hydrated (e.g. `NVIDIA_API_KEY` exported in shell), prints a `[resume] recreating…` note and forces the inference setup step to re-run. - If the credential is missing in interactive mode, prompts for it via `replaceNamedCredential` then forces inference setup. - If the credential is missing in `--non-interactive` mode, prints a clear error pointing to the missing env and exits 1. The new module takes its onboard.ts dependencies via dependency injection so it stays unit-testable in isolation (6 tests cover each branch). `src/lib/onboard.ts` wires the deps once at module scope and passes the bound function into the resume loop. Supersedes #3285 (deepujain), which carries the same fix forward but is no longer mergeable because main has drifted (`mergeStateStatus=DIRTY`). The original module-level monkey-patching test pattern has been replaced with a deps-injection test against the extracted module. ⚠️ The `onboard-entrypoint-budget` CI check will fail on this PR (`src/lib/onboard.ts` grew by 14 net lines). The growth is the deps object literal plus the call-site wiring; the actual feature logic lives in `src/lib/onboard/resume-provider-recovery.ts`. Reducing the budget hit further would require extracting onboard.ts-resident helpers (`isRoutedInferenceProvider`, `replaceNamedCredential`, `note`, `isNonInteractive`) to submodules — out of scope for #3278. Reviewer to decide whether to bump the budget or chain a follow-up extraction. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Charan Jagwani --- src/lib/onboard.ts | 21 ++- .../onboard/resume-provider-recovery.test.ts | 129 ++++++++++++++++++ src/lib/onboard/resume-provider-recovery.ts | 123 +++++++++++++++++ 3 files changed, 270 insertions(+), 3 deletions(-) create mode 100644 src/lib/onboard/resume-provider-recovery.test.ts create mode 100644 src/lib/onboard/resume-provider-recovery.ts diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 0bd183d995..faa6ff3416 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -204,9 +204,12 @@ const { detectVllmProfile, installVllm } = require("./inference/vllm"); const inferenceConfig: typeof import("./inference/config") = require("./inference/config"); const { DEFAULT_CLOUD_MODEL, + DEFAULT_ROUTE_CREDENTIAL_ENV, getProviderSelectionConfig, parseGatewayInference, } = inferenceConfig; +const resumeProviderRecovery: typeof import("./onboard/resume-provider-recovery") = + require("./onboard/resume-provider-recovery"); const onboardProviders = require("./onboard/providers"); const hermesProviderAuth = require("./hermes-provider-auth"); @@ -1789,6 +1792,16 @@ function providerExistsInGateway(name: string) { return onboardProviders.providerExistsInGateway(name, runOpenshell); } +const resumeProviderRecoveryDeps: import("./onboard/resume-provider-recovery").ResumeProviderRecoveryDeps = { + remoteProviderConfig: REMOTE_PROVIDER_CONFIG, defaultRouteCredentialEnv: DEFAULT_ROUTE_CREDENTIAL_ENV, + isRoutedInferenceProvider, providerExistsInGateway, getProviderLabel, isNonInteractive, + hydrateCredentialEnv: (e) => hydrateCredentialEnv(e) ?? undefined, + log: (m) => console.log(m), warn: (m) => console.error(m), note, + exit: (c) => process.exit(c), replaceNamedCredential, validateNvidiaApiKeyValue, +}; +const ensureResumeProviderReady = (p: string | null | undefined, c: string | null | undefined) => + resumeProviderRecovery.ensureResumeProviderReady(p, c, resumeProviderRecoveryDeps); + function getMessagingChannelForEnvKey(envKey: string): string | null { if (envKey === "DISCORD_BOT_TOKEN") return "discord"; if (envKey === "SLACK_BOT_TOKEN") return "slack"; @@ -9525,6 +9538,7 @@ async function onboard(opts: OnboardOptions = {}): Promise { let nimContainer = session?.nimContainer || null; let webSearchConfig = session?.webSearchConfig || null; let forceProviderSelection = forceProviderSelectionForAgentChange; + let forceInferenceSetup = false; while (true) { const resumeProviderSelection = !forceProviderSelection && @@ -9533,13 +9547,13 @@ async function onboard(opts: OnboardOptions = {}): Promise { typeof provider === "string" && typeof model === "string"; if (resumeProviderSelection) { + ({ forceInferenceSetup } = await ensureResumeProviderReady(provider, credentialEnv)); skippedStepMessage("provider_selection", `${provider} / ${model}`); hydrateCredentialEnv(credentialEnv); - // #3342: resume short-circuits provider selection — repair the - // ollama-local systemd loopback override here so legacy 0.0.0.0 - // drop-ins from older NemoClaw versions get rewritten every resume. + // #3342: ollama-local systemd loopback drop-in repair on resume. repairLocalInferenceSystemdOverrideOrExit(provider, isNonInteractive); } else { + forceInferenceSetup = false; // #2753: do not persist sandboxName to onboard-session.json before // the sandbox actually exists in the gateway (Step 6 markStepComplete // below). A SIGINT between any earlier step and createSandbox would @@ -9581,6 +9595,7 @@ async function onboard(opts: OnboardOptions = {}): Promise { const resumeInference = !needsBedrockRuntimeAdapter && !forceProviderSelection && + !forceInferenceSetup && resume && isInferenceRouteReady(provider, model); if (resumeInference) { diff --git a/src/lib/onboard/resume-provider-recovery.test.ts b/src/lib/onboard/resume-provider-recovery.test.ts new file mode 100644 index 0000000000..43c8e3c923 --- /dev/null +++ b/src/lib/onboard/resume-provider-recovery.test.ts @@ -0,0 +1,129 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { + ensureResumeProviderReady, + type RemoteProviderConfigEntry, + type ResumeProviderRecoveryDeps, +} from "./resume-provider-recovery"; + +const COMPATIBLE_ENDPOINT_CONFIG: RemoteProviderConfigEntry = { + label: "Compatible Endpoint", + providerName: "compatible-endpoint", + providerType: "openai", + credentialEnv: "COMPATIBLE_API_KEY", + endpointUrl: "https://example/v1", + helpUrl: null, + modelMode: "input", + defaultModel: "test-model", +}; + +type DepsRecorder = { + log: string[]; + warn: string[]; + note: string[]; + exitCalls: number[]; + replaceCalls: Array<{ env: string; label: string }>; + deps: ResumeProviderRecoveryDeps; +}; + +function makeDeps(overrides: { + providerExists?: boolean; + credentialValue?: string | undefined; + nonInteractive?: boolean; + remoteProviderConfig?: Record; +}): DepsRecorder { + const log: string[] = []; + const warn: string[] = []; + const note: string[] = []; + const exitCalls: number[] = []; + const replaceCalls: Array<{ env: string; label: string }> = []; + const deps: ResumeProviderRecoveryDeps = { + remoteProviderConfig: overrides.remoteProviderConfig ?? { + compatible: COMPATIBLE_ENDPOINT_CONFIG, + }, + defaultRouteCredentialEnv: "OPENAI_API_KEY", + isRoutedInferenceProvider: () => false, + providerExistsInGateway: () => overrides.providerExists ?? true, + hydrateCredentialEnv: () => overrides.credentialValue, + getProviderLabel: (key) => key, + isNonInteractive: () => overrides.nonInteractive ?? false, + log: (m) => log.push(m), + warn: (m) => warn.push(m), + note: (m) => note.push(m), + exit: (code) => exitCalls.push(code), + replaceNamedCredential: async (env, label) => { + replaceCalls.push({ env, label }); + return "fresh-key"; + }, + validateNvidiaApiKeyValue: () => null, + }; + return { log, warn, note, exitCalls, replaceCalls, deps }; +} + +describe("ensureResumeProviderReady", () => { + it("returns false-forced when no provider is set (nothing to recover)", async () => { + const { deps } = makeDeps({ providerExists: false }); + const result = await ensureResumeProviderReady(null, null, deps); + expect(result.forceInferenceSetup).toBe(false); + }); + + it("returns false-forced when the provider is unknown and not a routed provider", async () => { + const { deps } = makeDeps({ providerExists: false }); + const result = await ensureResumeProviderReady("mystery-provider", null, deps); + expect(result.forceInferenceSetup).toBe(false); + }); + + it("returns false-forced when the provider still exists in the gateway", async () => { + const { deps } = makeDeps({ providerExists: true }); + const result = await ensureResumeProviderReady("compatible-endpoint", "COMPATIBLE_API_KEY", deps); + expect(result.forceInferenceSetup).toBe(false); + }); + + it("emits a [resume] note and forces inference setup when credential is already hydrated", async () => { + const recorder = makeDeps({ + providerExists: false, + credentialValue: "already-hydrated-key", + }); + const result = await ensureResumeProviderReady( + "compatible-endpoint", + "COMPATIBLE_API_KEY", + recorder.deps, + ); + expect(result.forceInferenceSetup).toBe(true); + expect(recorder.note.join("\n")).toContain("[resume]"); + expect(recorder.replaceCalls).toHaveLength(0); + }); + + it("re-prompts for credentials when the provider was reset and credential is missing (#3278)", async () => { + const recorder = makeDeps({ + providerExists: false, + credentialValue: undefined, + }); + const result = await ensureResumeProviderReady( + "compatible-endpoint", + "COMPATIBLE_API_KEY", + recorder.deps, + ); + expect(result.forceInferenceSetup).toBe(true); + expect(recorder.replaceCalls).toEqual([ + { env: "COMPATIBLE_API_KEY", label: "Compatible Endpoint API key" }, + ]); + expect(recorder.exitCalls).toEqual([]); + }); + + it("exits 1 in non-interactive mode when the provider is missing and no credential is set", async () => { + const recorder = makeDeps({ + providerExists: false, + credentialValue: undefined, + nonInteractive: true, + }); + await ensureResumeProviderReady("compatible-endpoint", "COMPATIBLE_API_KEY", recorder.deps); + expect(recorder.exitCalls).toEqual([1]); + expect(recorder.warn.join("\n")).toContain("COMPATIBLE_API_KEY"); + expect(recorder.warn.join("\n")).toContain("during resume"); + expect(recorder.replaceCalls).toHaveLength(0); + }); +}); diff --git a/src/lib/onboard/resume-provider-recovery.ts b/src/lib/onboard/resume-provider-recovery.ts new file mode 100644 index 0000000000..50ea91528b --- /dev/null +++ b/src/lib/onboard/resume-provider-recovery.ts @@ -0,0 +1,123 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +// Re-prompts for a remote provider's credential during `nemoclaw onboard --resume` +// when the previously-recorded provider has been deleted from the gateway (e.g. +// after `nemoclaw credentials reset ` removed it). +// +// Resume mode would otherwise short-circuit the inference setup step on the +// recorded `provider`/`model`, leaving the sandbox rebuild to fail with an +// authentication error (#3278). + +export type RemoteProviderConfigEntry = { + label: string; + providerName: string; + providerType: string; + credentialEnv: string; + endpointUrl: string; + helpUrl: string | null; + modelMode: "catalog" | "curated" | "input"; + defaultModel: string; + skipVerify?: boolean; +}; + +export type ResumeProviderRecoveryDeps = { + remoteProviderConfig: Record; + defaultRouteCredentialEnv: string; + isRoutedInferenceProvider: (provider: string) => boolean; + providerExistsInGateway: (name: string) => boolean; + hydrateCredentialEnv: (envName: string) => string | undefined; + getProviderLabel: (key: string) => string; + isNonInteractive: () => boolean; + log: (message: string) => void; + warn: (message: string) => void; + note: (message: string) => void; + exit: (code: number) => void; + replaceNamedCredential: ( + envName: string, + label: string, + helpUrl: string | null, + validator: (value: string) => string | null, + ) => Promise; + validateNvidiaApiKeyValue: (key: string, credentialEnv: string) => string | null; +}; + +/** + * Resolve a persisted OpenShell provider name back to its onboard provider config. + */ +export function getRemoteProviderConfigForName( + provider: string | null | undefined, + remoteProviderConfig: Record, +): RemoteProviderConfigEntry | null { + if (!provider) return null; + if (provider === "nvidia-nim") return remoteProviderConfig.build; + return ( + Object.values(remoteProviderConfig).find((entry) => entry.providerName === provider) || null + ); +} + +/** + * Choose the credential env used to recreate a missing provider during resume. + */ +export function getResumeProviderCredentialEnv( + provider: string, + config: RemoteProviderConfigEntry | null, + credentialEnv: string | null | undefined, + deps: Pick, +): string { + if (credentialEnv) return credentialEnv; + if (config?.credentialEnv) return config.credentialEnv; + return deps.isRoutedInferenceProvider(provider) ? deps.defaultRouteCredentialEnv : ""; +} + +/** + * Ensure a resumed remote provider still exists in the gateway, re-prompting + * for credentials when needed. + * + * Returns `{ forceInferenceSetup: true }` when the caller must re-run the + * inference setup step (provider was missing and credential was hydrated or + * just re-entered). Returns `{ forceInferenceSetup: false }` when no action + * is needed. + * + * In non-interactive mode with a missing credential, calls `deps.exit(1)`. + */ +export async function ensureResumeProviderReady( + provider: string | null | undefined, + credentialEnv: string | null | undefined, + deps: ResumeProviderRecoveryDeps, +): Promise<{ forceInferenceSetup: boolean }> { + const config = getRemoteProviderConfigForName(provider, deps.remoteProviderConfig); + if (!provider || (!config && !deps.isRoutedInferenceProvider(provider))) { + return { forceInferenceSetup: false }; + } + if (deps.providerExistsInGateway(provider)) return { forceInferenceSetup: false }; + + const resolvedCredentialEnv = getResumeProviderCredentialEnv(provider, config, credentialEnv, deps); + const credentialValue = deps.hydrateCredentialEnv(resolvedCredentialEnv); + const providerLabel = config?.label || deps.getProviderLabel(provider) || provider; + const helpUrl = config?.helpUrl || null; + if (!credentialValue) { + if (deps.isNonInteractive()) { + deps.warn( + ` ${resolvedCredentialEnv} is required to recreate provider '${provider}' during resume.`, + ); + deps.warn( + ` Re-run without --non-interactive to enter it, or set ${resolvedCredentialEnv} and retry.`, + ); + deps.exit(1); + return { forceInferenceSetup: false }; + } + deps.log(""); + deps.log(` [resume] Provider '${provider}' is missing from the gateway.`); + deps.log(" Re-enter the API key so onboarding can recreate it before rebuilding."); + await deps.replaceNamedCredential( + resolvedCredentialEnv, + `${providerLabel} API key`, + helpUrl, + (value) => deps.validateNvidiaApiKeyValue(value, resolvedCredentialEnv), + ); + } else { + deps.note(` [resume] Provider '${provider}' is missing from the gateway; recreating it.`); + } + return { forceInferenceSetup: true }; +} From 202c1d01bd79ca83b8352051bc4264a422acb350 Mon Sep 17 00:00:00 2001 From: Charan Jagwani Date: Fri, 22 May 2026 12:49:25 -0700 Subject: [PATCH 2/5] fix(onboard): address CodeRabbit findings on #4085 1) Major (entrypoint budget): Extract the deps wiring out of `src/lib/onboard.ts` into a new `src/lib/onboard/resume-provider-shim.ts`. The shim does the 14-field deps assembly, importing what it can directly from submodules (`./providers`, `./credential-env`, `../inference/config`, `../validation`) and lazy-requiring the remaining 5 onboard.ts symbols (`isRoutedInferenceProvider`, `providerExistsInGateway`, `isNonInteractive`, `note`, `replaceNamedCredential`). `onboard.ts` now just `require`s the shim and exports those 5 helpers so the lazy-require sees them. Net change to `onboard.ts`: -8 lines (well under the entrypoint-budget gate). 2) Minor (type alignment): `ResumeProviderRecoveryDeps.hydrateCredentialEnv` now declares `string | null` to match the actual return type of `src/lib/onboard/credential-env.ts`. Updated the test makeDeps helper accordingly. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Charan Jagwani --- src/lib/onboard.ts | 17 ++---- .../onboard/resume-provider-recovery.test.ts | 8 +-- src/lib/onboard/resume-provider-recovery.ts | 2 +- src/lib/onboard/resume-provider-shim.ts | 52 +++++++++++++++++++ 4 files changed, 61 insertions(+), 18 deletions(-) create mode 100644 src/lib/onboard/resume-provider-shim.ts diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index faa6ff3416..ffca981631 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -204,12 +204,10 @@ const { detectVllmProfile, installVllm } = require("./inference/vllm"); const inferenceConfig: typeof import("./inference/config") = require("./inference/config"); const { DEFAULT_CLOUD_MODEL, - DEFAULT_ROUTE_CREDENTIAL_ENV, getProviderSelectionConfig, parseGatewayInference, } = inferenceConfig; -const resumeProviderRecovery: typeof import("./onboard/resume-provider-recovery") = - require("./onboard/resume-provider-recovery"); +const { ensureResumeProviderReady } = require("./onboard/resume-provider-shim"); const onboardProviders = require("./onboard/providers"); const hermesProviderAuth = require("./hermes-provider-auth"); @@ -1792,16 +1790,6 @@ function providerExistsInGateway(name: string) { return onboardProviders.providerExistsInGateway(name, runOpenshell); } -const resumeProviderRecoveryDeps: import("./onboard/resume-provider-recovery").ResumeProviderRecoveryDeps = { - remoteProviderConfig: REMOTE_PROVIDER_CONFIG, defaultRouteCredentialEnv: DEFAULT_ROUTE_CREDENTIAL_ENV, - isRoutedInferenceProvider, providerExistsInGateway, getProviderLabel, isNonInteractive, - hydrateCredentialEnv: (e) => hydrateCredentialEnv(e) ?? undefined, - log: (m) => console.log(m), warn: (m) => console.error(m), note, - exit: (c) => process.exit(c), replaceNamedCredential, validateNvidiaApiKeyValue, -}; -const ensureResumeProviderReady = (p: string | null | undefined, c: string | null | undefined) => - resumeProviderRecovery.ensureResumeProviderReady(p, c, resumeProviderRecoveryDeps); - function getMessagingChannelForEnvKey(envKey: string): string | null { if (envKey === "DISCORD_BOT_TOKEN") return "discord"; if (envKey === "SLACK_BOT_TOKEN") return "slack"; @@ -10139,6 +10127,9 @@ async function onboard(opts: OnboardOptions = {}): Promise { module.exports = { buildOrphanedSandboxRollbackMessage, buildProviderArgs, + isRoutedInferenceProvider, + note, + replaceNamedCredential, buildGatewayBootstrapSecretsScript, buildCompatibleEndpointSandboxSmokeCommand, buildCompatibleEndpointSandboxSmokeScript, diff --git a/src/lib/onboard/resume-provider-recovery.test.ts b/src/lib/onboard/resume-provider-recovery.test.ts index 43c8e3c923..ddbabac69e 100644 --- a/src/lib/onboard/resume-provider-recovery.test.ts +++ b/src/lib/onboard/resume-provider-recovery.test.ts @@ -31,7 +31,7 @@ type DepsRecorder = { function makeDeps(overrides: { providerExists?: boolean; - credentialValue?: string | undefined; + credentialValue?: string | null; nonInteractive?: boolean; remoteProviderConfig?: Record; }): DepsRecorder { @@ -47,7 +47,7 @@ function makeDeps(overrides: { defaultRouteCredentialEnv: "OPENAI_API_KEY", isRoutedInferenceProvider: () => false, providerExistsInGateway: () => overrides.providerExists ?? true, - hydrateCredentialEnv: () => overrides.credentialValue, + hydrateCredentialEnv: () => overrides.credentialValue ?? null, getProviderLabel: (key) => key, isNonInteractive: () => overrides.nonInteractive ?? false, log: (m) => log.push(m), @@ -100,7 +100,7 @@ describe("ensureResumeProviderReady", () => { it("re-prompts for credentials when the provider was reset and credential is missing (#3278)", async () => { const recorder = makeDeps({ providerExists: false, - credentialValue: undefined, + credentialValue: null, }); const result = await ensureResumeProviderReady( "compatible-endpoint", @@ -117,7 +117,7 @@ describe("ensureResumeProviderReady", () => { it("exits 1 in non-interactive mode when the provider is missing and no credential is set", async () => { const recorder = makeDeps({ providerExists: false, - credentialValue: undefined, + credentialValue: null, nonInteractive: true, }); await ensureResumeProviderReady("compatible-endpoint", "COMPATIBLE_API_KEY", recorder.deps); diff --git a/src/lib/onboard/resume-provider-recovery.ts b/src/lib/onboard/resume-provider-recovery.ts index 50ea91528b..d2d1014133 100644 --- a/src/lib/onboard/resume-provider-recovery.ts +++ b/src/lib/onboard/resume-provider-recovery.ts @@ -26,7 +26,7 @@ export type ResumeProviderRecoveryDeps = { defaultRouteCredentialEnv: string; isRoutedInferenceProvider: (provider: string) => boolean; providerExistsInGateway: (name: string) => boolean; - hydrateCredentialEnv: (envName: string) => string | undefined; + hydrateCredentialEnv: (envName: string) => string | null; getProviderLabel: (key: string) => string; isNonInteractive: () => boolean; log: (message: string) => void; diff --git a/src/lib/onboard/resume-provider-shim.ts b/src/lib/onboard/resume-provider-shim.ts new file mode 100644 index 0000000000..483ff1e3cf --- /dev/null +++ b/src/lib/onboard/resume-provider-shim.ts @@ -0,0 +1,52 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +// Wires `ensureResumeProviderReady` (in `./resume-provider-recovery`) to the +// dependencies it needs. Lives outside `src/lib/onboard.ts` so the wiring +// doesn't count against the entrypoint-budget gate. + +import { DEFAULT_ROUTE_CREDENTIAL_ENV } from "../inference/config"; +import { hydrateCredentialEnv } from "./credential-env"; +import { validateNvidiaApiKeyValue } from "../validation"; +import { + ensureResumeProviderReady as ensureResumeProviderReadyImpl, + type ResumeProviderRecoveryDeps, +} from "./resume-provider-recovery"; + +const onboardProviders = require("./providers") as { + REMOTE_PROVIDER_CONFIG: ResumeProviderRecoveryDeps["remoteProviderConfig"]; + getProviderLabel: ResumeProviderRecoveryDeps["getProviderLabel"]; +}; + +// Lazy require for the symbols that live in `../onboard` itself — avoids a +// circular module load. By the time `ensureResumeProviderReady` is called, +// the onboard.ts module has finished loading and its exports are populated. +type OnboardExports = { + isRoutedInferenceProvider: ResumeProviderRecoveryDeps["isRoutedInferenceProvider"]; + providerExistsInGateway: ResumeProviderRecoveryDeps["providerExistsInGateway"]; + isNonInteractive: ResumeProviderRecoveryDeps["isNonInteractive"]; + note: ResumeProviderRecoveryDeps["note"]; + replaceNamedCredential: ResumeProviderRecoveryDeps["replaceNamedCredential"]; +}; + +export async function ensureResumeProviderReady( + provider: string | null | undefined, + credentialEnv: string | null | undefined, +): Promise<{ forceInferenceSetup: boolean }> { + const o = require("../onboard") as OnboardExports; + return ensureResumeProviderReadyImpl(provider, credentialEnv, { + remoteProviderConfig: onboardProviders.REMOTE_PROVIDER_CONFIG, + defaultRouteCredentialEnv: DEFAULT_ROUTE_CREDENTIAL_ENV, + isRoutedInferenceProvider: o.isRoutedInferenceProvider, + providerExistsInGateway: o.providerExistsInGateway, + hydrateCredentialEnv, + getProviderLabel: onboardProviders.getProviderLabel, + isNonInteractive: o.isNonInteractive, + note: o.note, + replaceNamedCredential: o.replaceNamedCredential, + validateNvidiaApiKeyValue, + log: (m) => console.log(m), + warn: (m) => console.error(m), + exit: (c) => process.exit(c), + }); +} From 56e370599abd78e972bfd772f1dbebaa578d47ec Mon Sep 17 00:00:00 2001 From: Charan Jagwani Date: Fri, 22 May 2026 13:25:26 -0700 Subject: [PATCH 3/5] fix(onboard): further shrink onboard.ts to satisfy entrypoint budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to CodeRabbit's "entrypoint budget gate" finding on #4085: my prior extraction left +6 net on onboard.ts. This pass: - Scope `forceInferenceSetup` to the while-loop iteration (removes the explicit `= false` reset in the else branch since the `let` rebinds per iteration). - Drop the redundant `#3342:` one-liner comment above `repairLocalInferenceSystemdOverrideOrExit` — the function name is self-explanatory. - Bundle the 3 shim-only exports into a single `resumeProviderShimDeps` object literal; `note` is no longer needed at all because the shim inlines `console.log(`${D}${m}${R}`)` using D/R from cli/terminal-style. - Have the shim re-use onboard.ts's existing `providerExistsInGateway` export (already a thin runOpenshell wrapper) instead of plumbing runOpenshell to the shim. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Charan Jagwani --- src/lib/onboard.ts | 8 ++------ src/lib/onboard/resume-provider-shim.ts | 26 +++++++++++++------------ 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index ffca981631..d9b127c2a7 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -9526,8 +9526,8 @@ async function onboard(opts: OnboardOptions = {}): Promise { let nimContainer = session?.nimContainer || null; let webSearchConfig = session?.webSearchConfig || null; let forceProviderSelection = forceProviderSelectionForAgentChange; - let forceInferenceSetup = false; while (true) { + let forceInferenceSetup = false; const resumeProviderSelection = !forceProviderSelection && resume && @@ -9538,10 +9538,8 @@ async function onboard(opts: OnboardOptions = {}): Promise { ({ forceInferenceSetup } = await ensureResumeProviderReady(provider, credentialEnv)); skippedStepMessage("provider_selection", `${provider} / ${model}`); hydrateCredentialEnv(credentialEnv); - // #3342: ollama-local systemd loopback drop-in repair on resume. repairLocalInferenceSystemdOverrideOrExit(provider, isNonInteractive); } else { - forceInferenceSetup = false; // #2753: do not persist sandboxName to onboard-session.json before // the sandbox actually exists in the gateway (Step 6 markStepComplete // below). A SIGINT between any earlier step and createSandbox would @@ -10127,9 +10125,7 @@ async function onboard(opts: OnboardOptions = {}): Promise { module.exports = { buildOrphanedSandboxRollbackMessage, buildProviderArgs, - isRoutedInferenceProvider, - note, - replaceNamedCredential, + resumeProviderShimDeps: { isRoutedInferenceProvider, replaceNamedCredential }, buildGatewayBootstrapSecretsScript, buildCompatibleEndpointSandboxSmokeCommand, buildCompatibleEndpointSandboxSmokeScript, diff --git a/src/lib/onboard/resume-provider-shim.ts b/src/lib/onboard/resume-provider-shim.ts index 483ff1e3cf..ee6cd950be 100644 --- a/src/lib/onboard/resume-provider-shim.ts +++ b/src/lib/onboard/resume-provider-shim.ts @@ -8,6 +8,7 @@ import { DEFAULT_ROUTE_CREDENTIAL_ENV } from "../inference/config"; import { hydrateCredentialEnv } from "./credential-env"; import { validateNvidiaApiKeyValue } from "../validation"; +import { D, R } from "../cli/terminal-style"; import { ensureResumeProviderReady as ensureResumeProviderReadyImpl, type ResumeProviderRecoveryDeps, @@ -18,32 +19,33 @@ const onboardProviders = require("./providers") as { getProviderLabel: ResumeProviderRecoveryDeps["getProviderLabel"]; }; -// Lazy require for the symbols that live in `../onboard` itself — avoids a -// circular module load. By the time `ensureResumeProviderReady` is called, -// the onboard.ts module has finished loading and its exports are populated. -type OnboardExports = { - isRoutedInferenceProvider: ResumeProviderRecoveryDeps["isRoutedInferenceProvider"]; - providerExistsInGateway: ResumeProviderRecoveryDeps["providerExistsInGateway"]; +// Lazy require breaks the circular module load — by the time +// `ensureResumeProviderReady` is called, onboard.ts has finished loading +// and its `module.exports.resumeProviderShimDeps` is populated. +type OnboardLazy = { isNonInteractive: ResumeProviderRecoveryDeps["isNonInteractive"]; - note: ResumeProviderRecoveryDeps["note"]; - replaceNamedCredential: ResumeProviderRecoveryDeps["replaceNamedCredential"]; + providerExistsInGateway: ResumeProviderRecoveryDeps["providerExistsInGateway"]; + resumeProviderShimDeps: { + isRoutedInferenceProvider: ResumeProviderRecoveryDeps["isRoutedInferenceProvider"]; + replaceNamedCredential: ResumeProviderRecoveryDeps["replaceNamedCredential"]; + }; }; export async function ensureResumeProviderReady( provider: string | null | undefined, credentialEnv: string | null | undefined, ): Promise<{ forceInferenceSetup: boolean }> { - const o = require("../onboard") as OnboardExports; + const o = require("../onboard") as OnboardLazy; return ensureResumeProviderReadyImpl(provider, credentialEnv, { remoteProviderConfig: onboardProviders.REMOTE_PROVIDER_CONFIG, defaultRouteCredentialEnv: DEFAULT_ROUTE_CREDENTIAL_ENV, - isRoutedInferenceProvider: o.isRoutedInferenceProvider, + isRoutedInferenceProvider: o.resumeProviderShimDeps.isRoutedInferenceProvider, providerExistsInGateway: o.providerExistsInGateway, hydrateCredentialEnv, getProviderLabel: onboardProviders.getProviderLabel, isNonInteractive: o.isNonInteractive, - note: o.note, - replaceNamedCredential: o.replaceNamedCredential, + note: (m) => console.log(`${D}${m}${R}`), + replaceNamedCredential: o.resumeProviderShimDeps.replaceNamedCredential, validateNvidiaApiKeyValue, log: (m) => console.log(m), warn: (m) => console.error(m), From 1e446638568279249e6f0bf7e67a01ed7127d782 Mon Sep 17 00:00:00 2001 From: Charan Jagwani Date: Fri, 22 May 2026 13:42:44 -0700 Subject: [PATCH 4/5] fix(onboard): move resumeProviderShimDeps to end of module.exports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The nested object literal `resumeProviderShimDeps: { ... }` interrupts Node's CJS→ESM named-export hoister — any module.exports entry after the nested literal becomes invisible to \`await import("dist/lib/onboard.js")\`, so \`createSandbox\` (and many others) resolved to undefined for the ESM subprocess tests in test/shellquote-sandbox.test.ts. Moving the nested literal to the very end of the export block makes the hoist cutoff harmless: every other named export is enumerable, and the shim's lazy \`require("../onboard")\` still sees \`resumeProviderShimDeps\` because CommonJS require returns the full module.exports. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Charan Jagwani --- src/lib/onboard.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 34ba4aa443..b7a7776764 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -10125,7 +10125,6 @@ async function onboard(opts: OnboardOptions = {}): Promise { module.exports = { buildOrphanedSandboxRollbackMessage, buildProviderArgs, - resumeProviderShimDeps: { isRoutedInferenceProvider, replaceNamedCredential }, buildGatewayBootstrapSecretsScript, buildCompatibleEndpointSandboxSmokeCommand, buildCompatibleEndpointSandboxSmokeScript, @@ -10258,4 +10257,5 @@ module.exports = { checkTelegramReachability, TELEGRAM_NETWORK_CURL_CODES, verifyCompatibleEndpointSandboxSmoke, + resumeProviderShimDeps: { isRoutedInferenceProvider, replaceNamedCredential }, }; From 332b89ca856015b1f26a4fc225aa91bf4299275e Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Fri, 22 May 2026 15:17:00 -0700 Subject: [PATCH 5/5] fix(onboard): preserve recovered resume credential env Signed-off-by: Aaron Erickson --- src/lib/onboard.ts | 5 +---- .../onboard/resume-provider-recovery.test.ts | 15 +++++++++++++ src/lib/onboard/resume-provider-recovery.ts | 22 ++++++++++++------- src/lib/onboard/resume-provider-shim.ts | 3 ++- 4 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 9a83a66217..b2f7899a2c 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -1502,8 +1502,6 @@ const { shouldForceCompletionsApi, } = validation; -// validateNvidiaApiKeyValue — see validation import above - async function replaceNamedCredential( envName: string, label: string, @@ -9515,7 +9513,7 @@ async function onboard(opts: OnboardOptions = {}): Promise { typeof provider === "string" && typeof model === "string"; if (resumeProviderSelection) { - ({ forceInferenceSetup } = await ensureResumeProviderReady(provider, credentialEnv)); + ({ forceInferenceSetup, credentialEnv } = await ensureResumeProviderReady(provider, credentialEnv)); skippedStepMessage("provider_selection", `${provider} / ${model}`); hydrateCredentialEnv(credentialEnv); repairLocalInferenceSystemdOverrideOrExit(provider, isNonInteractive); @@ -10181,7 +10179,6 @@ module.exports = { recoverGatewayRuntime, buildChain, buildControlUiUrls, - startGateway, findAvailableDashboardPort, findDashboardForwardOwner, diff --git a/src/lib/onboard/resume-provider-recovery.test.ts b/src/lib/onboard/resume-provider-recovery.test.ts index ddbabac69e..8b72919013 100644 --- a/src/lib/onboard/resume-provider-recovery.test.ts +++ b/src/lib/onboard/resume-provider-recovery.test.ts @@ -68,18 +68,21 @@ describe("ensureResumeProviderReady", () => { const { deps } = makeDeps({ providerExists: false }); const result = await ensureResumeProviderReady(null, null, deps); expect(result.forceInferenceSetup).toBe(false); + expect(result.credentialEnv).toBeNull(); }); it("returns false-forced when the provider is unknown and not a routed provider", async () => { const { deps } = makeDeps({ providerExists: false }); const result = await ensureResumeProviderReady("mystery-provider", null, deps); expect(result.forceInferenceSetup).toBe(false); + expect(result.credentialEnv).toBeNull(); }); it("returns false-forced when the provider still exists in the gateway", async () => { const { deps } = makeDeps({ providerExists: true }); const result = await ensureResumeProviderReady("compatible-endpoint", "COMPATIBLE_API_KEY", deps); expect(result.forceInferenceSetup).toBe(false); + expect(result.credentialEnv).toBe("COMPATIBLE_API_KEY"); }); it("emits a [resume] note and forces inference setup when credential is already hydrated", async () => { @@ -93,10 +96,21 @@ describe("ensureResumeProviderReady", () => { recorder.deps, ); expect(result.forceInferenceSetup).toBe(true); + expect(result.credentialEnv).toBe("COMPATIBLE_API_KEY"); expect(recorder.note.join("\n")).toContain("[resume]"); expect(recorder.replaceCalls).toHaveLength(0); }); + it("returns the config credential env when the resumed session did not record one", async () => { + const recorder = makeDeps({ + providerExists: false, + credentialValue: "already-hydrated-key", + }); + const result = await ensureResumeProviderReady("compatible-endpoint", null, recorder.deps); + expect(result.forceInferenceSetup).toBe(true); + expect(result.credentialEnv).toBe("COMPATIBLE_API_KEY"); + }); + it("re-prompts for credentials when the provider was reset and credential is missing (#3278)", async () => { const recorder = makeDeps({ providerExists: false, @@ -108,6 +122,7 @@ describe("ensureResumeProviderReady", () => { recorder.deps, ); expect(result.forceInferenceSetup).toBe(true); + expect(result.credentialEnv).toBe("COMPATIBLE_API_KEY"); expect(recorder.replaceCalls).toEqual([ { env: "COMPATIBLE_API_KEY", label: "Compatible Endpoint API key" }, ]); diff --git a/src/lib/onboard/resume-provider-recovery.ts b/src/lib/onboard/resume-provider-recovery.ts index d2d1014133..7e1a574ffb 100644 --- a/src/lib/onboard/resume-provider-recovery.ts +++ b/src/lib/onboard/resume-provider-recovery.ts @@ -42,6 +42,11 @@ export type ResumeProviderRecoveryDeps = { validateNvidiaApiKeyValue: (key: string, credentialEnv: string) => string | null; }; +export type ResumeProviderRecoveryResult = { + forceInferenceSetup: boolean; + credentialEnv: string | null; +}; + /** * Resolve a persisted OpenShell provider name back to its onboard provider config. */ @@ -74,10 +79,9 @@ export function getResumeProviderCredentialEnv( * Ensure a resumed remote provider still exists in the gateway, re-prompting * for credentials when needed. * - * Returns `{ forceInferenceSetup: true }` when the caller must re-run the + * Returns `forceInferenceSetup: true` when the caller must re-run the * inference setup step (provider was missing and credential was hydrated or - * just re-entered). Returns `{ forceInferenceSetup: false }` when no action - * is needed. + * just re-entered). `credentialEnv` is the env var resolved for that recovery. * * In non-interactive mode with a missing credential, calls `deps.exit(1)`. */ @@ -85,12 +89,14 @@ export async function ensureResumeProviderReady( provider: string | null | undefined, credentialEnv: string | null | undefined, deps: ResumeProviderRecoveryDeps, -): Promise<{ forceInferenceSetup: boolean }> { +): Promise { const config = getRemoteProviderConfigForName(provider, deps.remoteProviderConfig); if (!provider || (!config && !deps.isRoutedInferenceProvider(provider))) { - return { forceInferenceSetup: false }; + return { forceInferenceSetup: false, credentialEnv: credentialEnv ?? null }; + } + if (deps.providerExistsInGateway(provider)) { + return { forceInferenceSetup: false, credentialEnv: credentialEnv ?? null }; } - if (deps.providerExistsInGateway(provider)) return { forceInferenceSetup: false }; const resolvedCredentialEnv = getResumeProviderCredentialEnv(provider, config, credentialEnv, deps); const credentialValue = deps.hydrateCredentialEnv(resolvedCredentialEnv); @@ -105,7 +111,7 @@ export async function ensureResumeProviderReady( ` Re-run without --non-interactive to enter it, or set ${resolvedCredentialEnv} and retry.`, ); deps.exit(1); - return { forceInferenceSetup: false }; + return { forceInferenceSetup: false, credentialEnv: resolvedCredentialEnv }; } deps.log(""); deps.log(` [resume] Provider '${provider}' is missing from the gateway.`); @@ -119,5 +125,5 @@ export async function ensureResumeProviderReady( } else { deps.note(` [resume] Provider '${provider}' is missing from the gateway; recreating it.`); } - return { forceInferenceSetup: true }; + return { forceInferenceSetup: true, credentialEnv: resolvedCredentialEnv }; } diff --git a/src/lib/onboard/resume-provider-shim.ts b/src/lib/onboard/resume-provider-shim.ts index ee6cd950be..267fdbd896 100644 --- a/src/lib/onboard/resume-provider-shim.ts +++ b/src/lib/onboard/resume-provider-shim.ts @@ -12,6 +12,7 @@ import { D, R } from "../cli/terminal-style"; import { ensureResumeProviderReady as ensureResumeProviderReadyImpl, type ResumeProviderRecoveryDeps, + type ResumeProviderRecoveryResult, } from "./resume-provider-recovery"; const onboardProviders = require("./providers") as { @@ -34,7 +35,7 @@ type OnboardLazy = { export async function ensureResumeProviderReady( provider: string | null | undefined, credentialEnv: string | null | undefined, -): Promise<{ forceInferenceSetup: boolean }> { +): Promise { const o = require("../onboard") as OnboardLazy; return ensureResumeProviderReadyImpl(provider, credentialEnv, { remoteProviderConfig: onboardProviders.REMOTE_PROVIDER_CONFIG,