fix(onboard): avoid repeating completed prompts on resume - #6934
Merged
Conversation
Signed-off-by: San Dang <sdang@nvidia.com>
sandl99
added
VDR
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughOnboarding now persists secret-free checkpoints for sandbox name, web search, messaging, and resource selection. Resume reuses completed choices and exact registered provider bindings, reacquires unavailable credentials, stages providers before sandbox creation, and preserves non-secret configuration across interruptions. ChangesOnboarding resume and credential checkpointing
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Operator
participant Onboard
participant Session
participant OpenShell
participant Sandbox
Operator->>Onboard: nemoclaw onboard --resume
Onboard->>Session: load completed prompt checkpoints
Onboard->>OpenShell: validate staged provider bindings
OpenShell-->>Onboard: matching bindings or credential requirements
Onboard->>Sandbox: resolve remaining choices and create sandbox
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
38 tasks
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-6934.docs.buildwithfern.com/nemoclaw |
Contributor
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 4 optional E2E recommendations
1 warning · 0 suggestionsWarningsWarnings do not block.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
src/lib/onboard/providers.test.ts (1)
373-402: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAssert that the thrown diagnostic excludes the credential.
toThrow("...gateway rejected")still passes if the full error contains the token afterward. Capture the error and assert its message does not containtoken, protecting the user-visible failure boundary.As per path instructions, “Review tests for behavioral confidence rather than implementation lock-in.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/onboard/providers.test.ts` around lines 373 - 402, Update the test around stageMessagingProvidersCreateMissingOnly to capture the thrown error, assert its message includes the gateway rejection context, and explicitly verify it does not contain the token. Keep the existing assertions that no secret-bearing command or update/delete operation is attempted.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/reference/commands.mdx`:
- Around line 231-235: Rewrite the new resume-rule sentences in the
documentation using direct second-person, active, present-tense wording. Address
the reader as “you” and preserve the existing semantics for prompt checkpoints,
legacy sessions, credential storage, provider validation, and interactive versus
non-interactive resume behavior.
In `@src/lib/onboard.ts`:
- Around line 3793-3818: Update stageSandboxCredentialProviders so existing
staged receipts are preserved when stageMessagingProvidersCreateMissingOnly
reports a reused provider. Clear receipts only when the provider is mismatched
or indeterminate, and continue setting them for created providers; avoid
unconditionally clearing receipts before probing.
- Line 1030: Update providerMatchesGatewayCredential in src/lib/onboard.ts at
lines 1030-1030 to require stagedCredentialProviders to contain the provider
name before applying the exact gateway metadata matcher. In
src/lib/onboard/messaging-prep.test.ts at lines 99-120, add coverage showing an
exact provider without a session receipt is not reusable, then add the receipt
and verify the provider becomes reusable.
In `@src/lib/onboard/gateway-provider-metadata.test.ts`:
- Around line 72-82: Add a test case in the
matchesGatewayCredentialOnlyProviderBinding suite using metadata with an empty
credentialKeys array, and assert that it returns false. Keep the existing
incorrect-key and multiple-key cases unchanged.
In `@src/lib/onboard/providers.ts`:
- Around line 519-635: Remove the `@ts-nocheck` suppression from providers.ts and
make the credential-staging path type-check under tsconfig.cli.json. Add
explicit types for providerCommandOutput, inspectStagedProvider,
stagedProviderCreateArgs, and stageMessagingProvidersCreateMissingOnly,
including runner results, token definitions, options, and returned collections;
resolve all resulting type errors without changing behavior.
In `@src/lib/onboard/sandbox-agent.ts`:
- Line 178: Move checkpointSandboxName(validatedSandboxName) out of the
validateName() try/catch boundary so checkpoint session-write failures propagate
as checkpoint errors rather than being reported as name-validation errors; keep
the existing validation error handling and validated-name flow unchanged.
In `@src/lib/state/onboard-session.ts`:
- Around line 381-410: The messaging progress validation in
parseSandboxPromptProgress must be tied to the recorded sandbox: pass
choices.sandboxName into parseSandboxMessagingPlan when validating
choices.messagingPlan, while preserving null handling and existing completion
checks. Add a regression test covering a messaging plan for a different sandbox
being rejected and messaging remaining incomplete.
---
Nitpick comments:
In `@src/lib/onboard/providers.test.ts`:
- Around line 373-402: Update the test around
stageMessagingProvidersCreateMissingOnly to capture the thrown error, assert its
message includes the gateway rejection context, and explicitly verify it does
not contain the token. Keep the existing assertions that no secret-bearing
command or update/delete operation is attempted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c836a145-7712-459a-88a3-fe4927385347
📒 Files selected for processing (35)
docs/reference/commands.mdxdocs/reference/troubleshooting.mdxdocs/security/credential-storage.mdxsrc/lib/onboard.tssrc/lib/onboard/agent-resume-state.tssrc/lib/onboard/gateway-provider-metadata.test.tssrc/lib/onboard/gateway-provider-metadata.tssrc/lib/onboard/lifecycle-contracts.mdsrc/lib/onboard/machine/core-flow-phases.test.tssrc/lib/onboard/machine/handlers/provider-inference.tssrc/lib/onboard/machine/handlers/sandbox-create-intent-boundary.test.tssrc/lib/onboard/machine/handlers/sandbox-messaging.test.tssrc/lib/onboard/machine/handlers/sandbox-messaging.tssrc/lib/onboard/machine/handlers/sandbox-route-mutation-lock.test.tssrc/lib/onboard/machine/handlers/sandbox-test-fixtures.tssrc/lib/onboard/machine/handlers/sandbox.test.tssrc/lib/onboard/machine/handlers/sandbox.tssrc/lib/onboard/messaging-channel-setup.test.tssrc/lib/onboard/messaging-channel-setup.tssrc/lib/onboard/messaging-prep.test.tssrc/lib/onboard/messaging-prep.tssrc/lib/onboard/providers.test.tssrc/lib/onboard/providers.tssrc/lib/onboard/sandbox-agent.test.tssrc/lib/onboard/sandbox-agent.tssrc/lib/onboard/sandbox-create-intent-resolution.tssrc/lib/onboard/sandbox-messaging-preflight.test.tssrc/lib/onboard/sandbox-messaging-preflight.tssrc/lib/onboard/session-bootstrap.test.tssrc/lib/onboard/session-bootstrap.tssrc/lib/state/onboard-session-sandbox-prompts.test.tssrc/lib/state/onboard-session.tstest/onboard-inference-reconciliation.test.tstest/onboard-messaging.test.tstest/onboard.test.ts
Signed-off-by: San Dang <sdang@nvidia.com>
Collaborator
Author
Security reviewVerdict: PASS — no blockers or warnings.
Residual informational note: OpenShell exposes credential binding metadata, not credential values, so an out-of-band value-only rotation cannot be distinguished from the previously registered value. Such a mutation already requires gateway credential-administration authority. |
Signed-off-by: San Dang <sdang@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/onboard/credential-provider-registration.ts`:
- Around line 107-117: Verify every caller of providerMatchesGatewayCredential,
especially the resume or skip-the-prompt decision path, checks that
session.stagedCredentialProviders includes the provider name before trusting the
predicate result. Add or move this same-session receipt gate into the caller if
missing, while keeping providerMatchesGatewayCredential limited to live gateway
metadata matching.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 1b987169-1c5e-4e6a-a53f-07ed11335ed7
📒 Files selected for processing (27)
docs/reference/commands.mdxdocs/reference/troubleshooting.mdxdocs/security/credential-storage.mdxsrc/lib/onboard.tssrc/lib/onboard/credential-provider-registration.test.tssrc/lib/onboard/credential-provider-registration.tssrc/lib/onboard/gateway-provider-metadata.test.tssrc/lib/onboard/lifecycle-contracts.mdsrc/lib/onboard/machine/handlers/provider-inference.tssrc/lib/onboard/machine/handlers/sandbox-create-intent-boundary.test.tssrc/lib/onboard/machine/handlers/sandbox-messaging.test.tssrc/lib/onboard/machine/handlers/sandbox-messaging.tssrc/lib/onboard/machine/handlers/sandbox-route-mutation-lock.test.tssrc/lib/onboard/machine/handlers/sandbox-tool-disclosure.test.tssrc/lib/onboard/machine/handlers/sandbox.test.tssrc/lib/onboard/machine/handlers/sandbox.tssrc/lib/onboard/messaging-prep.test.tssrc/lib/onboard/messaging-prep.tssrc/lib/onboard/sandbox-agent.test.tssrc/lib/onboard/sandbox-agent.tssrc/lib/onboard/sandbox-create-intent-resolution.tssrc/lib/onboard/sandbox-messaging-preflight.test.tssrc/lib/onboard/sandbox-messaging-preflight.tssrc/lib/onboard/session-bootstrap.tssrc/lib/onboard/types.tssrc/lib/state/onboard-session-sandbox-prompts.test.tssrc/lib/state/onboard-session.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- src/lib/onboard/machine/handlers/sandbox-route-mutation-lock.test.ts
- src/lib/onboard/sandbox-agent.test.ts
- src/lib/onboard/machine/handlers/provider-inference.ts
- src/lib/onboard/sandbox-messaging-preflight.ts
- src/lib/state/onboard-session-sandbox-prompts.test.ts
- docs/reference/commands.mdx
- src/lib/onboard/sandbox-messaging-preflight.test.ts
- src/lib/onboard/machine/handlers/sandbox-messaging.ts
- src/lib/state/onboard-session.ts
- src/lib/onboard/messaging-prep.ts
- src/lib/onboard/gateway-provider-metadata.test.ts
- src/lib/onboard/machine/handlers/sandbox-create-intent-boundary.test.ts
- docs/security/credential-storage.mdx
- src/lib/onboard/machine/handlers/sandbox-messaging.test.ts
- src/lib/onboard/lifecycle-contracts.md
- src/lib/onboard/machine/handlers/sandbox.ts
Collaborator
Author
PR Review Advisor follow-upI verified both findings against the full onboarding sequence and OpenShell 0.0.72. PRA-1 — no code changeThe reported null-name ordering is not reachable through a valid supported onboarding flow. The core flow always runs provider/inference before sandbox setup (
The only way to reach PRA-2 — accepted residual warningThe check/update boundary is a real residual control-plane race, but OpenShell 0.0.72 exposes no provider-ID or expected-resource-version/compare-and-swap option for This PR keeps the available containment: registration is pinned to the intended gateway, serialized under NemoClaw's gateway mutation lock, receipts are cleared before mutation and recorded only after a successful command, and credential-free resume requires both the same-session receipt and a fresh exact live name/type/credential-key match. Exploiting the remaining window requires existing authority to mutate the target OpenShell gateway. Closing that warning atomically requires an upstream OpenShell conditional-update API. Porting the MCP inspect/mutate/inspect compensation here would add provider-ID/resource-version lifecycle machinery while retaining the underlying race, so it is outside this completed-prompt resume fix. All E2E lanes selected by the advisor, including |
Contributor
E2E Target Results — ✅ All selected tests passedRun: 29420793234
|
21 tasks
cv
pushed a commit
that referenced
this pull request
Jul 16, 2026
<!-- markdownlint-disable MD041 --> ## Summary Adds the canonical `docs/changelog/2026-07-15.mdx` entry with the exact `## v0.0.84` heading for the release candidate range from `v0.0.83` through `710d2b36b9eebcb6bca3c2b2f796a1bdb69c3a31`. Fills two owner-page gaps for model-aware local inference health and pre-write OpenClaw candidate validation. ## Changes - Add the complete shared Fern changelog entry for `v0.0.84`, with literal CLI names and root-absolute OpenClaw and Hermes routes. - Document that sandbox status and doctor compare the configured Ollama or vLLM model with provider inventory without issuing a completion. - Document that host-side OpenClaw `config set` validates the complete candidate before replacing live config or reaching gateway restart. - Reconcile the `v0.0.84` release label with the commit range. PR #6773 is already contained in `v0.0.83` and remains documented there; CI, test-harness, docs-infrastructure, and `.js` to `.mts` migration-only changes require no additional user guidance. ### Source summary - [#6882](#6882) -> `docs/manage-sandboxes/backup-restore.mdx`, `docs/changelog/2026-07-15.mdx`: Explain that OpenClaw runtime identity and pairing state are excluded from snapshots and ignored during restore. - [#6873](#6873) -> `docs/inference/set-up-ollama.mdx`, `docs/changelog/2026-07-15.mdx`: Record the Ollama requested-model environment fallback and interactive default. - [#6835](#6835) -> `docs/changelog/2026-07-15.mdx`: Include the sandbox name in the documented rebuild resume-recovery behavior. - [#6886](#6886) -> `docs/inference/custom-endpoint-security.mdx`, `docs/inference/set-up-openai-compatible-endpoint.mdx`, `docs/changelog/2026-07-15.mdx`: Explain the exact-host trusted-private endpoint opt-in and retained SSRF boundaries. - [#6887](#6887) -> `docs/reference/commands.mdx`, `docs/changelog/2026-07-15.mdx`: Document Telegram channel health verdicts, summary behavior, and exit status. - [#6863](#6863) -> `docs/manage-sandboxes/lifecycle.mdx`, `docs/changelog/2026-07-15.mdx`: Add the missing model-inventory behavior for local status and doctor checks. - [#6902](#6902) -> `docs/manage-sandboxes/runtime-controls.mdx`, `docs/changelog/2026-07-15.mdx`: Add the missing pre-write OpenClaw candidate-validation contract. - [#6916](#6916) -> `docs/changelog/2026-07-15.mdx`: Preserve the failed-session fresh-install recovery correction in the release entry. - [#6934](#6934) -> `docs/reference/commands.mdx`, `docs/reference/troubleshooting.mdx`, `docs/security/credential-storage.mdx`, `docs/changelog/2026-07-15.mdx`: Summarize completed-prompt checkpointing and validated credential reuse during OpenClaw resume. - [#6898](#6898) -> `docs/inference/switch-models.mdx`, `docs/inference/switch-providers.mdx`, `docs/reference/troubleshooting.mdx`, `docs/changelog/2026-07-15.mdx`: Explain Hermes dashboard convergence after in-place inference changes. - [#6711](#6711) -> `docs/manage-sandboxes/run-sandboxes.mdx`, `docs/manage-sandboxes/uninstall-nemoclaw.mdx`, `docs/reference/architecture.mdx`, `docs/reference/commands.mdx`, `docs/changelog/2026-07-15.mdx`: Summarize port-scoped host state and uninstall preservation. - [#6767](#6767) -> `docs/inference/configure-model-limits.mdx`, `docs/inference/set-up-ollama.mdx`, `docs/reference/troubleshooting.mdx`, `docs/changelog/2026-07-15.mdx`: Record the Hermes `64000`-token Ollama floor and unchanged OpenClaw floor. - [#6862](#6862) -> `docs/get-started/quickstart.mdx`, `docs/inference/verify-inference-route.mdx`, `docs/changelog/2026-07-15.mdx`: Explain retryable not-ready finalization for unhealthy inference routes. - [#6766](#6766) -> `docs/security/tcb-boundary.mdx`, `docs/changelog/2026-07-15.mdx`: Document definitive stale transition-lock recovery and fail-closed ambiguous cases. - [#6948](#6948) -> `docs/manage-sandboxes/manage-mcp-servers.mdx`, `docs/changelog/2026-07-15.mdx`: Include Hermes MCP apply-state race recovery in the release entry without changing the established user workflow. - [#6964](#6964) -> `docs/reference/troubleshooting.mdx`, `docs/changelog/2026-07-15.mdx`: Record complete agent-specific fresh-install and resume recovery commands. - [#6883](#6883) -> `docs/get-started/quickstart.mdx`, `docs/inference/set-up-vllm.mdx`, `docs/reference/platform-support.mdx`, `docs/changelog/2026-07-15.mdx`: Summarize the DGX Station Nemotron Ultra express path and pinned managed-vLLM recipe. - [#6985](#6985) -> `docs/inference/set-up-vllm.mdx`, `docs/reference/commands.mdx`, `docs/changelog/2026-07-15.mdx`: Capture the final automated and interactive storage-warning behavior. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — `test/changelog-docs.test.ts` validates the dated-entry structure, exact version heading, and preserved history. - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs 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: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run test/changelog-docs.test.ts` (6 passed) - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — not run for this doc-only change. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — completed with 0 errors; Fern reported the unchanged unauthenticated redirect-check and light-theme contrast warnings. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) — the native changelog entry uses the required parser-safe MDX SPDX comment and intentionally has no frontmatter. --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added the v0.0.84 changelog entry covering setup, endpoint onboarding, model handling, sandbox readiness, recovery, channel status, and configuration safeguards. * Clarified that sandbox health checks validate configured models against local Ollama and vLLM provider inventories without generating completions or consuming tokens. * Documented that invalid runtime configuration changes are rejected while preserving the existing working configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
20 tasks
22 tasks
82 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
nemoclaw onboard --resumepreviously repeated completed sandbox configuration prompts after a later onboarding interruption. This change checkpoints completed OpenClaw choices and registers validated web-search and messaging credentials with OpenShell before the next interruptible prompt, so resume continues at the first incomplete choice without persisting raw credentials.Passed Test
Related Issue
Fixes #6932
Part of #6743
Changes
Type of Change
Quality Gates
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm run test:changed(190 files, 1,868 tests); focused resume/credential tests;npm run typecheck:clinpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Not applicable to this path-scoped onboarding fix; changed-test, type-check, docs, hook, and guardrail gates passed.npm run docsbuilds without warnings (doc changes only) — not applicable to this code-and-docs PR; build passed with 0 errors and 2 pre-existing warningsSigned-off-by: San Dang sdang@nvidia.com
Summary by CodeRabbit
Summary
nemoclaw onboard --resumewith finer-grained checkpointing for sandbox name, web search, messaging selection/config, and resource profile.