Skip to content

fix(onboard): honor back and exit at the credential re-entry prompt - #9558

Closed
udsy19 wants to merge 1 commit into
NVIDIA:mainfrom
udsy19:fix/reentry-credential-intent
Closed

fix(onboard): honor back and exit at the credential re-entry prompt#9558
udsy19 wants to merge 1 commit into
NVIDIA:mainfrom
udsy19:fix/reentry-credential-intent

Conversation

@udsy19

@udsy19 udsy19 commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

The onboarding recovery menu advertises back (change provider) and exit, then the credential
re-entry prompt it opens stages those words as the API key. back, exit, quit, and ? were each
saved into the provider's credential environment variable and registered with the OpenShell gateway,
and an empty answer looped on is required with no offered escape. The re-entry prompt now classifies
answers with getCredentialPromptIntent, the same classifier the canonical credential-navigation
helpers use: back returns to provider selection, exit and quit quit onboarding, and ?, help,
or an empty line re-prompt with an escape hint.

Related Issue

Fixes #9557

Changes

  • src/lib/onboard/validation-recovery-prompt.ts: the private replaceNamedCredential reads its
    answer through getCredentialPromptIntent instead of normalizeCredentialValue, which only trims
    whitespace and classified nothing. exit calls deps.exitOnboardFromPrompt(); back returns the
    shared BACK_TO_SELECTION sentinel from src/lib/navigation.ts; help and empty re-prompt with a
    hint naming the escapes. Both call sites map the sentinel to the existing "selection" outcome, so
    promptValidationRecovery's return type is unchanged. normalizeCredentialValue had no other use
    in the file and was removed from the import.
  • src/lib/onboard/credential-navigation.test.ts: extend describe("credential prompt navigation helpers") with a case driving the real promptValidationRecovery through menu retry → empty →
    back, asserting the outcome is selection, that the credential env var is never staged, that
    every scripted answer was consumed (so the empty answer looped and was escapable), and that exit
    was not called.

No new abstraction, configuration, fallback, or compatibility path. getCredentialPromptIntent is
already exported from ../credentials/store, which this file already imported, so this adds no
dependency edge there. The one new edge is to src/lib/navigation.ts, which has no
ci/source-architecture-budget.json override and therefore the default fan-in cap of 20; it goes
from 6 to 7.

Scope note: the outer recovery menu in the same file already handled back and exit correctly and
is unchanged. Only the inner re-entry prompt was affected.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification:
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit:
  • Station profile/scenario:
  • Result:
  • Supporting evidence:

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result: npx vitest run --project cli src/lib/onboard/credential-navigation.test.ts → 5 passed; npx vitest run --project integration test/onboard-selection.test.ts → 67 passed
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Behavior before and after

Driving the shipped helper with the menu answered retry and the re-entry prompt answered with each
navigation word.

Before:

typed "back" -> outcome="credential" OPENAI_API_KEY="back"
typed "exit" -> outcome="credential" OPENAI_API_KEY="exit"
typed "quit" -> outcome="credential" OPENAI_API_KEY="quit"
typed "?"    -> outcome="credential" OPENAI_API_KEY="?"

After:

typed "back" -> outcome="selection"  OPENAI_API_KEY=undefined
typed "exit" -> onboarding exit       OPENAI_API_KEY=undefined
typed "quit" -> onboarding exit       OPENAI_API_KEY=undefined
typed "?"    -> re-prompts, escapable OPENAI_API_KEY=undefined

The added test fails on the parent commit with
AssertionError: expected 'credential' to be 'selection' and passes with this change.

Prior art

The same class was fixed for the first-entry credential prompt in #3697 and again for the NVIDIA API
key prompt in #9404 / #9427. #9427 changed src/lib/onboard.ts,
src/lib/onboard/nvidia-featured-model-selection.ts,
src/lib/onboard/nvidia-featured-model-selection.test.ts, and
src/lib/onboard/setup-nim-selection.ts; it did not reach
src/lib/onboard/validation-recovery-prompt.ts. This applies the same intent-classifier approach to
that remaining prompt.

Related but distinct: #6005 requests a general back-navigation key at every wizard step and is
labeled needs: design. This change is narrower — a prompt that already advertises back and exit
in its own menu text now honors them — and does not depend on that design decision.


Signed-off-by: Udaya Tejas udayatejas2004@gmail.com

Summary by CodeRabbit

  • Bug Fixes
    • Fixed credential recovery so choosing “Back” after an invalid API key returns to provider selection.
    • Prevented the “Back” action from being saved as an API key or triggering unintended environment changes.
    • Ensured onboarding does not exit unexpectedly when returning to provider selection.

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 <udayatejas2004@gmail.com>
@copy-pr-bot

copy-pr-bot Bot commented Aug 19, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions

Copy link
Copy Markdown
Contributor

This repository limits contributors to 5 open pull requests. Please close or merge existing PRs before opening new ones.

@github-actions github-actions Bot closed this Aug 19, 2026
@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6f72007f-202e-4920-998e-41eab40987ce

📥 Commits

Reviewing files that changed from the base of the PR and between 164cb28 and f0784d3.

📒 Files selected for processing (2)
  • src/lib/onboard/credential-navigation.test.ts
  • src/lib/onboard/validation-recovery-prompt.ts

📝 Walkthrough

Walkthrough

The credential recovery prompt now interprets navigation input as intent. Selecting back returns to provider selection without saving the input as an API key. A regression test covers the retry and back flow.

Changes

Credential recovery navigation

Layer / File(s) Summary
Recovery intent handling
src/lib/onboard/validation-recovery-prompt.ts
Credential replacement now handles exit and back intents. Recovery flows propagate back requests as "selection" instead of treating navigation input as a credential.
Recovery navigation regression test
src/lib/onboard/credential-navigation.test.ts
The test verifies provider-selection navigation, no persisted OPENAI_API_KEY, consumed scripted answers, and no onboarding exit.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested labels: area: cli, bug-fix

Suggested reviewers: cv

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@senthilr-nv senthilr-nv added bug-fix PR fixes a bug or regression area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow area: providers Inference provider integrations and provider behavior security v0.0.111 labels Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow area: providers Inference provider integrations and provider behavior bug-fix PR fixes a bug or regression security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Credential re-entry prompt after a failed provider validation accepts back, exit, quit and ? as the literal API key

2 participants