diff --git a/docs/get-started/quickstart.mdx b/docs/get-started/quickstart.mdx index 6c3ae76d7a9..a991f4f0ebe 100644 --- a/docs/get-started/quickstart.mdx +++ b/docs/get-started/quickstart.mdx @@ -302,8 +302,11 @@ Use these details when your first-run path needs more control. ``` The default is `Y`. - Press Enter to continue, or answer `n` to abort cleanly, correct the entries, and rerun `nemoclaw onboard`. - Non-interactive runs print the summary for log clarity but skip the prompt. + Press Enter to accept the configuration. + If you answer `n`, onboarding exits with a nonzero status and clears the recorded provider, model, and sandbox name. + The rejected run does not register a new gateway credential. + Run `nemoclaw onboard` to make new choices. + Non-interactive runs print the summary and skip the prompt. diff --git a/docs/inference/set-up-ollama.mdx b/docs/inference/set-up-ollama.mdx index d95e263d5a4..de52d9f7760 100644 --- a/docs/inference/set-up-ollama.mdx +++ b/docs/inference/set-up-ollama.mdx @@ -170,7 +170,7 @@ The agent inside the sandbox never receives the token directly because the OpenS The wizard manages the proxy lifecycle: - It generates a random 24-byte token and stores it in `~/.nemoclaw/ollama-proxy-token` with `0600` permissions. -- It starts and verifies the proxy after Ollama. +- It starts and verifies the proxy only after you accept the onboarding configuration. - It removes stale matching proxy processes from previous runs. - It probes the sandbox Docker network path before saving the inference route. - It stops matching proxy processes during uninstall. diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 7c4636faa61..13b15ab6170 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -341,6 +341,16 @@ For a profile-backed session, resume requires the same catalog, preset, and reci Omit `--profile` to reuse that recorded selection, or pass the same profile explicitly; use `--fresh` to adopt a changed catalog definition. Legacy sessions without a profile-provenance record continue to resume normally, but cannot acquire a new `--profile` selection during resume. +Before the configuration review, NemoClaw records the sandbox name and the selected provider and model as an incomplete choice. +If onboarding stops at the review prompt, an interactive `--resume` run shows the prompt again. +A non-interactive `--resume` run reuses the recorded choice and continues to inference setup. +After you accept the review, NemoClaw records the choice before inference setup starts. +If inference setup fails, `--resume` reuses the accepted provider, model, and sandbox name. +If you reject the review, onboarding exits with a nonzero status and clears those recorded choices. +Run `$$nemoclaw onboard` to make new choices after rejection. +During a resume without terminal input, `--yes` or `NEMOCLAW_YES=1` also selects non-interactive resume behavior. +For a new or fresh session, `--yes` and `NEMOCLAW_YES=1` accept supported confirmations but do not replace `--non-interactive`. + OpenClaw sessions also record the web search selection, messaging selection and non-secret settings, and resource profile. diff --git a/src/lib/core/non-interactive.test.ts b/src/lib/core/non-interactive.test.ts index 2da282fe2d8..a2cd7d7680b 100644 --- a/src/lib/core/non-interactive.test.ts +++ b/src/lib/core/non-interactive.test.ts @@ -10,12 +10,13 @@ afterEach(() => { }); describe("non-interactive environment detection", () => { - it("treats only the canonical value as non-interactive", () => { + it("treats only the canonical explicit value as non-interactive", () => { expect(isNonInteractiveEnv({ NEMOCLAW_NON_INTERACTIVE: "1" } as NodeJS.ProcessEnv)).toBe(true); expect(isNonInteractiveEnv({ NEMOCLAW_NON_INTERACTIVE: "true" } as NodeJS.ProcessEnv)).toBe( false, ); expect(isNonInteractiveEnv({ NEMOCLAW_NON_INTERACTIVE: "" } as NodeJS.ProcessEnv)).toBe(false); + expect(isNonInteractiveEnv({ NEMOCLAW_YES: "1" } as NodeJS.ProcessEnv)).toBe(false); expect(isNonInteractiveEnv({} as NodeJS.ProcessEnv)).toBe(false); }); diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index ce55a31fc5d..d5a79abc5e7 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -1061,7 +1061,6 @@ const { ensureOllamaLoopbackSystemdOverride, runOllamaStartupOrGate, shouldFrontOllamaWithProxy, - startOllamaAuthProxy, getLocalProviderBaseUrl, selectAndValidateOllamaModel, printOllamaExposureWarning, @@ -3671,6 +3670,7 @@ const { const startRecordedStep = onboardRuntimeBoundary.startRecordedStep.bind(onboardRuntimeBoundary); const recordStepComplete = onboardRuntimeBoundary.recordStepComplete.bind(onboardRuntimeBoundary); +const recordStepRejected = onboardRuntimeBoundary.recordStepRejected.bind(onboardRuntimeBoundary); const recordStepSkipped = onboardRuntimeBoundary.recordStepSkipped.bind(onboardRuntimeBoundary); const recordStepFailed = onboardRuntimeBoundary.recordStepFailed.bind(onboardRuntimeBoundary); const recordStateSkipped = onboardRuntimeBoundary.recordStateSkipped.bind(onboardRuntimeBoundary); @@ -3752,33 +3752,16 @@ async function runOnboard(opts: OnboardOptions = {}): Promise { authoritativeGateway?.name ?? GATEWAY_NAME, ); setOnboardBrandingAgent(opts.agent || process.env.NEMOCLAW_AGENT || null); - NON_INTERACTIVE = opts.nonInteractive || isNonInteractiveEnv(); - RECREATE_SANDBOX = opts.recreateSandbox || process.env.NEMOCLAW_RECREATE_SANDBOX === "1"; AUTO_YES = opts.autoYes === true || process.env.NEMOCLAW_YES === "1"; + // biome-ignore format: keep src/lib/onboard.ts net-neutral for growth guardrail. + const { fresh, nonInteractive, requestedFromDockerfile, requestedSandboxName, cannotPrompt, resume } = onboardEntryOptions.resolveOnboardRunEntryOptions(opts, process.env, onboardSession.loadSession()?.status ?? null, isNonInteractiveEnv, { validateName, reservedSandboxNames: RESERVED_SANDBOX_NAMES, cliDisplayName, getNameValidationGuidance, error: (message) => console.error(message), exitProcess: (code) => process.exit(code) }); + NON_INTERACTIVE = nonInteractive; + RECREATE_SANDBOX = opts.recreateSandbox || process.env.NEMOCLAW_RECREATE_SANDBOX === "1"; _preflightDashboardPort = opts.controlUiPort ?? (process.env.NEMOCLAW_DASHBOARD_PORT != null ? DASHBOARD_PORT : null); onboardRuntimeBoundary.reset(); if (!authoritativeGateway) delete process.env.OPENSHELL_GATEWAY; preparedDcodeRuntime.applyGatewayEnv(process.env); - const { resume, fresh, requestedFromDockerfile, requestedSandboxName, cannotPrompt } = - onboardEntryOptions.resolveOnboardEntryOptions( - { - opts, - env: process.env, - stdinIsTty: Boolean(process.stdin && process.stdin.isTTY), - stdoutIsTty: Boolean(process.stdout && process.stdout.isTTY), - persistedSessionStatus: onboardSession.loadSession()?.status ?? null, - }, - { - isNonInteractive, - validateName, - reservedSandboxNames: RESERVED_SANDBOX_NAMES, - cliDisplayName, - getNameValidationGuidance, - error: (message) => console.error(message), - exitProcess: (code) => process.exit(code), - }, - ); const baseImageResolutionContext = baseImageResolutionFlow.createBaseImageResolutionContext({ fresh, initialHint: opts.baseImageResolutionHint, @@ -4111,6 +4094,9 @@ async function runOnboard(opts: OnboardOptions = {}): Promise { // biome-ignore format: keep src/lib/onboard.ts net-neutral for growth guardrail. const endpointProvenance = { endpointSource: opts.endpointSource, endpointSourceProvider: opts.rebuildRegistryInferenceRoute?.route.provider ?? null, endpointSourceEndpointUrl: opts.rebuildRegistryInferenceRoute?.route.endpointUrl ?? null, getSandboxRegistryEntry: registry.getSandbox }; // biome-ignore format: keep src/lib/onboard.ts net-neutral for growth guardrail. + // biome-ignore format: keep src/lib/onboard.ts net-neutral for growth guardrail. + const providerReviewDeps = setupInferenceFactory.createProviderReviewDeps(onboardSession.updateSession, onboardSessionBootstrap.checkpointSandboxName, { shouldFrontOllamaWithProxy, startOllamaAuthProxy, getOllamaProxyToken, persistAndProbeOllamaProxy }, process.exit, console.error); + // biome-ignore format: keep src/lib/onboard.ts net-neutral for growth guardrail. const coreFlowPhases = createCoreOnboardFlowPhases({ // biome-ignore format: keep src/lib/onboard.ts net-neutral for growth guardrail. resumeProvider: { isNonInteractive, isRoutedInferenceProvider, providerExistsInGateway, replaceNamedCredential, resumeManagedLlamaCppRuntime: (sandboxName) => setupNimFlow.resumeManagedLlamaCppRuntime(sandboxName, { gatewayPort: GATEWAY_PORT, runtimeProvider: setupNimFlow.resolveCurrentRuntimeProviderBundle() }) }, @@ -4136,6 +4122,7 @@ async function runOnboard(opts: OnboardOptions = {}): Promise { setupInference, startRecordedStep, recordStepComplete, + recordStepRejected, toSessionUpdates: (updates) => toSessionUpdates(updates as Parameters[0]), skippedStepMessage, @@ -4167,6 +4154,7 @@ async function runOnboard(opts: OnboardOptions = {}): Promise { }, reserveSandboxInferenceRoute: registry.reserveSandboxInferenceRoute, registryUpdateSandbox: (name, updates) => registry.updateSandbox(name, updates), + ...providerReviewDeps, promptValidatedSandboxName, assessHost, formatSandboxBuildEstimateNote, diff --git a/src/lib/onboard/entry-options.test.ts b/src/lib/onboard/entry-options.test.ts index 83a9fc2a456..f6a69a163b6 100644 --- a/src/lib/onboard/entry-options.test.ts +++ b/src/lib/onboard/entry-options.test.ts @@ -6,6 +6,7 @@ import { describe, expect, it, vi } from "vitest"; import { type OnboardEntryOptionsDeps, resolveOnboardEntryOptions, + resolveOnboardRunOptions, withNonInteractiveEnvironment, } from "./entry-options"; @@ -30,6 +31,41 @@ function createDeps(overrides: Partial = {}): OnboardEn }; } +describe("resolveOnboardRunOptions", () => { + it.each([ + [false, true], + [false, false], + ])("treats auto-yes resume as non-interactive when stdin=%s and stdout=%s", (stdinIsTty, stdoutIsTty) => { + expect( + resolveOnboardRunOptions({ autoYes: true, resume: true }, {}, null, () => false, { + stdinIsTty, + stdoutIsTty, + }).nonInteractive, + ).toBe(true); + }); + + it.each([ + [true, true], + [true, false], + ])("keeps auto-yes resume interactive when stdin=%s and stdout=%s", (stdinIsTty, stdoutIsTty) => { + expect( + resolveOnboardRunOptions({ autoYes: true, resume: true }, {}, null, () => false, { + stdinIsTty, + stdoutIsTty, + }).nonInteractive, + ).toBe(false); + }); + + it("keeps fresh no-TTY auto-yes interactive", () => { + expect( + resolveOnboardRunOptions({ autoYes: true }, {}, null, () => false, { + stdinIsTty: false, + stdoutIsTty: false, + }).nonInteractive, + ).toBe(false); + }); +}); + describe("resolveOnboardEntryOptions", () => { it("rejects mutually exclusive resume and fresh flags", () => { const deps = createDeps(); @@ -157,6 +193,23 @@ describe("resolveOnboardEntryOptions", () => { expect(deps.error).not.toHaveBeenCalled(); }); + it("does not auto-resume a rejected non-resumable session", () => { + const deps = createDeps(); + + const result = resolveOnboardEntryOptions( + { + opts: {}, + env: {}, + stdinIsTty: true, + stdoutIsTty: true, + persistedSessionStatus: "failed", + }, + deps, + ); + + expect(result.resume).toBe(false); + }); + it("does not auto-resume when --fresh is set even with an in_progress session (#5470)", () => { const deps = createDeps(); diff --git a/src/lib/onboard/entry-options.ts b/src/lib/onboard/entry-options.ts index 39e225b38e0..220178b64c4 100644 --- a/src/lib/onboard/entry-options.ts +++ b/src/lib/onboard/entry-options.ts @@ -51,6 +51,51 @@ export interface ResolvedOnboardEntryOptions { type NonInteractiveEntryOptions = { nonInteractive?: boolean }; type ResumableEntryOptions = NonInteractiveEntryOptions & { resume?: boolean; fresh?: boolean }; + +export function resolveOnboardRunOptions( + options: OnboardEntryOptionsInput["opts"] & { autoYes?: boolean; nonInteractive?: boolean }, + env: NodeJS.ProcessEnv, + persistedSessionStatus: string | null, + isNonInteractiveEnv: () => boolean, + terminal: { stdinIsTty: boolean; stdoutIsTty: boolean } = { + stdinIsTty: Boolean(process.stdin?.isTTY), + stdoutIsTty: Boolean(process.stdout?.isTTY), + }, +) { + const resume = + options.resume === true || (options.fresh !== true && persistedSessionStatus === "in_progress"); + const nonInteractive = + options.nonInteractive === true || + ((options.autoYes === true || env.NEMOCLAW_YES === "1") && resume && !terminal.stdinIsTty) || + isNonInteractiveEnv(); + return { + resume, + nonInteractive, + entryOptionsInput: { opts: options, env, ...terminal, persistedSessionStatus }, + }; +} + +export function resolveOnboardRunEntryOptions( + options: OnboardEntryOptionsInput["opts"] & { autoYes?: boolean; nonInteractive?: boolean }, + env: NodeJS.ProcessEnv, + persistedSessionStatus: string | null, + isNonInteractiveEnv: () => boolean, + deps: Omit, +) { + const context = resolveOnboardRunOptions( + options, + env, + persistedSessionStatus, + isNonInteractiveEnv, + ); + return { + ...context, + ...resolveOnboardEntryOptions(context.entryOptionsInput, { + ...deps, + isNonInteractive: () => context.nonInteractive, + }), + }; +} interface StationExpressSessionLifecycle { loadSession(): StationExpressSessionLike | null; reconcileStationExpressReceiptRetirement(generation: string): void; diff --git a/src/lib/onboard/inference-providers/ollama-local.ts b/src/lib/onboard/inference-providers/ollama-local.ts index 95a44b70b60..028b2fe1b77 100644 --- a/src/lib/onboard/inference-providers/ollama-local.ts +++ b/src/lib/onboard/inference-providers/ollama-local.ts @@ -7,7 +7,12 @@ import type { OllamaDeps, SetupInferenceResult } from "./types"; export async function setupOllamaLocalInference( - args: { model: string; provider: string; allowToolsIncompatible: boolean }, + args: { + model: string; + provider: string; + allowToolsIncompatible: boolean; + preparedProxyToken?: string; + }, deps: OllamaDeps, ): Promise<{ done: true; result: SetupInferenceResult } | { done: false }> { const { model, provider, allowToolsIncompatible } = args; @@ -40,7 +45,7 @@ export async function setupOllamaLocalInference( // 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(); + if (!args.preparedProxyToken) ensureOllamaAuthProxy(); proxyReady = isProxyHealthy(); } if (proxyReady) { @@ -66,17 +71,19 @@ export async function setupOllamaLocalInference( const baseUrl = getLocalProviderBaseUrl(provider); let ollamaCredential = "ollama"; if (frontOllamaWithProxy) { - // Skip if already started during the fallback recovery above. - if (!proxyReady) ensureOllamaAuthProxy(); - const proxyToken = getOllamaProxyToken(); + // The normal onboarding path prepares the proxy once, after review. The + // fallback remains for recovery callers that enter provider setup without + // a prepared token. + if (!args.preparedProxyToken && !proxyReady) ensureOllamaAuthProxy(); + const proxyToken = args.preparedProxyToken ?? getOllamaProxyToken(); if (!proxyToken) { error(" Ollama auth proxy token is not set. Re-run onboard to initialize the proxy."); return exitProcess(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 (!args.preparedProxyToken) { + await persistAndProbeOllamaProxy(proxyToken); + } } // Use a dedicated internal credential env (NEMOCLAW_OLLAMA_PROXY_TOKEN) // so the gateway never reads the user's host OPENAI_API_KEY for local diff --git a/src/lib/onboard/machine/core-flow-phases.test.ts b/src/lib/onboard/machine/core-flow-phases.test.ts index 3e788d169f8..e6f35ab664b 100644 --- a/src/lib/onboard/machine/core-flow-phases.test.ts +++ b/src/lib/onboard/machine/core-flow-phases.test.ts @@ -147,6 +147,7 @@ function createPhases( recordStepComplete: vi.fn(async (_stepName: string, updates: SessionUpdates = {}) => sessionWithUpdates(updates), ), + recordStepRejected: vi.fn(async () => createSession()), toSessionUpdates: (updates) => updates as SessionUpdates, skippedStepMessage: vi.fn(), ensureManagedLlamaCppResumeReady: vi.fn(async () => false), @@ -183,6 +184,8 @@ function createPhases( }), reserveSandboxInferenceRoute: vi.fn(() => true), registryUpdateSandbox: vi.fn(), + checkpointSandboxIdentity: vi.fn(async () => undefined), + prepareLocalProviderForInference: vi.fn(async () => null), promptValidatedSandboxName: vi.fn(async () => "my-sandbox"), assessHost: () => ({ memoryGb: 64 }), formatSandboxBuildEstimateNote: () => null, diff --git a/src/lib/onboard/machine/handlers/provider-inference-route-containment.test.ts b/src/lib/onboard/machine/handlers/provider-inference-route-containment.test.ts index a832eaf86da..fc61bcadd8f 100644 --- a/src/lib/onboard/machine/handlers/provider-inference-route-containment.test.ts +++ b/src/lib/onboard/machine/handlers/provider-inference-route-containment.test.ts @@ -93,6 +93,7 @@ function createDeps() { setupInference: calls.setupInference, startRecordedStep: vi.fn(async () => undefined), recordStepComplete: calls.recordStepComplete, + recordStepRejected: vi.fn(async () => createSession()), toSessionUpdates: (updates: Record) => updates as SessionUpdates, skippedStepMessage: vi.fn(), ensureManagedLlamaCppResumeReady: vi.fn(async () => false), @@ -118,6 +119,8 @@ function createDeps() { reupsertRoutedProvider: calls.reupsertRoutedProvider, reserveSandboxInferenceRoute: calls.reserveRoute, registryUpdateSandbox: calls.updateSandbox, + checkpointSandboxIdentity: vi.fn(async () => undefined), + prepareLocalProviderForInference: vi.fn(async () => null), promptValidatedSandboxName: vi.fn(async () => "target-sandbox"), assessHost: () => ({ cpus: 8 }), formatSandboxBuildEstimateNote: () => "estimate", diff --git a/src/lib/onboard/machine/handlers/provider-inference.test-support.ts b/src/lib/onboard/machine/handlers/provider-inference.test-support.ts index 94f0169e1c7..a1571048c29 100644 --- a/src/lib/onboard/machine/handlers/provider-inference.test-support.ts +++ b/src/lib/onboard/machine/handlers/provider-inference.test-support.ts @@ -101,6 +101,7 @@ export function createDeps( setupInference: vi.fn(async () => ({ ok: true as const })), startStep: vi.fn(async () => undefined), complete: vi.fn(async () => createSession()), + rejected: vi.fn(async () => createSession()), skipped: vi.fn(), recoverProvider: vi.fn( async ( @@ -133,6 +134,8 @@ export function createDeps( ), reserveRoute: vi.fn(() => true), updateSandbox: vi.fn(), + checkpointSandboxIdentity: vi.fn(async () => undefined), + prepareLocalProviderForInference: vi.fn(async () => null), promptName: vi.fn(async () => "my-assistant"), promptYesNo: vi.fn(async () => true), log: vi.fn(), @@ -158,6 +161,7 @@ export function createDeps( setupInference: calls.setupInference, startRecordedStep: calls.startStep, recordStepComplete: calls.complete, + recordStepRejected: calls.rejected, toSessionUpdates: (updates: Record) => updates as SessionUpdates, skippedStepMessage: calls.skipped, ensureManagedLlamaCppResumeReady: calls.recoverManagedLlamaCpp, @@ -195,6 +199,8 @@ export function createDeps( reupsertRoutedProvider: calls.reupsertRoutedProvider, reserveSandboxInferenceRoute: calls.reserveRoute, registryUpdateSandbox: calls.updateSandbox, + checkpointSandboxIdentity: calls.checkpointSandboxIdentity, + prepareLocalProviderForInference: calls.prepareLocalProviderForInference, promptValidatedSandboxName: calls.promptName, assessHost: () => ({ cpus: 8 }), formatSandboxBuildEstimateNote: () => "estimate", diff --git a/src/lib/onboard/machine/handlers/provider-inference.test.ts b/src/lib/onboard/machine/handlers/provider-inference.test.ts index 212588faeaa..05237753bc9 100644 --- a/src/lib/onboard/machine/handlers/provider-inference.test.ts +++ b/src/lib/onboard/machine/handlers/provider-inference.test.ts @@ -6,7 +6,7 @@ import { describe, expect, it, vi } from "vitest"; import { decisionSelected, decisionUnset } from "../../../state/onboard-checkpoint-decision"; import { deriveCheckpointFromSession } from "../../../state/onboard-checkpoint-migrate"; import type { CheckpointSandboxIdentity } from "../../../state/onboard-checkpoint-types"; -import { createSession } from "../../../state/onboard-session"; +import { createSession, type SessionUpdates } from "../../../state/onboard-session"; import { handleProviderInferenceState, type ProviderInferenceStateOptions, @@ -38,7 +38,11 @@ describe("handleProviderInferenceState", () => { it("runs provider selection and inference setup on a fresh flow", async () => { const { deps, calls } = createDeps(); const session = createSession(); - calls.complete.mockResolvedValue(session); + calls.complete.mockImplementation(async (...args: unknown[]) => { + session.steps.provider_selection.status = + args[0] === "provider_selection" ? "complete" : session.steps.provider_selection.status; + return session; + }); const result = await handleProviderInferenceState(baseOptions(deps, session)); @@ -60,7 +64,11 @@ describe("handleProviderInferenceState", () => { expect(selectionUpdates).not.toHaveProperty("onboardEndpointUrl"); expect(calls.promptName).toHaveBeenCalledWith(null); expect(calls.log).toHaveBeenCalledWith("summary:nvidia-prod/nvidia/test/my-assistant"); - expect(calls.startStep).toHaveBeenNthCalledWith(2, "inference", { + expect(calls.startStep).toHaveBeenNthCalledWith(2, "provider_selection", { + provider: "nvidia-prod", + model: "nvidia/test", + }); + expect(calls.startStep).toHaveBeenNthCalledWith(3, "inference", { provider: "nvidia-prod", model: "nvidia/test", }); @@ -1420,18 +1428,6 @@ describe("handleProviderInferenceState", () => { ]); }); - it("aborts before inference setup when the configuration summary is rejected", async () => { - const { deps, calls } = createDeps({ - isNonInteractive: () => false, - promptYesNoOrDefault: vi.fn(async () => false), - }); - - await expect(handleProviderInferenceState(baseOptions(deps))).rejects.toThrow("exit 0"); - - expect(calls.exit).toHaveBeenCalledWith(0); - expect(calls.setupInference).not.toHaveBeenCalled(); - }); - // Regression: #4241. When the provider selection step accepted a no-tools // Ollama model (the user answered "yes" to the override prompt or // NEMOCLAW_OLLAMA_REQUIRE_TOOLS=0 was set), the same flag must reach diff --git a/src/lib/onboard/machine/handlers/provider-inference.ts b/src/lib/onboard/machine/handlers/provider-inference.ts index 048f05c8eb7..9cef63634ac 100644 --- a/src/lib/onboard/machine/handlers/provider-inference.ts +++ b/src/lib/onboard/machine/handlers/provider-inference.ts @@ -67,6 +67,8 @@ export interface ProviderInferenceSetupOptions { reservationSessionId?: string; /** Recheck recorded-route ownership after acquiring route mutation locks. */ isRecordedProviderRecoveryAuthorized?: () => boolean; + /** Proxy token prepared after configuration review; avoids repeating host mutations in setup. */ + preparedOllamaProxyToken?: string; } export interface ProviderSelectionResult { @@ -170,6 +172,7 @@ export interface ProviderInferenceStateOptions { updates?: { provider?: string | null; model?: string | null }, ): Promise; recordStepComplete(stepName: string, updates: SessionUpdates): Promise; + recordStepRejected(stepName: string): Promise; toSessionUpdates(updates: Record): SessionUpdates; skippedStepMessage(stepName: string, detail?: string | null): void; ensureResumeProviderReady( @@ -238,6 +241,8 @@ export interface ProviderInferenceStateOptions { }, ): boolean; registryUpdateSandbox(sandboxName: string, updates: { nimContainer?: string | null }): void; + checkpointSandboxIdentity(sandboxName: string, agent: Agent): Promise; + prepareLocalProviderForInference(provider: string): Promise; promptValidatedSandboxName(agent: Agent): Promise; assessHost(): Host; formatSandboxBuildEstimateNote(host: Host): string | null; @@ -413,6 +418,109 @@ function provenResumeSandboxName( : null; } +function reviewRecoveryState(session: Session | null, sandboxName: string | null) { + return ( + session?.steps?.provider_selection?.status === "failed" && + session.sandboxPromptProgress.sandboxName === true && + session.sandboxName === sandboxName + ); +} + +function canResumeProviderSelection( + forceProviderSelection: boolean, + effectiveResume: boolean, + authoritativeResumeConfig: boolean, + session: Session | null, + interruptedReview: boolean, + provider: string | null, + model: string | null, +): boolean { + return ( + !forceProviderSelection && + effectiveResume && + (authoritativeResumeConfig || + session?.steps?.provider_selection?.status === "complete" || + interruptedReview) && + typeof provider === "string" && + typeof model === "string" + ); +} + +type ResumeReasoningDeps = Pick< + ProviderInferenceStateOptions["deps"], + | "clearCompatibleEndpointReasoning" + | "clearCompatibleEndpointReasoningEffort" + | "cliName" + | "configureCompatibleEndpointReasoning" + | "configureCompatibleEndpointReasoningEffort" + | "log" +>; + +async function configureResumeReasoning( + provider: string, + reasoning: string | null, + effort: string | null, + env: NodeJS.ProcessEnv, + deps: ResumeReasoningDeps, +): Promise<{ reasoning: string | null; effort: string | null }> { + if (provider !== "compatible-endpoint") { + return { + reasoning: deps.clearCompatibleEndpointReasoning(), + effort: deps.clearCompatibleEndpointReasoningEffort(), + }; + } + const ignoredReasoning = describeIgnoredReasoningEnv(reasoning, deps.cliName()); + if (ignoredReasoning) deps.log(ignoredReasoning); + const ignoredEffort = describeIgnoredReasoningEffortEnv(effort, deps.cliName(), env); + if (ignoredEffort) deps.log(ignoredEffort); + return { + reasoning: await deps.configureCompatibleEndpointReasoning(reasoning), + effort: await deps.configureCompatibleEndpointReasoningEffort(effort, env, false), + }; +} + +type LocalInferenceRepairDeps = Pick< + ProviderInferenceStateOptions["deps"], + "recordRepairEvent" | "repairLocalInferenceSystemdOverrideOrExit" +>; + +async function repairResumedLocalInference( + provider: string, + model: string, + agent: unknown, + deps: LocalInferenceRepairDeps, +): Promise { + const options = { + provider, + model, + contextWindowFloor: getOllamaContextWindowFloorForAgent(agentName(agent)), + isNonInteractive: () => false, + }; + if (provider !== "ollama-local") { + deps.repairLocalInferenceSystemdOverrideOrExit(options); + return; + } + const metadata = { repair: "ollama-systemd-loopback" }; + await deps.recordRepairEvent("state.repair.started", { + state: "provider_selection", + metadata, + }); + try { + deps.repairLocalInferenceSystemdOverrideOrExit(options); + } catch (error) { + await deps.recordRepairEvent("state.repair.failed", { + state: "provider_selection", + error: error instanceof Error ? error.message : String(error), + metadata, + }); + throw error; + } + await deps.recordRepairEvent("state.repair.completed", { + state: "provider_selection", + metadata, + }); +} + export async function handleProviderInferenceState({ gatewayName, resume, @@ -500,13 +608,26 @@ export async function handleProviderInferenceState({ recoveryReceiptLedger: providerRecoveryReceiptLedger, gatewayName, }); - const resumeProviderSelection = - !forceProviderSelection && - effectiveResume && - (authoritativeResumeConfig || session?.steps?.provider_selection?.status === "complete") && - typeof provider === "string" && - typeof model === "string"; + const completeRecoveredReviewSelectionAfterInference = reviewRecoveryState( + session, + sandboxName, + ); + const reviewRecoveredInteractively = + completeRecoveredReviewSelectionAfterInference && !deps.isNonInteractive(); + const resumeProviderSelection = canResumeProviderSelection( + forceProviderSelection, + effectiveResume, + authoritativeResumeConfig, + session, + completeRecoveredReviewSelectionAfterInference, + provider, + model, + ); let shouldRecordProviderSelection = false; + // A review interruption selected a provider but did not configure its + // route. Do not let a coincidentally ready gateway route skip setup. + forceInferenceSetup ||= + completeRecoveredReviewSelectionAfterInference || reviewRecoveredInteractively; if (resumeProviderSelection) { assertOnboardReasoningEffortRoute(reasoningEffortRequest, provider, preferredInferenceApi); assertProviderInferenceRouteCompatible(deps, gatewayName, sandboxName, { @@ -591,61 +712,24 @@ export async function handleProviderInferenceState({ provider, model, }); - if (provider === "compatible-endpoint") { - // Report before configuring: configureCompatibleEndpointReasoning - // overwrites process.env.NEMOCLAW_REASONING with the recorded value. - const ignoredReasoning = describeIgnoredReasoningEnv( - compatibleEndpointReasoning, - deps.cliName(), - ); - if (ignoredReasoning) deps.log(ignoredReasoning); - const ignoredReasoningEffort = describeIgnoredReasoningEffortEnv( - compatibleEndpointReasoningEffort, - deps.cliName(), - env, - ); - if (ignoredReasoningEffort) deps.log(ignoredReasoningEffort); - compatibleEndpointReasoning = await deps.configureCompatibleEndpointReasoning( - compatibleEndpointReasoning, - ); - compatibleEndpointReasoningEffort = await deps.configureCompatibleEndpointReasoningEffort( - compatibleEndpointReasoningEffort, - env, - false, - ); - } else { - compatibleEndpointReasoning = deps.clearCompatibleEndpointReasoning(); - compatibleEndpointReasoningEffort = deps.clearCompatibleEndpointReasoningEffort(); - } - const localInferenceRepairOptions = { - provider, - model, - contextWindowFloor: getOllamaContextWindowFloorForAgent(agentName(agent)), - isNonInteractive: deps.isNonInteractive, - }; - if (provider === "ollama-local") { - const repairMetadata = { repair: "ollama-systemd-loopback" }; - await deps.recordRepairEvent("state.repair.started", { - state: "provider_selection", - metadata: repairMetadata, - }); - try { - deps.repairLocalInferenceSystemdOverrideOrExit(localInferenceRepairOptions); - } catch (err) { - await deps.recordRepairEvent("state.repair.failed", { - state: "provider_selection", - error: err instanceof Error ? err.message : String(err), - metadata: repairMetadata, - }); - throw err; - } - await deps.recordRepairEvent("state.repair.completed", { - state: "provider_selection", - metadata: repairMetadata, - }); - } else { - deps.repairLocalInferenceSystemdOverrideOrExit(localInferenceRepairOptions); - } + const resumedSelection = requireSelection(provider, model, deps); + const configuredReasoning = await configureResumeReasoning( + resumedSelection.provider, + compatibleEndpointReasoning, + compatibleEndpointReasoningEffort, + env, + deps, + ); + compatibleEndpointReasoning = configuredReasoning.reasoning; + compatibleEndpointReasoningEffort = configuredReasoning.effort; + await repairResumedLocalInference(resumedSelection.provider, resumedSelection.model, agent, { + ...deps, + repairLocalInferenceSystemdOverrideOrExit: (options) => + deps.repairLocalInferenceSystemdOverrideOrExit({ + ...options, + isNonInteractive: deps.isNonInteractive, + }), + }); } else { // An incomplete Station Express resume intentionally retries setupNim here. The outer // Station resume wrapper restores the exact provider/model as non-interactive env input, @@ -748,10 +832,14 @@ export async function handleProviderInferenceState({ preferredInferenceApi, }); } - if (shouldRecordProviderSelection) { - // Provider selection is not yet durable route trust. Deliberately omit - // endpointSource/onboardEndpointUrl here so an interrupted run fails - // closed and revalidates the endpoint before inference setup on resume. + if ( + shouldRecordProviderSelection && + (authoritativeResumeConfig || effectiveResume) && + !completeRecoveredReviewSelectionAfterInference + ) { + // Authoritative rebuild selections are already route-validated. Persist + // them before inference setup so their reservation can retain the + // authoritative session identity. session = await deps.recordStepComplete( "provider_selection", deps.toSessionUpdates({ @@ -761,9 +849,6 @@ export async function handleProviderInferenceState({ credentialEnv, hermesAuthMethod, hermesToolGateways, - // An authoritative rebuild records route fidelity before inference - // setup. Keep the stale marker until the provider surface heal - // succeeds so a failed attempt remains armed on the next resume. preferredInferenceApi: healAdjustedInferenceApi ? initial.preferredInferenceApi : preferredInferenceApi, @@ -992,18 +1077,52 @@ export async function handleProviderInferenceState({ }), ); deps.log(" Web search and messaging channels will be prompted next."); + // Persist canonical sandbox identity and selection before local-provider + // preparation. A preparation failure or SIGINT must leave a no-TTY- + // resumable provider_selection step without claiming inference setup + // completed. + await deps.checkpointSandboxIdentity(confirmedSandboxName, agent); + const needsReviewSelectionRecovery = + session?.steps?.provider_selection?.status !== "complete"; + if (needsReviewSelectionRecovery) { + await deps.startRecordedStep("provider_selection", { provider, model }); + } if (!deps.isNonInteractive()) { if (!(await deps.promptYesNoOrDefault(" Apply this configuration?", null, true))) { + await deps.recordStepRejected("provider_selection"); deps.log(` Aborted. Re-run \`${deps.cliName()} onboard\` to start over.`); deps.log(" Credentials entered so far were only staged in memory for this run."); deps.log(" No new gateway credential was registered because onboarding stopped here."); - deps.exitProcess(0); + deps.exitProcess(1); } } + // The review acceptance authorizes this fresh selection. Persist it + // before inference setup starts so an interruption in setup can resume + // the accepted provider/model rather than returning to the default menu. + if (shouldRecordProviderSelection && !effectiveResume) { + session = await deps.recordStepComplete( + "provider_selection", + deps.toSessionUpdates({ + provider, + model, + endpointUrl, + credentialEnv, + hermesAuthMethod, + hermesToolGateways, + preferredInferenceApi, + compatibleEndpointReasoning, + compatibleEndpointReasoningEffort, + nimContainer, + stationExpressModelIdentity: vllmModelIdentity, + }), + ); + } + const preparedOllamaProxyToken = await deps.prepareLocalProviderForInference(provider); const inferenceOptions = { gatewayName, allowToolsIncompatible, + ...(preparedOllamaProxyToken ? { preparedOllamaProxyToken } : {}), ...(skipHostInferenceSmoke ? { skipHostInferenceSmoke } : {}), ...(reuseGatewayCredentialWithoutLocalKey ? { reuseGatewayCredentialWithoutLocalKey } : {}), ...(preferredInferenceApi ? { preferredInferenceApi } : {}), @@ -1064,6 +1183,29 @@ export async function handleProviderInferenceState({ ...(healAdjustedInferenceApi ? { preferredInferenceApi } : {}), }), ); + if (completeRecoveredReviewSelectionAfterInference) { + // Provider selection remains in progress until its inference route has + // configured successfully. This retains the selected provider/model for + // interruption recovery without claiming a usable route prematurely. + session = await deps.recordStepComplete( + "provider_selection", + deps.toSessionUpdates({ + provider, + model, + endpointUrl, + credentialEnv, + hermesAuthMethod, + hermesToolGateways, + preferredInferenceApi: healAdjustedInferenceApi + ? initial.preferredInferenceApi + : preferredInferenceApi, + compatibleEndpointReasoning, + compatibleEndpointReasoningEffort, + nimContainer, + stationExpressModelIdentity: vllmModelIdentity, + }), + ); + } break; } diff --git a/src/lib/onboard/machine/handlers/provider-review-recovery.test.ts b/src/lib/onboard/machine/handlers/provider-review-recovery.test.ts new file mode 100644 index 00000000000..c01723e045e --- /dev/null +++ b/src/lib/onboard/machine/handlers/provider-review-recovery.test.ts @@ -0,0 +1,314 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it, vi } from "vitest"; + +import { createSession, type SessionUpdates } from "../../../state/onboard-session"; +import { createProviderReviewDeps } from "../../setup-inference"; +import { handleProviderInferenceState } from "./provider-inference"; +import { baseOptions, baseSelection, createDeps } from "./provider-inference.test-support"; + +describe("provider inference review recovery", () => { + it("rejects configuration review with a non-zero exit before inference setup (#8686)", async () => { + const { deps, calls } = createDeps({ + isNonInteractive: () => false, + promptYesNoOrDefault: vi.fn(async () => false), + }); + + await expect(handleProviderInferenceState(baseOptions(deps))).rejects.toThrow("exit 1"); + + expect(calls.complete).not.toHaveBeenCalledWith("provider_selection", expect.anything()); + expect(calls.checkpointSandboxIdentity).toHaveBeenCalledWith("my-assistant", null); + expect(calls.startStep).toHaveBeenCalledWith("provider_selection", { + provider: "nvidia-prod", + model: "nvidia/test", + }); + expect(calls.rejected).toHaveBeenCalledWith("provider_selection"); + expect(calls.exit).toHaveBeenCalledWith(1); + expect(calls.setupInference).not.toHaveBeenCalled(); + }); + + it("replaces an explicitly rejected review selection on no-TTY resume (#8686)", async () => { + const session = createSession({ + sandboxName: "rejected-review", + provider: "ollama-local", + model: "qwen3.5:9b", + sandboxPromptProgress: { + sandboxName: true, + webSearch: false, + messaging: false, + resourceProfile: false, + }, + }); + session.steps.provider_selection.status = "skipped"; + const { deps, calls } = createDeps({ isInferenceRouteReady: vi.fn(() => false) }); + + const result = await handleProviderInferenceState({ + ...baseOptions(deps, session), + resume: true, + sandboxName: "rejected-review", + }); + + expect(calls.setupNim).toHaveBeenCalled(); + expect(calls.setupInference).toHaveBeenCalled(); + expect(result).toMatchObject({ provider: "nvidia-prod", model: "nvidia/test" }); + expect(calls.setupInference).not.toHaveBeenCalledWith( + "rejected-review", + "qwen3.5:9b", + "ollama-local", + expect.anything(), + expect.anything(), + expect.anything(), + expect.anything(), + expect.anything(), + ); + }); + + it("checkpoints a prompted sandbox identity before interactive review (#8686)", async () => { + const promptYesNoOrDefault = vi.fn(async () => true); + const { deps, calls } = createDeps({ isNonInteractive: () => false, promptYesNoOrDefault }); + + await handleProviderInferenceState(baseOptions(deps)); + + expect(calls.promptName).toHaveBeenCalledWith(null); + expect(calls.checkpointSandboxIdentity).toHaveBeenCalledWith("my-assistant", null); + expect(promptYesNoOrDefault).toHaveBeenCalledWith(" Apply this configuration?", null, true); + expect(calls.checkpointSandboxIdentity.mock.invocationCallOrder[0]).toBeLessThan( + promptYesNoOrDefault.mock.invocationCallOrder[0], + ); + expect(calls.setupInference).toHaveBeenCalled(); + expect(calls.complete).toHaveBeenCalledWith( + "provider_selection", + expect.objectContaining({ provider: "nvidia-prod", model: "nvidia/test" }), + ); + expect(calls.exit).not.toHaveBeenCalled(); + }); + + it("resumes an accepted selection after inference setup throws (#8687)", async () => { + const session = createSession({ sandboxName: "accepted-review" }); + const setupInference = vi + .fn() + .mockRejectedValueOnce(new Error("inference setup failed")) + .mockResolvedValueOnce({ ok: true as const }); + const { deps, calls } = createDeps({ + setupInference, + isInferenceRouteReady: vi.fn(() => false), + promptYesNoOrDefault: vi.fn(async () => true), + }); + calls.complete.mockImplementation(async (...args: unknown[]) => { + const stepName = args[0] as string; + const updates = args[1] as SessionUpdates; + Object.assign(session, updates); + session.steps[stepName].status = "complete"; + return session; + }); + + await expect( + handleProviderInferenceState({ + ...baseOptions(deps, session), + sandboxName: "accepted-review", + }), + ).rejects.toThrow("inference setup failed"); + expect(session.steps.provider_selection.status).toBe("complete"); + expect(session.provider).toBe("nvidia-prod"); + + await handleProviderInferenceState({ + ...baseOptions(deps, session), + resume: true, + sandboxName: "accepted-review", + }); + + expect(calls.setupNim).toHaveBeenCalledTimes(1); + expect(setupInference).toHaveBeenCalledTimes(2); + }); + + it("checkpoints a supplied sandbox identity before review (#8687)", async () => { + const promptYesNoOrDefault = vi.fn(async () => false); + const { deps, calls } = createDeps({ isNonInteractive: () => false, promptYesNoOrDefault }); + + await expect( + handleProviderInferenceState({ ...baseOptions(deps), sandboxName: "supplied-review" }), + ).rejects.toThrow("exit 1"); + + expect(calls.promptName).not.toHaveBeenCalled(); + expect(calls.checkpointSandboxIdentity).toHaveBeenCalledWith("supplied-review", null); + expect(calls.checkpointSandboxIdentity.mock.invocationCallOrder[0]).toBeLessThan( + promptYesNoOrDefault.mock.invocationCallOrder[0], + ); + expect(calls.startStep).toHaveBeenCalledWith("provider_selection", { + provider: "nvidia-prod", + model: "nvidia/test", + }); + expect(calls.prepareLocalProviderForInference).not.toHaveBeenCalled(); + }); + + it("does not prepare the Ollama proxy after interactive review decline (#8687)", async () => { + const startOllamaAuthProxy = vi.fn(() => true); + const getOllamaProxyToken = vi.fn(() => "proxy-token"); + const persistAndProbeOllamaProxy = vi.fn(async () => undefined); + const providerReviewDeps = createProviderReviewDeps( + vi.fn(), + vi.fn(async () => undefined), + { + shouldFrontOllamaWithProxy: () => true, + startOllamaAuthProxy, + getOllamaProxyToken, + persistAndProbeOllamaProxy, + }, + (code): never => { + throw new Error(`exit ${code}`); + }, + vi.fn(), + ); + const { deps, calls } = createDeps({ + isNonInteractive: () => false, + setupNim: vi.fn(async () => ({ + ...baseSelection, + provider: "ollama-local", + model: "qwen3.5:9b", + endpointUrl: "http://127.0.0.1:11435/v1", + credentialEnv: null, + })), + prepareLocalProviderForInference: providerReviewDeps.prepareLocalProviderForInference, + promptYesNoOrDefault: vi.fn(async () => false), + }); + + await expect(handleProviderInferenceState(baseOptions(deps))).rejects.toThrow("exit 1"); + + expect(calls.startStep).toHaveBeenCalledWith("provider_selection", { + provider: "ollama-local", + model: "qwen3.5:9b", + }); + expect(calls.prepareLocalProviderForInference).not.toHaveBeenCalled(); + expect(calls.setupInference).not.toHaveBeenCalled(); + expect(startOllamaAuthProxy).not.toHaveBeenCalled(); + expect(getOllamaProxyToken).not.toHaveBeenCalled(); + expect(persistAndProbeOllamaProxy).not.toHaveBeenCalled(); + }); + + it("prepares the Ollama proxy after review acceptance and before inference setup (#8687)", async () => { + const prepareLocalProviderForInference = vi.fn(async () => "proxy-token"); + const { deps, calls } = createDeps({ + isNonInteractive: () => false, + setupNim: vi.fn(async () => ({ + ...baseSelection, + provider: "ollama-local", + model: "qwen3.5:9b", + endpointUrl: "http://127.0.0.1:11435/v1", + credentialEnv: null, + })), + prepareLocalProviderForInference, + promptYesNoOrDefault: vi.fn(async () => true), + }); + + await handleProviderInferenceState(baseOptions(deps)); + + expect(prepareLocalProviderForInference).toHaveBeenCalledWith("ollama-local"); + expect(prepareLocalProviderForInference.mock.invocationCallOrder[0]).toBeLessThan( + calls.setupInference.mock.invocationCallOrder[0], + ); + expect(calls.setupInference).toHaveBeenCalledWith( + "my-assistant", + "qwen3.5:9b", + "ollama-local", + "http://127.0.0.1:11435/v1", + null, + null, + [], + expect.objectContaining({ preparedOllamaProxyToken: "proxy-token" }), + ); + }); + + it("skips configuration review in explicit non-interactive mode (#8687)", async () => { + const promptYesNoOrDefault = vi.fn(async () => true); + const { deps, calls } = createDeps({ isNonInteractive: () => true, promptYesNoOrDefault }); + + await handleProviderInferenceState(baseOptions(deps)); + + expect(promptYesNoOrDefault).not.toHaveBeenCalled(); + expect(calls.startStep).toHaveBeenCalledWith("provider_selection", { + provider: "nvidia-prod", + model: "nvidia/test", + }); + expect(calls.setupInference).toHaveBeenCalled(); + }); + + function failedReviewSession() { + const session = createSession({ + sandboxName: "review-interrupted", + provider: "ollama-local", + model: "qwen3.5:9b", + sandboxPromptProgress: { + sandboxName: true, + webSearch: false, + messaging: false, + resourceProfile: false, + }, + }); + session.steps.provider_selection.status = "failed"; + return session; + } + + it("prompts for review when interactive resume reuses an interrupted selection (#8687)", async () => { + const session = failedReviewSession(); + const promptYesNoOrDefault = vi.fn(async () => true); + const { deps, calls } = createDeps({ + isNonInteractive: () => false, + promptYesNoOrDefault, + isInferenceRouteReady: vi.fn(() => false), + }); + + await handleProviderInferenceState({ + ...baseOptions(deps, session), + resume: true, + sandboxName: "review-interrupted", + }); + + expect(calls.setupNim).not.toHaveBeenCalled(); + expect(promptYesNoOrDefault).toHaveBeenCalledWith(" Apply this configuration?", null, true); + expect(calls.setupInference).toHaveBeenCalledWith( + "review-interrupted", + "qwen3.5:9b", + "ollama-local", + null, + null, + null, + [], + expect.any(Object), + ); + }); + + it("bypasses review when non-interactive resume reuses an interrupted selection (#8687)", async () => { + const session = failedReviewSession(); + const promptYesNoOrDefault = vi.fn(async () => true); + const { deps, calls } = createDeps({ + isNonInteractive: () => true, + promptYesNoOrDefault, + isInferenceRouteReady: vi.fn(() => false), + }); + + const result = await handleProviderInferenceState({ + ...baseOptions(deps, session), + resume: true, + sandboxName: "review-interrupted", + }); + + expect(calls.setupNim).not.toHaveBeenCalled(); + expect(promptYesNoOrDefault).not.toHaveBeenCalled(); + expect(calls.setupInference).toHaveBeenCalledWith( + "review-interrupted", + "qwen3.5:9b", + "ollama-local", + null, + null, + null, + [], + expect.any(Object), + ); + expect(result).toMatchObject({ + sandboxName: "review-interrupted", + provider: "ollama-local", + model: "qwen3.5:9b", + }); + }); +}); diff --git a/src/lib/onboard/machine/runtime.ts b/src/lib/onboard/machine/runtime.ts index 6f197c9f48f..dab6748b15f 100644 --- a/src/lib/onboard/machine/runtime.ts +++ b/src/lib/onboard/machine/runtime.ts @@ -34,6 +34,7 @@ export interface OnboardRuntimeDeps { markStepStarted(stepName: string): Session; markStepComplete(stepName: string, updates?: SessionUpdates): Session; markStepSkipped(stepName: string): Session; + markStepRejected?(stepName: string): Session; markStepFailed(stepName: string, message?: string | null): Session; completeSession(updates?: SessionUpdates, options?: CompleteSessionOptions): Session; filterSafeUpdates(updates: SessionUpdates): Partial; @@ -76,6 +77,7 @@ function defaultDeps(): OnboardRuntimeDeps { markStepStarted: onboardSession.markStepStarted, markStepComplete: onboardSession.markStepComplete, markStepSkipped: onboardSession.markStepSkipped, + markStepRejected: onboardSession.markStepRejected, markStepFailed: onboardSession.markStepFailed, completeSession: onboardSession.completeSession, filterSafeUpdates: onboardSession.filterSafeUpdates, @@ -169,6 +171,10 @@ export class OnboardRuntime { return updated; } + async markStepRejected(stepName: string): Promise { + return this.deps.markStepRejected?.(stepName) ?? this.deps.markStepSkipped(stepName); + } + async markStepSkipped(stepName: string): Promise { const current = this.ensureSession(); const state = machineStateFromOnboardSessionStep(stepName); diff --git a/src/lib/onboard/resume-config.test.ts b/src/lib/onboard/resume-config.test.ts index 0ecb8b8834a..aa7e2526235 100644 --- a/src/lib/onboard/resume-config.test.ts +++ b/src/lib/onboard/resume-config.test.ts @@ -3,6 +3,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; +import { decisionSelected } from "../state/onboard-checkpoint-decision"; import { normalizeSession } from "../state/onboard-session"; import { getResumeConfigConflicts } from "./resume-config"; @@ -32,6 +33,25 @@ describe("authoritative rebuild resume config", () => { expect(process.env.COMPATIBLE_API_KEY).toBe(""); }); + it("rejects --resume --name that conflicts with an incomplete canonical sandbox identity (#8687)", () => { + expect( + getResumeConfigConflicts( + { + sandboxName: "review-sandbox", + steps: { sandbox: { status: "pending" } }, + checkpoint: { + sandboxIdentity: decisionSelected({ name: "review-sandbox", agent: "openclaw" }), + }, + }, + { sandboxName: "other-sandbox" }, + ), + ).toContainEqual({ + field: "sandbox", + requested: "other-sandbox", + recorded: "review-sandbox", + }); + }); + it("reports an explicit tool-disclosure mismatch against recorded resume state", () => { expect( getResumeConfigConflicts( diff --git a/src/lib/onboard/resume-config.ts b/src/lib/onboard/resume-config.ts index 48f428c5cb7..8c3f0e96c30 100644 --- a/src/lib/onboard/resume-config.ts +++ b/src/lib/onboard/resume-config.ts @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import path from "node:path"; +import { isDecisionSelected } from "../state/onboard-checkpoint-decision"; import { hasInvalidSessionToolDisclosure } from "../state/onboard-session"; import { normalizeToolDisclosure, type ToolDisclosure } from "../tool-disclosure"; import { preflightVllmModelEnvOrExit } from "./vllm-model-preflight"; @@ -17,6 +18,11 @@ export interface ResumeSessionLike { observabilityEnabled?: boolean; metadata?: { fromDockerfile?: string | null } | null; steps?: { sandbox?: { status?: string | null } | null } | null; + checkpoint?: { + sandboxIdentity?: import("../state/onboard-checkpoint-types").CheckpointDecision< + import("../state/onboard-checkpoint-types").CheckpointSandboxIdentity + >; + } | null; } export interface ResumeConfigConflict { @@ -52,8 +58,14 @@ export function getResumeSandboxConflict( // is supplying precisely to recover from the phantom. const raw = typeof opts.sandboxName === "string" ? opts.sandboxName.trim().toLowerCase() : ""; const requestedSandboxName = raw || null; + const checkpointIdentity = session?.checkpoint?.sandboxIdentity; + const checkpointedSandboxName = + checkpointIdentity && isDecisionSelected(checkpointIdentity) + ? checkpointIdentity.value.name + : null; const recordedSandboxName = - session?.steps?.sandbox?.status === "complete" ? (session?.sandboxName ?? null) : null; + checkpointedSandboxName ?? + (session?.steps?.sandbox?.status === "complete" ? (session?.sandboxName ?? null) : null); if (!requestedSandboxName || !recordedSandboxName) { return null; } diff --git a/src/lib/onboard/runtime-boundary.test.ts b/src/lib/onboard/runtime-boundary.test.ts index 04b0755e232..4948e226319 100644 --- a/src/lib/onboard/runtime-boundary.test.ts +++ b/src/lib/onboard/runtime-boundary.test.ts @@ -197,6 +197,32 @@ describe("OnboardRuntimeBoundary", () => { }); }); + it("persists review-stage selection context without completing provider selection (#8686)", async () => { + const harness = createRuntimeHarness(); + const boundary = new OnboardRuntimeBoundary({ + toSessionUpdates: (updates) => filterSafeUpdates(updates as SessionUpdates) as SessionUpdates, + maybeForceE2eStepFailure: () => undefined, + createRuntime: harness.createRuntime, + }); + + await boundary.startRecordedStep("provider_selection", { + sandboxName: "review-interrupted", + provider: "ollama-local", + model: "qwen3.5:9b", + }); + await boundary.recordStepFailed( + "provider_selection", + "Onboarding exited before the step completed.", + ); + + expect(harness.getSession()).toMatchObject({ + sandboxName: "review-interrupted", + provider: "ollama-local", + model: "qwen3.5:9b", + steps: { provider_selection: { status: "failed" } }, + }); + }); + it("applies each explicit transition exactly once", async () => { const harness = createRuntimeHarness(); const boundary = new OnboardRuntimeBoundary({ diff --git a/src/lib/onboard/runtime-boundary.ts b/src/lib/onboard/runtime-boundary.ts index 18c9c8443da..3453a3ce694 100644 --- a/src/lib/onboard/runtime-boundary.ts +++ b/src/lib/onboard/runtime-boundary.ts @@ -46,6 +46,7 @@ export class OnboardRuntimeBoundary { recordOnboardStarted: this.recordOnboardStarted.bind(this), startRecordedStep: this.startRecordedStep.bind(this), recordStepComplete: this.recordStepComplete.bind(this), + recordStepRejected: this.recordStepRejected.bind(this), recordStepSkipped: this.recordStepSkipped.bind(this), recordStateSkipped: this.recordStateSkipped.bind(this), recordRepairEvent: this.recordRepairEvent.bind(this), @@ -87,6 +88,10 @@ export class OnboardRuntimeBoundary { return this.getRuntime().markStepComplete(stepName, updates); } + async recordStepRejected(stepName: string): Promise { + return this.getRuntime().markStepRejected(stepName); + } + async recordStepSkipped(stepName: string): Promise { return this.getRuntime().markStepSkipped(stepName); } diff --git a/src/lib/onboard/sandbox-agent.test.ts b/src/lib/onboard/sandbox-agent.test.ts index 211e57538bd..8c8e53511cb 100644 --- a/src/lib/onboard/sandbox-agent.test.ts +++ b/src/lib/onboard/sandbox-agent.test.ts @@ -5,8 +5,16 @@ import { describe, expect, it, vi } from "vitest"; import { createPromptValidatedSandboxName } from "./sandbox-agent"; describe("sandbox name prompt", () => { - it("checkpoints a validated name before returning it to onboarding (#6743)", async () => { - const checkpointSandboxName = vi.fn(); + it("waits for a validated-name checkpoint before returning it to onboarding (#8687)", async () => { + let release: (() => void) | undefined; + const checkpointStarted = new Promise((resolve) => { + release = resolve; + }); + let persisted = false; + const checkpointSandboxName = vi.fn(async () => { + await checkpointStarted; + persisted = true; + }); const promptValidatedSandboxName = createPromptValidatedSandboxName({ promptOrDefault: vi.fn(async () => "tm"), cliDisplayName: () => "NemoClaw", @@ -17,7 +25,12 @@ describe("sandbox name prompt", () => { }, }); - await expect(promptValidatedSandboxName()).resolves.toBe("tm"); + const result = promptValidatedSandboxName(); + await Promise.resolve(); + expect(persisted).toBe(false); + release?.(); + await expect(result).resolves.toBe("tm"); + expect(persisted).toBe(true); expect(checkpointSandboxName).toHaveBeenCalledWith("tm", null); }); @@ -28,7 +41,7 @@ describe("sandbox name prompt", () => { promptOrDefault, cliDisplayName: () => "NemoClaw", isNonInteractive: () => false, - checkpointSandboxName: () => { + checkpointSandboxName: async () => { throw checkpointError; }, exit: (code) => { diff --git a/src/lib/onboard/sandbox-agent.ts b/src/lib/onboard/sandbox-agent.ts index 98f6e50011b..91b9ffbbc46 100644 --- a/src/lib/onboard/sandbox-agent.ts +++ b/src/lib/onboard/sandbox-agent.ts @@ -144,7 +144,7 @@ export interface PromptSandboxNameDeps { promptOrDefault(question: string, envVar: string, defaultValue: string): Promise; cliDisplayName(): string; isNonInteractive(): boolean; - checkpointSandboxName(sandboxName: string, agent: AgentDefinition | null): void; + checkpointSandboxName(sandboxName: string, agent: AgentDefinition | null): Promise; exit(code: number): never; } @@ -198,7 +198,7 @@ export function createPromptValidatedSandboxName(deps: PromptSandboxNameDeps) { continue; } - deps.checkpointSandboxName(validatedSandboxName, agent); + await deps.checkpointSandboxName(validatedSandboxName, agent); return validatedSandboxName; } diff --git a/src/lib/onboard/session-bootstrap.test.ts b/src/lib/onboard/session-bootstrap.test.ts index 1db5862981d..2e80497fec8 100644 --- a/src/lib/onboard/session-bootstrap.test.ts +++ b/src/lib/onboard/session-bootstrap.test.ts @@ -11,7 +11,11 @@ import { } from "../state/onboard-checkpoint-types"; import { createSession, type Session, type SessionRecoveryReceipt } from "../state/onboard-session"; import type { ResumeConfigConflict } from "./resume-config"; -import { type OnboardSessionBootstrapDeps, prepareOnboardSession } from "./session-bootstrap"; +import { + checkpointSandboxName, + type OnboardSessionBootstrapDeps, + prepareOnboardSession, +} from "./session-bootstrap"; class ExitError extends Error { constructor(readonly code: number) { @@ -431,6 +435,104 @@ describe("prepareOnboardSession", () => { expect(deps.exitProcess).not.toHaveBeenCalled(); }); + it("allows no-TTY resume after review-stage identity persistence (#8687)", async () => { + const session = createSession({ + provider: "ollama-local", + model: "qwen3.5:9b", + status: "failed", + }); + await checkpointSandboxName("review-interrupted", { name: "openclaw" }, (mutator) => { + return mutator(session) ?? session; + }); + session.steps.provider_selection.status = "failed"; + const { deps } = createDeps(session); + + const result = await prepareOnboardSession( + { + resume: true, + fresh: false, + requestedFromDockerfile: null, + requestedSandboxName: null, + cannotPrompt: true, + nonInteractive: true, + }, + deps, + ); + + expect(result.session).toMatchObject({ + sandboxName: "review-interrupted", + provider: "ollama-local", + model: "qwen3.5:9b", + checkpoint: { + sandboxIdentity: decisionSelected({ name: "review-interrupted", agent: "openclaw" }), + }, + }); + expect(deps.exitProcess).not.toHaveBeenCalled(); + }); + + it("persists Hermes review identity for no-TTY resume (#8687)", async () => { + const session = createSession({ agent: "hermes", status: "failed" }); + await checkpointSandboxName("hermes-review", { name: "hermes" }, (mutator) => { + return mutator(session) ?? session; + }); + session.steps.provider_selection.status = "failed"; + const { deps } = createDeps(session); + + const result = await prepareOnboardSession( + { + resume: true, + fresh: false, + requestedFromDockerfile: null, + requestedSandboxName: null, + cannotPrompt: true, + nonInteractive: true, + }, + deps, + ); + + expect(result.session).toMatchObject({ + sandboxName: "hermes-review", + checkpoint: { + sandboxIdentity: decisionSelected({ name: "hermes-review", agent: "hermes" }), + }, + }); + expect(deps.exitProcess).not.toHaveBeenCalled(); + }); + + it("waits for canonical sandbox identity persistence before returning (#8687)", async () => { + const session = createSession(); + let release: (() => void) | undefined; + const writeStarted = new Promise((resolve) => { + release = resolve; + }); + let completed = false; + const checkpoint = checkpointSandboxName( + "review-race", + { name: "openclaw" }, + async (mutator) => { + await writeStarted; + const next = mutator(session) ?? session; + completed = true; + return next; + }, + ); + + await Promise.resolve(); + expect(completed).toBe(false); + expect(session.checkpoint).toBeNull(); + release?.(); + await checkpoint; + + expect(completed).toBe(true); + expect(session).toMatchObject({ + sandboxName: "review-race", + sandboxPromptProgress: { sandboxName: true }, + checkpoint: { + sandboxIdentity: decisionSelected({ name: "review-race", agent: "openclaw" }), + }, + }); + }); + it("recovers a non-OpenClaw checkpointed sandbox name after a crash before the legacy field was written (#7022)", async () => { const session = createSession({ agent: "hermes", sandboxName: null }); const checkpoint: OnboardCheckpoint = { diff --git a/src/lib/onboard/session-bootstrap.ts b/src/lib/onboard/session-bootstrap.ts index 3eada1aa070..fcbdaa5d5d8 100644 --- a/src/lib/onboard/session-bootstrap.ts +++ b/src/lib/onboard/session-bootstrap.ts @@ -33,7 +33,7 @@ export interface OnboardSessionBootstrapDeps { clearSession(): void; createSession(overrides?: Partial): Session; saveSession(session: Session): Session; - updateSession(mutator: (session: Session) => Session | void): Session; + updateSession(mutator: (session: Session) => Session | void): Session | Promise; applySessionRecovery(session: Session): void; setOnboardBrandingAgent(agentName: string | null): void; getResumeConfigConflicts( @@ -63,20 +63,16 @@ export interface OnboardSessionBootstrapResult { export const defaultResolveResumeCheckpoint: () => CheckpointLoadResult = loadResumeCheckpoint; -export function checkpointSandboxName( +export async function checkpointSandboxName( sandboxName: string, agent: { name?: string } | null, updateSession: OnboardSessionBootstrapDeps["updateSession"], -): void { - if (agent?.name && agent.name !== "openclaw") return; - updateSession((current) => { +): Promise { + await updateSession((current) => { + const checkpointAgent = agent?.name ?? current.agent ?? "openclaw"; current.sandboxName = sandboxName; current.sandboxPromptProgress.sandboxName = true; - recordCheckpointSandboxIdentity( - current, - sandboxName, - current.agent ?? agent?.name ?? "openclaw", - ); + recordCheckpointSandboxIdentity(current, sandboxName, checkpointAgent); return current; }); } @@ -92,10 +88,7 @@ export function getCheckpointedSandboxName( ? session.checkpoint.sandboxIdentity.value.name : null; } - return (!agent?.name || agent.name === "openclaw") && - session?.sandboxPromptProgress?.sandboxName === true - ? session.sandboxName - : null; + return session?.sandboxPromptProgress?.sandboxName === true ? session.sandboxName : null; } function mode(nonInteractive: boolean): "non-interactive" | "interactive" { @@ -212,8 +205,7 @@ function assertRecoverableResumeSandboxName( const nameRecoverable = checkpoint ? checkpointProvesSandboxStepComplete(session) || isDecisionSelected(checkpoint.sandboxIdentity) : session?.steps?.sandbox?.status === "complete" || - ((!session?.agent || session.agent === "openclaw") && - session?.sandboxPromptProgress?.sandboxName === true); + session?.sandboxPromptProgress?.sandboxName === true; const checkpointedSandboxName = checkpoint && isDecisionSelected(checkpoint.sandboxIdentity) ? checkpoint.sandboxIdentity.value.name diff --git a/src/lib/onboard/setup-inference.test.ts b/src/lib/onboard/setup-inference.test.ts new file mode 100644 index 00000000000..db604cee47e --- /dev/null +++ b/src/lib/onboard/setup-inference.test.ts @@ -0,0 +1,176 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it, vi } from "vitest"; + +import { setupOllamaLocalInference } from "./inference-providers/ollama-local"; +import { createProviderReviewDeps } from "./setup-inference"; + +describe("createProviderReviewDeps", () => { + it("prepares the Ollama proxy after review acceptance", async () => { + const updateSession = vi.fn(); + const checkpointSandboxName = vi.fn(async () => undefined); + const startOllamaAuthProxy = vi.fn(() => true); + const persistAndProbeOllamaProxy = vi.fn(async () => undefined); + const exitProcess = vi.fn((code: number): never => { + throw new Error(`exit ${code}`); + }); + const getOllamaProxyToken = vi.fn(() => "proxy-token"); + const deps = createProviderReviewDeps( + updateSession, + checkpointSandboxName, + { + shouldFrontOllamaWithProxy: () => true, + startOllamaAuthProxy, + getOllamaProxyToken, + persistAndProbeOllamaProxy, + }, + exitProcess, + vi.fn(), + ); + + const preparedToken = await deps.prepareLocalProviderForInference("ollama-local"); + + expect(startOllamaAuthProxy).toHaveBeenCalledOnce(); + expect(getOllamaProxyToken).toHaveBeenCalledOnce(); + expect(persistAndProbeOllamaProxy).toHaveBeenCalledWith("proxy-token"); + expect(preparedToken).toBe("proxy-token"); + }); + + it("does not mutate local provider state for another provider", async () => { + const startOllamaAuthProxy = vi.fn(() => true); + const persistAndProbeOllamaProxy = vi.fn(async () => undefined); + const deps = createProviderReviewDeps( + vi.fn(), + vi.fn(async () => undefined), + { + shouldFrontOllamaWithProxy: () => true, + startOllamaAuthProxy, + getOllamaProxyToken: () => "proxy-token", + persistAndProbeOllamaProxy, + }, + (code): never => { + throw new Error(`exit ${code}`); + }, + vi.fn(), + ); + + await expect(deps.prepareLocalProviderForInference("nvidia-prod")).resolves.toBeNull(); + + expect(startOllamaAuthProxy).not.toHaveBeenCalled(); + expect(persistAndProbeOllamaProxy).not.toHaveBeenCalled(); + }); + + it("exits without persisting when the Ollama proxy cannot start", async () => { + const persistAndProbeOllamaProxy = vi.fn(async () => undefined); + const exitProcess = vi.fn((code: number): never => { + throw new Error(`exit ${code}`); + }); + const deps = createProviderReviewDeps( + vi.fn(), + vi.fn(async () => undefined), + { + shouldFrontOllamaWithProxy: () => true, + startOllamaAuthProxy: () => false, + getOllamaProxyToken: () => "proxy-token", + persistAndProbeOllamaProxy, + }, + exitProcess, + vi.fn(), + ); + + await expect(deps.prepareLocalProviderForInference("ollama-local")).rejects.toThrow("exit 1"); + + expect(exitProcess).toHaveBeenCalledWith(1); + expect(persistAndProbeOllamaProxy).not.toHaveBeenCalled(); + }); + + it("exits without persisting when the Ollama proxy token is unavailable", async () => { + const persistAndProbeOllamaProxy = vi.fn(async () => undefined); + const exitProcess = vi.fn((code: number): never => { + throw new Error(`exit ${code}`); + }); + const writeError = vi.fn(); + const deps = createProviderReviewDeps( + vi.fn(), + vi.fn(async () => undefined), + { + shouldFrontOllamaWithProxy: () => true, + startOllamaAuthProxy: () => true, + getOllamaProxyToken: () => null, + persistAndProbeOllamaProxy, + }, + exitProcess, + writeError, + ); + + await expect(deps.prepareLocalProviderForInference("ollama-local")).rejects.toThrow("exit 1"); + + expect(writeError).toHaveBeenCalledWith(expect.stringContaining("proxy token is not set")); + expect(exitProcess).toHaveBeenCalledWith(1); + expect(persistAndProbeOllamaProxy).not.toHaveBeenCalled(); + }); + + it("hands the accepted proxy token to provider setup without repeating proxy mutations", async () => { + const startOllamaAuthProxy = vi.fn(() => true); + const getOllamaProxyToken = vi.fn(() => "proxy-token"); + const persistAndProbeOllamaProxy = vi.fn(async () => undefined); + const reviewDeps = createProviderReviewDeps( + vi.fn(), + vi.fn(async () => undefined), + { + shouldFrontOllamaWithProxy: () => true, + startOllamaAuthProxy, + getOllamaProxyToken, + persistAndProbeOllamaProxy, + }, + (code): never => { + throw new Error(`exit ${code}`); + }, + vi.fn(), + ); + const preparedProxyToken = await reviewDeps.prepareLocalProviderForInference("ollama-local"); + const ensureOllamaAuthProxy = vi.fn(); + + await setupOllamaLocalInference( + { + model: "qwen3.5:9b", + provider: "ollama-local", + allowToolsIncompatible: false, + preparedProxyToken: preparedProxyToken ?? undefined, + }, + { + runOpenshell: () => ({ status: 0 }), + upsertProvider: () => ({ ok: true }), + verifyInferenceRoute: vi.fn(), + verifyOnboardInferenceSmoke: vi.fn(), + isNonInteractive: () => true, + registry: { updateSandbox: vi.fn() as never }, + exitProcess: (code): never => { + throw new Error(`exit ${code}`); + }, + error: vi.fn(), + log: vi.fn(), + validateLocalProvider: () => ({ ok: true }), + getLocalProviderBaseUrl: () => "http://host.openshell.internal:11435/v1", + applyLocalInferenceRoute: async () => false, + getOllamaWarmupCommand: () => ["ollama", "run", "qwen3.5:9b"], + run: vi.fn() as never, + shouldFrontOllamaWithProxy: () => true, + ensureOllamaAuthProxy, + isProxyHealthy: () => true, + getOllamaProxyToken, + persistAndProbeOllamaProxy, + localInference: { + validateOllamaModelWithToolsOverride: () => ({ ok: true }), + }, + OLLAMA_PROXY_CREDENTIAL_ENV: "NEMOCLAW_OLLAMA_PROXY_TOKEN", + }, + ); + + expect(startOllamaAuthProxy).toHaveBeenCalledOnce(); + expect(getOllamaProxyToken).toHaveBeenCalledOnce(); + expect(persistAndProbeOllamaProxy).toHaveBeenCalledOnce(); + expect(ensureOllamaAuthProxy).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/onboard/setup-inference.ts b/src/lib/onboard/setup-inference.ts index fdd769e98ff..f73aa96e8aa 100644 --- a/src/lib/onboard/setup-inference.ts +++ b/src/lib/onboard/setup-inference.ts @@ -22,9 +22,45 @@ import { type OpenShellGatewayEndpointEnvironment, } from "../openshell-gateway-endpoint-guard"; import { withSandboxMutationLock } from "../state/mcp-lifecycle-lock"; +import type { Session } from "../state/onboard-session"; export { assertNoOpenShellGatewayEndpointOverride }; +export function createProviderReviewDeps( + updateSession: (mutator: (session: Session) => Session | void) => Session | Promise, + checkpointSandboxName: ( + sandboxName: string, + agent: { name?: string } | null, + updateSession: (mutator: (session: Session) => Session | void) => Session | Promise, + ) => Promise, + localProvider: { + shouldFrontOllamaWithProxy: () => boolean; + startOllamaAuthProxy: () => boolean; + getOllamaProxyToken: () => string | null; + persistAndProbeOllamaProxy: (token: string) => Promise; + }, + exitProcess: (code: number) => never, + writeError: (message: string) => void, +) { + return { + checkpointSandboxIdentity: (sandboxName: string, agent: { name?: string } | null) => + checkpointSandboxName(sandboxName, agent, updateSession), + prepareLocalProviderForInference: async (providerName: string) => { + if (providerName !== "ollama-local" || !localProvider.shouldFrontOllamaWithProxy()) { + return null; + } + if (!localProvider.startOllamaAuthProxy()) exitProcess(1); + const proxyToken = localProvider.getOllamaProxyToken(); + if (!proxyToken) { + writeError(" Ollama auth proxy token is not set. Re-run onboard to initialize the proxy."); + exitProcess(1); + } + await localProvider.persistAndProbeOllamaProxy(proxyToken); + return proxyToken; + }, + }; +} + import type { HermesAuthMethod } from "./hermes-auth"; function matchesOnboardEndpoint( @@ -451,7 +487,12 @@ export function createSetupInference( if (outcome.done) return outcome.result; } else if (provider === "ollama-local") { const outcome = await inferenceProviders.setupOllamaLocalInference( - { model, provider, allowToolsIncompatible: options.allowToolsIncompatible === true }, + { + model, + provider, + allowToolsIncompatible: options.allowToolsIncompatible === true, + preparedProxyToken: options.preparedOllamaProxyToken, + }, { ...commonDeps, validateLocalProvider: deps.validateLocalProvider, diff --git a/src/lib/onboard/setup-nim-ollama.test.ts b/src/lib/onboard/setup-nim-ollama.test.ts index 98d1c2e43c1..1d7199bd69c 100644 --- a/src/lib/onboard/setup-nim-ollama.test.ts +++ b/src/lib/onboard/setup-nim-ollama.test.ts @@ -41,7 +41,6 @@ function makeDeps(overrides: Partial = {}): Deps { ensureOllamaLoopbackSystemdOverride: () => "unchanged", runOllamaStartupOrGate: () => ({ kind: "ready" }), shouldFrontOllamaWithProxy: () => false, - startOllamaAuthProxy: () => true, getLocalProviderBaseUrl: () => "http://127.0.0.1:11434/v1", selectAndValidateOllamaModel: async () => ({ outcome: "selected", @@ -361,10 +360,9 @@ describe("createSetupNimOllamaHandlers", () => { assert.equal(state.allowToolsIncompatible, true); }); - it("fronts installed WSL-local Ollama with the sandbox proxy when host loopback is not container-reachable (#7318)", async () => { + it("selects the proxy route without starting it before configuration review (#7318)", async () => { const state = makeState(); const install = vi.fn(() => ({ ok: true })); - const startProxy = vi.fn(() => true); const selectModel = vi.fn(async () => ({ outcome: "selected", model: "qwen3:0.6b", @@ -376,7 +374,6 @@ describe("createSetupNimOllamaHandlers", () => { process: { ...process, platform: "linux" } as NodeJS.Process, installOllamaOnLinux: install, shouldFrontOllamaWithProxy: () => true, - startOllamaAuthProxy: startProxy, getLocalProviderBaseUrl: () => "http://host.openshell.internal:11435/v1", selectAndValidateOllamaModel: selectModel, }), @@ -388,7 +385,6 @@ describe("createSetupNimOllamaHandlers", () => { expect(result).toBe("selected"); expect(install).toHaveBeenCalledTimes(1); - expect(startProxy).toHaveBeenCalledTimes(1); expect(state).toMatchObject({ provider: "ollama-local", endpointUrl: "http://host.openshell.internal:11435/v1", @@ -407,7 +403,6 @@ describe("createSetupNimOllamaHandlers", () => { const exit = vi.fn((code?: number) => { throw new Error(`exit ${code}`); }); - const startProxy = vi.fn(() => true); const selectModel = vi.fn(async () => ({ outcome: "selected" as const, model: "should-not-run", @@ -417,7 +412,6 @@ describe("createSetupNimOllamaHandlers", () => { makeDeps({ process: { ...process, exit: exit as never }, runOllamaStartupOrGate: () => ({ kind: "mystery" }) as never, - startOllamaAuthProxy: startProxy, selectAndValidateOllamaModel: selectModel, }), ); @@ -429,7 +423,6 @@ describe("createSetupNimOllamaHandlers", () => { assert.deepEqual(state, before); assert.equal(exit.mock.calls[0]?.[0], 1); - assert.equal(startProxy.mock.calls.length, 0); assert.equal(selectModel.mock.calls.length, 0); }); @@ -442,7 +435,6 @@ describe("createSetupNimOllamaHandlers", () => { state.preferredInferenceApi = "responses"; state.nimContainer = "stale-nim"; state.allowToolsIncompatible = true; - const startProxy = vi.fn(() => true); const selectModel = vi.fn(async () => ({ outcome: "selected" as const, model: "should-not-run", @@ -460,7 +452,6 @@ describe("createSetupNimOllamaHandlers", () => { preferredInferenceApi: "openai-completions", }, }), - startOllamaAuthProxy: startProxy, selectAndValidateOllamaModel: selectModel, }), ); @@ -480,7 +471,6 @@ describe("createSetupNimOllamaHandlers", () => { allowToolsIncompatible: false, skipHostInferenceSmoke: false, }); - assert.equal(startProxy.mock.calls.length, 0); assert.equal(selectModel.mock.calls.length, 0); }); }); diff --git a/src/lib/onboard/setup-nim-ollama.ts b/src/lib/onboard/setup-nim-ollama.ts index be7b5c620ed..dbac3f610d8 100644 --- a/src/lib/onboard/setup-nim-ollama.ts +++ b/src/lib/onboard/setup-nim-ollama.ts @@ -31,7 +31,6 @@ type SetupNimOllamaDeps = { contextWindowFloor?: number; }) => OllamaStartupOutcome; shouldFrontOllamaWithProxy: () => boolean; - startOllamaAuthProxy: () => boolean; getLocalProviderBaseUrl: (provider: string) => string | null; selectAndValidateOllamaModel: ( gpu: any, @@ -134,9 +133,8 @@ export function createSetupNimOllamaHandlers(deps: SetupNimOllamaDeps): { return "selected"; } - function startProxyOrAnnounceDirect(): void { + function announceOllamaRoute(): void { if (deps.shouldFrontOllamaWithProxy()) { - if (!deps.startOllamaAuthProxy()) deps.process.exit(1); console.log( ` ✓ Using Ollama on localhost:${deps.OLLAMA_PORT} (proxy on :${deps.OLLAMA_PROXY_PORT})`, ); @@ -306,7 +304,7 @@ export function createSetupNimOllamaHandlers(deps: SetupNimOllamaDeps): { state.assertRouteCompatible?.(); return "selected"; case "ready": - startProxyOrAnnounceDirect(); + announceOllamaRoute(); return selectModel(gpu, state, requestedModel, recoveredModel, lockedModel); default: { const kind = (startup as { kind?: unknown }).kind; @@ -352,7 +350,7 @@ export function createSetupNimOllamaHandlers(deps: SetupNimOllamaDeps): { if (deps.isNonInteractive()) deps.process.exit(1); return "retry-selection"; } - startProxyOrAnnounceDirect(); + announceOllamaRoute(); return selectModel(gpu, state, requestedModel, recoveredModel, lockedModel); } diff --git a/src/lib/state/onboard-session.test.ts b/src/lib/state/onboard-session.test.ts index 10ed03abf43..e9665da9992 100644 --- a/src/lib/state/onboard-session.test.ts +++ b/src/lib/state/onboard-session.test.ts @@ -9,6 +9,7 @@ import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { makeMessagingPlan } from "../../../test/helpers/messaging-plan-fixtures"; +import { decisionSelected } from "./onboard-checkpoint-decision"; const require = createRequire(import.meta.url); const distPath = require.resolve("./onboard-session"); @@ -252,6 +253,59 @@ describe("onboard session", () => { expect(loaded.machine).toMatchObject({ state: "init", revision: 0 }); }); + it("clears provider selection authority when a review is rejected", () => { + session.saveSession( + session.createSession({ + provider: "ollama-local", + model: "qwen3.5:9b", + endpointUrl: "http://127.0.0.1:11435/v1", + credentialEnv: "NEMOCLAW_OLLAMA_PROXY_TOKEN", + sandboxName: "rejected-review", + sandboxPromptProgress: { + sandboxName: true, + webSearch: false, + messaging: false, + resourceProfile: false, + }, + }), + ); + session.markStepStarted("provider_selection"); + session.updateSession((current) => { + current.checkpoint = { + schemaVersion: 3, + sessionId: current.sessionId, + machineState: "init", + updatedAt: new Date().toISOString(), + sandboxIdentity: decisionSelected({ name: "rejected-review", agent: "openclaw" }), + webSearch: { kind: "unset" }, + messaging: { kind: "unset" }, + resourceProfile: { kind: "unset" }, + gatewayAuthority: { kind: "unset" }, + effectGroups: {}, + bindings: { credentialEnvs: [], registeredProviders: [] }, + sandboxRecreate: null, + }; + return current; + }); + + const rejected = session.markStepRejected("provider_selection"); + + expect(rejected).toMatchObject({ + provider: null, + model: null, + endpointUrl: null, + credentialEnv: null, + sandboxName: null, + sandboxPromptProgress: { sandboxName: false }, + lastStepStarted: null, + resumable: false, + status: "failed", + failure: null, + steps: { provider_selection: { status: "skipped" } }, + checkpoint: { sandboxIdentity: { kind: "unset" } }, + }); + }); + it("can record step boundaries without mutating the machine snapshot", () => { const emitted: OnboardMachineEvent[] = []; machineEvents.addOnboardMachineEventListener((event) => emitted.push(event)); diff --git a/src/lib/state/onboard-session.ts b/src/lib/state/onboard-session.ts index acb9732dae3..7c2cfb37641 100644 --- a/src/lib/state/onboard-session.ts +++ b/src/lib/state/onboard-session.ts @@ -46,6 +46,7 @@ import { } from "../onboard/station-express-resume"; import { redactSensitiveText, redactUrl } from "../security/redact"; import { inspectCheckpoint, serializeCheckpoint } from "./onboard-checkpoint"; +import { decisionUnset } from "./onboard-checkpoint-decision"; import type { OnboardCheckpoint } from "./onboard-checkpoint-types"; import { assignSafeToolDisclosureUpdate, @@ -1462,6 +1463,44 @@ export function markStepSkipped(stepName: string): Session { step.startedAt = null; step.completedAt = null; step.error = null; + if (session.lastStepStarted === stepName) session.lastStepStarted = null; + return session; + }); +} + +export function markStepRejected(stepName: string): Session { + return updateSession((session) => { + const step = session.steps[stepName]; + if (!step) return session; + step.status = "skipped"; + step.startedAt = null; + step.completedAt = null; + step.error = null; + if (session.lastStepStarted === stepName) session.lastStepStarted = null; + if (stepName === "provider_selection") { + session.provider = null; + session.model = null; + session.endpointUrl = null; + session.credentialEnv = null; + session.hermesAuthMethod = null; + session.preferredInferenceApi = null; + session.compatibleEndpointReasoning = null; + session.compatibleEndpointReasoningEffort = null; + session.nimContainer = null; + session.hermesToolGateways = null; + session.sandboxName = null; + session.sandboxPromptProgress.sandboxName = false; + session.resumable = false; + session.status = "failed"; + session.failure = null; + if (session.checkpoint) { + session.checkpoint = { + ...session.checkpoint, + sandboxIdentity: decisionUnset(), + updatedAt: new Date().toISOString(), + }; + } + } return session; }); } diff --git a/test/onboard-inference-reconciliation.test.ts b/test/onboard-inference-reconciliation.test.ts index 16b991d34da..36409d35725 100644 --- a/test/onboard-inference-reconciliation.test.ts +++ b/test/onboard-inference-reconciliation.test.ts @@ -423,8 +423,8 @@ const { onboard } = require(${onboardPath}); assert.ok(!payload.commands.some((entry) => /provider (create|update)/.test(entry.command))); assert.equal( payload.inferenceSessionSandboxName, - null, - "resume inference must not persist sandboxName before sandbox creation", + "hermes-resume", + "resume inference persists the canonical sandbox identity before sandbox creation", ); assert.ok( payload.registryUpdates.some( diff --git a/test/onboard-ollama-autostart.test.ts b/test/onboard-ollama-autostart.test.ts index a627af8421e..3e17d29f1a4 100644 --- a/test/onboard-ollama-autostart.test.ts +++ b/test/onboard-ollama-autostart.test.ts @@ -28,9 +28,9 @@ type ScenarioOptions = { // When true, stub waitForHttp to return false. Only used to verify that the // gated path does not even reach waitForHttp. waitForHttpReturnsFalse?: boolean; - // When true, allow the wizard to reach selectAndValidateOllamaModel by - // stubbing startOllamaAuthProxy to a no-op success rather than the bail-out - // sentinel. Used by the #4365 runner-crash escape scenarios. + // When true, allow the wizard to reach selectAndValidateOllamaModel rather + // than stopping at its prompt boundary. Used by the #4365 runner-crash + // escape scenarios. proceedToModelSelection?: boolean; // Body returned by the fake curl for `/api/generate` probes (used by // validateOllamaModel). Defaults to a healthy response. Set to a runner- @@ -235,20 +235,20 @@ localInference.resetOllamaHostCache(); // onboard require() below. localInference.findReachableOllamaHost = () => (ollamaRunning ? "127.0.0.1" : null); -// Sentinel: startOllamaAuthProxy is called downstream of the Ollama branch -// (after either the spawn path or the "already running" path). Throwing a -// sentinel here bails out of the wizard once it has done everything that -// matters for the gated-vs-spawn assertions. The fallback branch breaks out -// of selectionLoop BEFORE this is reached, so Scenarios A and D never see -// the sentinel — only B and C do. The #4365 scenarios opt out so the wizard -// can reach selectAndValidateOllamaModel. +// Keep the proxy process inert. Proxy preparation now occurs only after the +// configuration review is accepted, outside setupNim's selection boundary. const proxy = require(${proxyPath}); class OllamaAutostartSentinel extends Error {} const proceedToModelSelection = ${JSON.stringify(opts.proceedToModelSelection === true)}; -if (proceedToModelSelection) { - proxy.startOllamaAuthProxy = () => true; -} else { - proxy.startOllamaAuthProxy = () => { +proxy.startOllamaAuthProxy = () => true; + +// promptOllamaModel is reached after either the spawn path or the +// already-running path. Throwing here stops the wizard once it has done +// everything needed for the gated-vs-spawn assertions. The fallback branch +// exits selectionLoop before this boundary, so Scenarios A and D do not see +// the sentinel. The #4365 scenarios continue into model validation. +if (!proceedToModelSelection) { + proxy.promptOllamaModel = () => { throw new OllamaAutostartSentinel("ollama-autostart-test-sentinel"); }; } @@ -394,13 +394,9 @@ describe("nemoclaw onboard --no-ollama-autostart (#3751)", () => { ); // selectAndValidateOllamaModel is intentionally bypassed. assert.equal(payload.selectAndValidateOllamaModelCalled, false); - // The fallback `break` exits selectionLoop BEFORE startOllamaAuthProxy is - // reached — sentinel must not have tripped. - assert.equal( - payload.sentinelTripped, - false, - "gated fallback must not reach startOllamaAuthProxy", - ); + // The fallback `break` exits selectionLoop before model selection, so the + // sentinel must not have tripped. + assert.equal(payload.sentinelTripped, false, "gated fallback must not reach model selection"); }); it("preserves the existing spawn path for stopped Ollama without the flag in scenario B", { @@ -427,12 +423,11 @@ describe("nemoclaw onboard --no-ollama-autostart (#3751)", () => { "gate warning must not fire when the flag is unset", ); // Sentinel tripped — proves the wizard exited the !ollamaReady block via - // the spawn-then-proxy path (i.e. moved on to startOllamaAuthProxy), NOT - // via the gated `break` that fallback uses. + // the spawn path and reached model selection, not the gated fallback. assert.equal( payload.sentinelTripped, true, - `expected wizard to reach the post-spawn proxy step; lines:\n${payload.lines.join("\n")}`, + `expected wizard to reach model selection after spawning; lines:\n${payload.lines.join("\n")}`, ); }); @@ -460,8 +455,8 @@ describe("nemoclaw onboard --no-ollama-autostart (#3751)", () => { !payload.lines.some((line) => line.includes("--no-ollama-autostart is set")), "gate warning must not fire when daemon is already up", ); - // Wizard should have reached the proxy step (post-readiness), not the - // gated `break` path. + // Wizard should have reached model selection after readiness, not the + // gated fallback path. assert.equal(payload.sentinelTripped, true); }); @@ -481,8 +476,8 @@ describe("nemoclaw onboard --no-ollama-autostart (#3751)", () => { !payload.lines.some((line) => line.includes("--no-ollama-autostart is set")), "gate warning must not fire when daemon is already up — flag is orthogonal", ); - // Flag is irrelevant here: the wizard still proceeds via the proxy path, - // not the fallback break. + // Flag is irrelevant here: the wizard still reaches model selection, + // rather than taking the fallback path. assert.equal(payload.sentinelTripped, true); }); @@ -522,7 +517,7 @@ describe("nemoclaw onboard --no-ollama-autostart (#3751)", () => { ); assert.equal(payload.result!.model, DEFAULT_OLLAMA_MODEL); assert.equal(payload.result!.provider, "ollama-local"); - // Non-interactive gate path must not reach the proxy stage either. + // Non-interactive gate path must not reach model selection either. assert.equal(payload.sentinelTripped, false); }); @@ -622,13 +617,13 @@ describe("nemoclaw onboard --no-ollama-autostart (#3751)", () => { ), `expected pinned-provider abort message; lines:\n${payload.lines.join("\n")}`, ); - // Sentinel guards the post-spawn proxy step. If selectionLoop had looped - // and a future iteration reached the proxy, the sentinel would have - // tripped. With the fix, we exit before that. + // The sentinel guards model selection. If selectionLoop had looped and a + // future iteration reached it, the sentinel would have tripped. With the + // fix, we exit before that. assert.equal( payload.sentinelTripped, false, - "abort must happen before reaching the proxy stage", + "abort must happen before reaching model selection", ); }); }); diff --git a/test/onboard-selection.test.ts b/test/onboard-selection.test.ts index d06518ec133..308ee3e3771 100644 --- a/test/onboard-selection.test.ts +++ b/test/onboard-selection.test.ts @@ -450,7 +450,7 @@ function makeSetupNimOllamaDeps(overrides: Partial = {}): Se ensureOllamaLoopbackSystemdOverride: () => "not-applicable", runOllamaStartupOrGate: () => ({ kind: "ready" }), shouldFrontOllamaWithProxy: () => false, - startOllamaAuthProxy: () => true, + // Proxy startup is deferred until configuration review acceptance. getLocalProviderBaseUrl: () => "http://host.docker.internal:11434/v1", selectAndValidateOllamaModel: async () => ({ outcome: "selected",