From f0784d33034b87b01b3ee89f2d02229d62b30ed6 Mon Sep 17 00:00:00 2001 From: Udaya Tejas Date: Tue, 18 Aug 2026 11:59:51 -0700 Subject: [PATCH 1/3] fix(onboard): honor back and exit at the credential re-entry prompt The onboarding validation-recovery re-entry prompt read the answer through normalizeCredentialValue, which only trims whitespace. It performed no intent classification, so back, exit, quit, and ? were staged as the literal API key for every provider whose credential env is not NVIDIA_INFERENCE_API_KEY or NVIDIA_API_KEY, because validateNvidiaApiKeyValue only enforces the nvapi- prefix for those two. The recovery menu advertises "back (change provider)" one line earlier, so back is an expected answer at that prompt. An empty answer also looped on "is required" with no advertised escape. Route the answer through getCredentialPromptIntent, the same classifier the canonical credential-navigation helpers use: exit quits onboarding, back returns the shared BACK_TO_SELECTION sentinel that both call sites map to provider selection, and help or empty re-prompts with an escape hint. Signed-off-by: Udaya Tejas --- src/lib/onboard/credential-navigation.test.ts | 35 +++++++++++++++++++ src/lib/onboard/validation-recovery-prompt.ts | 32 ++++++++++++----- 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/src/lib/onboard/credential-navigation.test.ts b/src/lib/onboard/credential-navigation.test.ts index 4921f4456e4..4d9d4fe3beb 100644 --- a/src/lib/onboard/credential-navigation.test.ts +++ b/src/lib/onboard/credential-navigation.test.ts @@ -4,12 +4,14 @@ import { describe, expect, it, vi } from "vitest"; import * as credentials from "../credentials/store"; +import { validateNvidiaApiKeyValue } from "../validation"; import { BACK_TO_SELECTION, replaceNamedCredential, returningToProviderSelection, shouldReturnToProviderSelection, } from "./credential-navigation"; +import { createValidationRecoveryPromptHelpers } from "./validation-recovery-prompt"; describe("credential prompt navigation helpers", () => { it("treats both the shared back sentinel and credential back intents as provider-selection navigation", () => { @@ -74,4 +76,37 @@ describe("credential prompt navigation helpers", () => { vi.restoreAllMocks(); } }); + + it("returns to provider selection instead of staging back as the re-entered API key (#3697)", async () => { + const answers = ["retry", "", "back"]; + const exitOnboardFromPrompt = vi.fn(() => { + throw new Error("unexpected exit"); + }) as unknown as () => never; + vi.spyOn(console, "log").mockImplementation(() => {}); + vi.spyOn(console, "error").mockImplementation(() => {}); + const { promptValidationRecovery } = createValidationRecoveryPromptHelpers({ + isNonInteractive: () => false, + prompt: async () => answers.shift() ?? "", + validateNvidiaApiKeyValue: (key, credentialEnv) => + validateNvidiaApiKeyValue(key, credentialEnv ?? undefined), + getTransportRecoveryMessage: () => "", + exitOnboardFromPrompt, + }); + delete process.env.OPENAI_API_KEY; + try { + await expect( + promptValidationRecovery( + "OpenAI", + { kind: "credential", retry: "credential" }, + "OPENAI_API_KEY", + ), + ).resolves.toBe("selection"); + expect(process.env.OPENAI_API_KEY).toBeUndefined(); + expect(answers).toEqual([]); + expect(exitOnboardFromPrompt).not.toHaveBeenCalled(); + } finally { + delete process.env.OPENAI_API_KEY; + vi.restoreAllMocks(); + } + }); }); diff --git a/src/lib/onboard/validation-recovery-prompt.ts b/src/lib/onboard/validation-recovery-prompt.ts index bb39aca867e..ee9edc7479e 100644 --- a/src/lib/onboard/validation-recovery-prompt.ts +++ b/src/lib/onboard/validation-recovery-prompt.ts @@ -1,7 +1,8 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { normalizeCredentialValue, saveCredential } from "../credentials/store"; +import { getCredentialPromptIntent, saveCredential } from "../credentials/store"; +import { BACK_TO_SELECTION, type BackToSelection, isBackToSelection } from "../navigation"; import type { ProbeRecovery } from "../validation-recovery"; export interface ValidationRecoveryPromptDeps { @@ -18,7 +19,7 @@ export interface ValidationRecoveryPromptHelpers { label: string, helpUrl?: string | null, validator?: ((value: string) => string | null) | null, - ): Promise; + ): Promise; promptValidationRecovery( label: string, recovery: ProbeRecovery, @@ -35,7 +36,7 @@ export function createValidationRecoveryPromptHelpers( label: string, helpUrl: string | null = null, validator: ((value: string) => string | null) | null = null, - ): Promise { + ): Promise { if (helpUrl) { console.log(""); console.log(` Get your ${label} from: ${helpUrl}`); @@ -43,9 +44,12 @@ export function createValidationRecoveryPromptHelpers( } while (true) { - const key = normalizeCredentialValue(await deps.prompt(` ${label}: `, { secret: true })); + const intent = getCredentialPromptIntent(await deps.prompt(` ${label}: `, { secret: true })); + if (intent.kind === "exit") deps.exitOnboardFromPrompt(); + if (intent.kind === "back") return BACK_TO_SELECTION; + const key = intent.kind === "credential" ? intent.value : ""; if (!key) { - console.error(` ${label} is required.`); + console.error(` ${label} is required. Type back to change provider, or exit to quit.`); continue; } const validationError = typeof validator === "function" ? validator(key) : null; @@ -103,8 +107,13 @@ export function createValidationRecoveryPromptHelpers( if (looksLikeToken) { console.log(" ⚠️ That looks like an API key — do not paste credentials here."); console.log(" Treating as 'retry'. You will be prompted to enter the key securely."); - await replaceNamedCredential(credentialEnv, `${label} API key`, helpUrl, validator); - return "credential"; + const pasted = await replaceNamedCredential( + credentialEnv, + `${label} API key`, + helpUrl, + validator, + ); + return isBackToSelection(pasted) ? "selection" : "credential"; } if (choice === "back") { console.log(" Returning to provider selection."); @@ -115,8 +124,13 @@ export function createValidationRecoveryPromptHelpers( deps.exitOnboardFromPrompt(); } if (choice === "" || choice === "retry") { - await replaceNamedCredential(credentialEnv, `${label} API key`, helpUrl, validator); - return "credential"; + const replaced = await replaceNamedCredential( + credentialEnv, + `${label} API key`, + helpUrl, + validator, + ); + return isBackToSelection(replaced) ? "selection" : "credential"; } console.log(" Please choose a provider/model again."); console.log(""); From 87d08d133bab9d7bad3dc0e8f7be7bbc52b905be Mon Sep 17 00:00:00 2001 From: Udaya Tejas Date: Wed, 19 Aug 2026 09:54:25 -0700 Subject: [PATCH 2/3] test(onboard): assert the empty-credential re-prompt names both escapes The empty-input sequence proved only that another prompt followed. A bare required-field notice satisfied it, which is the shape this change replaced, so the assertion now requires the re-prompt to name back and exit. Signed-off-by: Udaya Tejas --- src/lib/onboard/credential-navigation.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/lib/onboard/credential-navigation.test.ts b/src/lib/onboard/credential-navigation.test.ts index 4d9d4fe3beb..1a56b9ac42d 100644 --- a/src/lib/onboard/credential-navigation.test.ts +++ b/src/lib/onboard/credential-navigation.test.ts @@ -83,7 +83,7 @@ describe("credential prompt navigation helpers", () => { throw new Error("unexpected exit"); }) as unknown as () => never; vi.spyOn(console, "log").mockImplementation(() => {}); - vi.spyOn(console, "error").mockImplementation(() => {}); + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); const { promptValidationRecovery } = createValidationRecoveryPromptHelpers({ isNonInteractive: () => false, prompt: async () => answers.shift() ?? "", @@ -104,6 +104,11 @@ describe("credential prompt navigation helpers", () => { expect(process.env.OPENAI_API_KEY).toBeUndefined(); expect(answers).toEqual([]); expect(exitOnboardFromPrompt).not.toHaveBeenCalled(); + // The escape route is the contract here, so a bare required-field notice is not enough: + // the re-prompt has to name both ways out of the loop. + expect(consoleError).toHaveBeenCalledWith( + expect.stringMatching(/is required\..*\bback\b.*\bexit\b/), + ); } finally { delete process.env.OPENAI_API_KEY; vi.restoreAllMocks(); From 902329e66576be00553b76f568349c7358c5c06f Mon Sep 17 00:00:00 2001 From: Udaya Tejas Date: Wed, 19 Aug 2026 10:47:02 -0700 Subject: [PATCH 3/3] test(onboard): match the re-prompt escapes without pinning their order The previous assertion used one regex, which required back to appear before exit. The contract is that the re-prompt names both ways out of the loop, not the order it names them in, so a reworded message that still advertises both would have failed. Assert each word independently against the reported message. Signed-off-by: Udaya Tejas --- src/lib/onboard/credential-navigation.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/lib/onboard/credential-navigation.test.ts b/src/lib/onboard/credential-navigation.test.ts index 1a56b9ac42d..d9f274257db 100644 --- a/src/lib/onboard/credential-navigation.test.ts +++ b/src/lib/onboard/credential-navigation.test.ts @@ -105,10 +105,11 @@ describe("credential prompt navigation helpers", () => { expect(answers).toEqual([]); expect(exitOnboardFromPrompt).not.toHaveBeenCalled(); // The escape route is the contract here, so a bare required-field notice is not enough: - // the re-prompt has to name both ways out of the loop. - expect(consoleError).toHaveBeenCalledWith( - expect.stringMatching(/is required\..*\bback\b.*\bexit\b/), - ); + // the re-prompt has to name both ways out of the loop, in either order. + const [[requiredMessage]] = consoleError.mock.calls; + expect(requiredMessage).toContain("is required."); + expect(requiredMessage).toContain("back"); + expect(requiredMessage).toContain("exit"); } finally { delete process.env.OPENAI_API_KEY; vi.restoreAllMocks();