fix(cli): auto-recover non-interactive onboard when no sandbox recorded

[jason-ma-nv]

Summary

When a previous onboarding run was interrupted before sandbox creation completed, the saved session has no sandbox name. install.sh auto-attaches --resume for any in_progress session, so a non-interactive nemoclaw onboard then aborted with exit 1 and left curl|bash installs with no recovery path. This change treats that state as recoverable by discarding the stale session and starting a fresh onboard.

Related Issue

Fixes #5626

Changes

• src/lib/onboard/session-bootstrap.ts: in non-interactive resume with no recoverable sandbox name (interrupted before sandbox creation), discard the stale session and fall back to a fresh onboard instead of calling exitProcess(1). Replaced the assertRecoverableResumeSandboxName abort helper with a pure recoverableResumeSandboxName check. Interactive resume and resumes with a requested/recorded sandbox name (--name / NEMOCLAW_SANDBOX_NAME) are unchanged.
• src/lib/onboard/session-bootstrap.test.ts: replaced the test asserting the old abort with one asserting auto-recovery (clears the stale session, starts fresh, emits an explanatory message), plus a test confirming a requested sandbox name still resumes.

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

• PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
• Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
• Targeted tests pass for changed behavior
• Full npm test passes (broad runtime changes only)
• Tests added or updated for new or changed behavior
• No secrets, API keys, or credentials committed
• Docs updated for user-facing behavior changes
• 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)

---

Signed-off-by: Jason Ma jama@nvidia.com

Summary by CodeRabbit

• Bug Fixes

  • Improved session recovery to automatically initialize a fresh session when non-interactive resume fails, preventing errors.

• Tests

  • Updated test expectations for session bootstrap behavior and recovery scenarios.

