fix(onboard): persist credentialEnv clears when switching remote→local (#2625)

[jyaunches]

Summary

Fixes the upgrade-sandboxes / per-sandbox rebuild preflight incorrectly demanding a stale provider credential (e.g. OPENAI_API_KEY) after the user switches from a remote provider to a local one (Ollama / vLLM / local NIM). The wizard already emits credentialEnv=null on local-provider selection, but the session-update pipeline silently dropped the clear, so the stale value survived in ~/.nemoclaw/onboard-session.json and the next rebuild preflight bailed out.

Related Issue

Closes #2625
Related: #2306 (credential resolution canonicalization umbrella), #2519 (local Ollama caching OPENAI_API_KEY — pre-fix migration retained as safety net for on-disk sessions)

Changes

Two halves of the write path were broken for nullable session fields. Both fixed together:

• src/lib/state/onboard-session.ts — new assignNullableString helper in filterSafeUpdates treats null as an explicit clear, string as assign (with optional normalizer), and undefined as leave-unchanged. Applied uniformly to all six nullable string fields on Session: sandboxName, provider, model, endpointUrl, credentialEnv, preferredInferenceApi, nimContainer. SessionUpdates widened to string | null for those fields.
• src/lib/onboard.ts — new toNullableString helper in toSessionUpdates preserves null instead of collapsing to undefined via toOptionalString. toOptionalString stays for genuinely optional (non-clearable) fields.
• src/lib/actions/sandbox/rebuild.ts — comment-only. Noted the [Linux][Brev]Ollama-local provider returns HTTP 401 Unauthorized via inference.local — wizard caches OPENAI_API_KEY env var into credentials.json and not refreshable via env unset #2519 legacy-migration block is still needed for users whose session.json predates this fix; fresh sessions no longer need it.
• src/lib/state/onboard-session.test.ts — 4 new cases:
  • Provider switch OpenAI → Ollama clears credentialEnv.
  • undefined regression guard: partial updates do not accidentally wipe fields.
  • null-clear contract holds for every nullable field in one shot.
  • completeSession honors the clear on terminal finalize.

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)

Verification

• npm test passes for the affected areas:
  • src/lib/state/onboard-session.test.ts — 42/42 (4 new + 38 existing)
  • test/rebuild-credential-preflight.test.ts + test/rebuild-credential-hydration.test.ts — 16/16 (including [Linux][Brev]Ollama-local provider returns HTTP 401 Unauthorized via inference.local — wizard caches OPENAI_API_KEY env var into credentials.json and not refreshable via env unset #2519 legacy-migration guards)
  • src/lib/onboard-command.test.ts, src/lib/onboard-inference-probes.test.ts — unchanged, pass
• Tests added for the new behavior (null-clear contract at the write layer)
• No secrets, API keys, or credentials committed
• npm run build:cli clean (tsc -p tsconfig.src.json + blueprint)
• biome lint clean on touched files
• make docs — not applicable (no user-facing doc changes; rebuild error path is the same on the failure case)

Test plan for reviewers

The reporter's exact repro from the issue body is covered by the unit tests but is worth the manual check too:

1. nemoclaw onboard with OpenAI, complete.
2. unset OPENAI_API_KEY && nemoclaw credentials reset OPENAI_API_KEY.
3. nemoclaw onboard selecting local Ollama (or any local provider).
4. Inspect ~/.nemoclaw/onboard-session.json — credentialEnv should now be null (pre-fix it stayed as "OPENAI_API_KEY").
5. nemoclaw upgrade-sandboxes should no longer abort with Missing credential: OPENAI_API_KEY.

Follow-up (not in this PR)

Phase 3 from the dev plan — downgrade the rebuild preflight to a warning when credentialEnv is set but no env/saved value is found — would generalize the narrow #2519 migration into defense-in-depth for any stale remote-provider session on disk. Intentionally deferred to a separate PR to keep this one tightly scoped to the write-path root cause.

---

Signed-off-by: Julie Yaunches jyaunches@nvidia.com

Summary by CodeRabbit

• Bug Fixes

  • Fixed session updates to properly distinguish between clearing a field (null) and leaving it unchanged (undefined). Several onboarding settings—including sandbox name, provider, model, endpoint URL, and inference preferences—now correctly persist explicit clear operations.

• Tests

  • Added test coverage for field-clearing behavior in onboarding sessions.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6b268b34-9cac-4e6e-ae29-f465d7dce1aa

📥 Commits

Reviewing files that changed from the base of the PR and between b1320d5 and b450b11.

📒 Files selected for processing (4)

• src/lib/actions/sandbox/rebuild.ts
• src/lib/onboard.ts
• src/lib/state/onboard-session.test.ts
• src/lib/state/onboard-session.ts

---

📝 Walkthrough

Walkthrough

This PR fixes #2625 by introducing explicit null semantics to session state updates. When a user switches from a remote provider (OpenAI, Anthropic, etc.) to local inference (Ollama, vLLM), the new code correctly clears the stale credentialEnv rather than leaving it to block subsequent rebuilds. Seven session fields now accept null as an explicit "clear" value while undefined continues to mean "leave unchanged."

Changes

Session Null Clearing for Provider Switches

Layer / File(s)	Summary
Session Update Type Contract src/lib/state/onboard-session.ts	SessionUpdates expands sandboxName, provider, model, endpointUrl, credentialEnv, preferredInferenceApi, and nimContainer to accept `string
Null-Safe Helper Functions src/lib/state/onboard-session.ts, src/lib/onboard.ts	assignNullableString() and toNullableString() helpers distinguish undefined (leave unchanged), null (persist explicit clear), and string (persist after optional normalization).
Session Update Filtering src/lib/state/onboard-session.ts	filterSafeUpdates() refactored to apply assignNullableString() across nullable fields, ensuring explicit null clears are persisted instead of dropped.
Session Update Normalization src/lib/onboard.ts	toSessionUpdates() now uses toNullableString() for seven session fields to preserve nullability through normalization.
Tests and Documentation src/lib/state/onboard-session.test.ts, src/lib/actions/sandbox/rebuild.ts	Four new regression and contract tests validate null-clearing across provider switches, field-by-field contracts, and session completion; rebuild.ts documentation extended to record post-#2625 behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A rabbit's verse on session clarity:
When switching from remote to local's bright,
The old credential key must fade to night,
With null explicit, the session stays clean—
No stale OPENAI_KEY in between,
Local inference runs without a fight! 🌙

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately summarizes the main fix: clearing credentialEnv when switching from remote to local providers to resolve rebuild preflight failures.
Linked Issues check	✅ Passed	The PR fully implements the primary objective from #2625: clearing credentialEnv on provider switch, properly propagating null clears through the session update pipeline, and preserving legacy-migration safety.
Out of Scope Changes check	✅ Passed	All changes directly address the linked issue objectives: nullable field type expansion, null-clear semantics in session updates, documentation clarification, and test coverage for the fix.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-2625-clear-credential-env-on-provider-switch

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[ericksoa]

Reviewed current head b450b11 against current main. The null/undefined contract is preserved end-to-end: provider selection can persist credentialEnv=null for local providers, undefined remains a no-op for partial updates, and the nullable session string fields now share the same explicit-clear handling. The existing rebuild legacy migration remains intact for old local-inference sessions.\n\nValidated locally on a merge simulation with current origin/main: npm run build:cli; npx vitest run src/lib/state/onboard-session.test.ts test/rebuild-credential-preflight.test.ts test/rebuild-credential-hydration.test.ts; git diff --check.