[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: a0b292f6-2dc2-4172-8fbb-cb1b3a2002ef

📥 Commits

Reviewing files that changed from the base of the PR and between 96a1a8c and 55f9b00.

📒 Files selected for processing (2)

• src/lib/onboard/session-bootstrap.test.ts
• src/lib/onboard/session-bootstrap.ts

---

📝 Walkthrough

Walkthrough

prepareResumeSession no longer calls assertRecoverableResumeSandboxName (which exited with an error). A new helper recoverableResumeSandboxName returns a nullable name; when cannotPrompt is true and the result is null, the flow calls prepareFreshSession with fresh: true. Tests are updated accordingly.

Changes

Non-interactive onboard auto-recovery

Layer / File(s)	Summary
recoverableResumeSandboxName helper and auto-recovery logic src/lib/onboard/session-bootstrap.ts	Removes the exit-on-error assertRecoverableResumeSandboxName helper and replaces it with recoverableResumeSandboxName(session, input) that returns a nullable sandbox name. In prepareResumeSession, when input.cannotPrompt is true and no recoverable name exists, the code now delegates to prepareFreshSession({ ...input, fresh: true }, deps) instead of aborting. The old assertion call site is deleted.
Updated and new resume tests src/lib/onboard/session-bootstrap.test.ts	Replaces the prior rejection test (which expected ExitError and "cannot resume" messages) with an auto-recovery test asserting that the session is cleared, a new session ID is issued, non-interactive mode is preserved, and a fresh-onboarding error message is emitted. Adds a second test verifying that when a sandbox name is supplied, the existing in-progress session is kept and neither exitProcess nor clearSession is triggered.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

• #5626 – This PR directly implements option (a) from the issue's expected result: auto-recovering from an interrupted non-interactive session by treating it as a fresh onboard when no sandbox name is recorded.
• #5627 – The PR implements exactly the mechanism described: introducing recoverableResumeSandboxName and falling back to prepareFreshSession instead of exiting with an error.

Poem

🐇 A stale session once sent the rabbit away,
"No sandbox name!" cried the error each day.
But now little bunny just hops right on through—
Fresh start, no fuss, new session ID too!
Auto-recover! The warren rejoiced with a cheer. 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.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 title 'fix(cli): auto-recover non-interactive onboard when no sandbox recorded' accurately describes the primary change: auto-recovery behavior for non-interactive onboarding when no sandbox name exists.
Linked Issues check	✅ Passed	The PR fully implements solution (a) from issue #5626 by auto-recovering from interrupted sessions without recorded sandbox names in non-interactive mode, allowing fresh onboard without manual intervention.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the auto-recovery feature: modifications to resume logic in session-bootstrap.ts, updated test expectations, and removal of the strict abort helper. No unrelated changes detected.

_{✏️ 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 auto/fix-5626-noninteractive-onboard-recover

---

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

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the auto/fix-5626-nonint... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.

File	main	auto/fix-5626-nonint... 55f9b00	+/-
nemoclaw/src/se...cret-scanner.ts	—	100%	—
nemoclaw/src/commands/slash.ts	—	100%	—
nemoclaw/src/li...bprocess-env.ts	—	100%	—
nemoclaw/src/bl...eprint/state.ts	—	98%	—
nemoclaw/src/onboard/config.ts	—	98%	—
nemoclaw/src/bl...int/snapshot.ts	—	97%	—
nemoclaw/src/bl...print/runner.ts	—	95%	—
nemoclaw/src/co...ration-state.ts	—	94%	—
nemoclaw/src/bl...ate-networks.ts	—	94%	—
nemoclaw/src/index.ts	—	94%	—

---

_{Code Coverage is in Public Preview. Learn more and provide us with your feedback.}

[github-actions]

E2E Advisor Recommendation

Required E2E: onboard-resume-e2e, cloud-onboard-e2e
Optional E2E: onboard-negative-paths-e2e

Dispatch hint: onboard-resume-e2e,cloud-onboard-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

• onboard-resume-e2e (medium): Directly exercises interrupted onboarding state and nemoclaw onboard --resume --non-interactive; this PR changes the resume bootstrap decision point and stale-session handling.
• cloud-onboard-e2e (medium): Validates the main non-interactive installer/onboard path with hosted inference after this change routes an unrecoverable resume into fresh onboarding.

Optional E2E

• onboard-negative-paths-e2e (medium): Adjacent confidence for non-interactive onboarding edge cases and validation behavior, since the PR changes a previous non-interactive failure path into auto-recovery.

New E2E recommendations

• onboarding/session-resume (high): Existing onboard-resume-e2e covers resume after sandbox creation, but this PR’s specific regression is an in-progress resumable session with no recorded sandbox name. Add coverage that seeds such a stale session, runs nemoclaw onboard --resume --non-interactive, and asserts it clears the stale session and starts fresh instead of exiting.
  • Suggested test: Extend test/e2e/test-onboard-resume.sh with a stale pre-sandbox session auto-recovery scenario.

Dispatch hint

• Workflow: nightly-e2e.yaml
• jobs input: onboard-resume-e2e,cloud-onboard-e2e

[github-actions]

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: onboard-resume-vitest
Optional Vitest E2E scenarios: cloud-onboard-vitest

Dispatch required Vitest E2E scenarios:

• gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-resume-vitest

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

• onboard-resume-vitest: The PR changes non-interactive onboard resume behavior in session-bootstrap.ts, specifically stale/resumable session handling before sandbox creation. The wired free-standing onboard resume Vitest job is the smallest live E2E surface that exercises interrupted onboarding and nemoclaw onboard --resume --non-interactive integration.
  • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-resume-vitest

Optional Vitest E2E scenarios

• cloud-onboard-vitest: Optional adjacent coverage for the fresh non-interactive cloud onboarding path that the new auto-recovery branch falls back to after discarding a stale resume session.
  • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=cloud-onboard-vitest

Relevant changed files

• src/lib/onboard/session-bootstrap.ts

[github-actions]

PR Review Advisor — Changes requested

Merge posture: Do not merge yet
Primary next action: Fix PRA-2: Auto-recovered sessions still flow downstream as resume mode; then add or justify PRA-T1.
Open items: 1 required · 1 warning · 0 suggestions · 8 test follow-ups
Top item: Auto-recovered sessions still flow downstream as resume mode

Action checklist

• PRA-2 Fix: Auto-recovered sessions still flow downstream as resume mode in src/lib/onboard/session-bootstrap.ts:158
• PRA-1 Resolve or justify: Source-of-truth review needed: Non-interactive/cannotPrompt resume recovery in `prepareResumeSession()`
• PRA-T1 Add or justify test follow-up: Mocked behavioral coverage
• PRA-T2 Add or justify test follow-up: Mocked behavioral coverage
• PRA-T3 Add or justify test follow-up: Acceptance clause
• PRA-T4 Add or justify test follow-up: Acceptance clause
• PRA-T5 Add or justify test follow-up: Acceptance clause
• PRA-T6 Add or justify test follow-up: Acceptance clause
• PRA-T7 Add or justify test follow-up: Acceptance clause
• PRA-T8 Add or justify test follow-up: Non-interactive/cannotPrompt resume recovery in `prepareResumeSession()`

Findings index

ID	Severity	Category	Location	Required action
PRA-1	Resolve/justify	architecture	—	Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2	Required	acceptance	src/lib/onboard/session-bootstrap.ts:158	Make the bootstrap/caller contract carry the effective mode after recovery, for example by returning an `effectiveResume`/`recoveredFresh` flag or by performing this recovery at a layer that can reset the caller’s `resume` value. Use that effective value for lifecycle recording, banners, and onboarding flow context. Also document the recovery condition/removal condition near the fallback so the stale-state workaround has a clear source of truth.

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

PRA-2 Required — Auto-recovered sessions still flow downstream as resume mode

• Location: src/lib/onboard/session-bootstrap.ts:158
• Category: acceptance
• Problem: The new recovery branch clears the stale session and returns `prepareFreshSession(...)`, but the bootstrap result does not tell `src/lib/onboard.ts` that the effective mode changed from resume to fresh. The caller keeps using its original `resume` boolean for `recordOnboardStarted(resume)`, the `(resume mode)` banner, `initialFlowContext.resume`, and later phase handling. That means the recovered run is not fully treated as a fresh onboard even though the linked issue’s accepted path is to auto-recover by treating the interrupted state as fresh.
• Impact: Installer-style reruns can avoid the old early exit while still reporting and executing downstream onboarding as a resume. That can produce misleading user output/telemetry and keeps resume compatibility/backstop behavior active for a newly-created session, leaving the fix only partially aligned with the issue expectation.
• Required action: Make the bootstrap/caller contract carry the effective mode after recovery, for example by returning an `effectiveResume`/`recoveredFresh` flag or by performing this recovery at a layer that can reset the caller’s `resume` value. Use that effective value for lifecycle recording, banners, and onboarding flow context. Also document the recovery condition/removal condition near the fallback so the stale-state workaround has a clear source of truth.
• Expected follow-up: Fix before merge or get explicit maintainer override.
• Verification: Read `src/lib/onboard.ts` around the `prepareOnboardSession(...)` call and the subsequent uses of the original `resume` variable; confirm none of those paths are updated by the new `prepareFreshSession({ ...input, fresh: true }, deps)` return.
• Missing regression test: Add a caller-level mocked test for an auto-detected `in_progress` session with no sandbox name and `cannotPrompt: true` that proves the recovered run is recorded/rendered/passed to phase context as fresh rather than resume.
• Done when: The required change is committed and verification passes: Read `src/lib/onboard.ts` around the `prepareOnboardSession(...)` call and the subsequent uses of the original `resume` variable; confirm none of those paths are updated by the new `prepareFreshSession({ ...input, fresh: true }, deps)` return.
• Evidence: The new branch at `src/lib/onboard/session-bootstrap.ts` returns `prepareFreshSession({ ...input, fresh: true }, deps)`, but `OnboardSessionBootstrapResult` still only contains `{ session, fromDockerfile }`; the unchanged caller continues to use its pre-bootstrap `resume` variable.

Review findings by urgency: 1 required fix, 1 item to resolve/justify, 0 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: Non-interactive/cannotPrompt resume recovery in `prepareResumeSession()`

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: The changed unit test proves bootstrap clears the stale session and creates a new one. A caller-level test is still needed to prove the recovered run is treated as fresh after bootstrap.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: Comments at `src/lib/onboard/session-bootstrap.ts` describe auto-recovery, and the test at `src/lib/onboard/session-bootstrap.test.ts` covers the local bootstrap behavior, but the result type does not expose the effective mode.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

• None.

Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

• PRA-T1 Mocked behavioral coverage — auto-recovered stale resume is recorded, bannered, and passed to onboarding phases as fresh/effectiveResume false. The changed code is state/process-boundary onboarding logic. The added unit tests use mocks appropriately for bootstrap behavior, but confidence is incomplete because the recovery decision changes the callee contract and needs caller-level behavioral coverage.
• PRA-T2 Mocked behavioral coverage — cannotPrompt stale resume without nonInteractive either auto-recovers intentionally or has documented behavior matching the `cannotPrompt` predicate. The changed code is state/process-boundary onboarding logic. The added unit tests use mocks appropriately for bootstrap behavior, but confidence is incomplete because the recovery decision changes the callee contract and needs caller-level behavioral coverage.
• PRA-T3 Acceptance clause — When `nemoclaw onboard --non-interactive` is run on a host where a previous onboarding session was interrupted before sandbox creation completed, the installer finds the stale session state and aborts with "Cannot resume non-interactive onboard: the previous run was interrupted before sandbox creation completed, so no sandbox name was recorded." — add test evidence or identify existing coverage. The bootstrap unit now covers a stale session with `sandboxName: null`, `cannotPrompt: true`, and `nonInteractive: true`, and asserts `exitProcess` is not called. Caller-level effective fresh behavior is not covered.
• PRA-T4 Acceptance clause — The user is given no automatic recovery path; the non-interactive flow cannot recover from this state without manual intervention. — add test evidence or identify existing coverage. `prepareResumeSession()` now clears the stale session and creates a fresh session in the no-recoverable-sandbox-name branch. However, downstream onboarding still receives the original `resume` mode from the caller.
• PRA-T5 Acceptance clause — 1. Run `nemoclaw onboard` (interactive or non-interactive) and interrupt it before sandbox creation completes (e.g. Ctrl-C at step [2/8] or [3/8]). — add test evidence or identify existing coverage. The test models the durable symptom, an in-progress session with no sandbox name, but does not exercise the full interruption/caller path.
• PRA-T6 Acceptance clause — 2. Re-run: ```bash curl -fsSL https://www.nvidia.com/nemoclaw.sh | NEMOCLAW_NON_INTERACTIVE=1 NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1 bash ``` — add test evidence or identify existing coverage. The bootstrap test sets `cannotPrompt: true` and `nonInteractive: true`, but no installer-style entry/caller test proves auto-detected resume is converted to effective fresh mode.
• PRA-T7 Acceptance clause — Non-interactive onboard should either: — add test evidence or identify existing coverage. The PR chooses the auto-recovery route, but only the bootstrap session creation is fresh; downstream caller state remains resume.
• PRA-T8 Non-interactive/cannotPrompt resume recovery in `prepareResumeSession()` — The changed unit test proves bootstrap clears the stale session and creates a new one. A caller-level test is still needed to prove the recovered run is treated as fresh after bootstrap.. Comments at `src/lib/onboard/session-bootstrap.ts` describe auto-recovery, and the test at `src/lib/onboard/session-bootstrap.test.ts` covers the local bootstrap behavior, but the result type does not expose the effective mode.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

[jason-ma-nv]

Superseded by #5641. The original approach changed the CLI resume bootstrap (session-bootstrap.ts) to auto-recover, which reversed the deliberate non-interactive resume guard added in #2753 (and broke its tests). #5641 instead fixes the root cause at the source — install.sh was auto-attaching --resume to a session that had nothing to resume — so the curl|bash dead-end is fixed while the #2753 guard stays intact. Closing in favor of #5641.