Repository navigation
fix(onboard): auto-detect resume from an in_progress session - #5487
Merged
Merged
Conversation
Resume mode was keyed only off the --resume flag (entry-options.ts), so re-running `nemoclaw onboard` after an interrupted attempt — where ~/.nemoclaw/onboard-session.json has status:"in_progress" — started over instead of resuming: no "(resume mode)" banner and no resume preflight. Auto-detect resume from the persisted in_progress status when --fresh is not set. --fresh always wins, and the explicit --resume/--fresh conflict guard is unchanged (it applies only to the explicit flags), so no existing flow regresses. Scope: this resolves the resume-detection symptom of #5470. The stale gateway.json reuse symptom is rooted in OpenShell's package-managed gateway lifecycle — gateway.json is OpenShell-owned and NemoClaw reuses on live `openshell status` health, so forcing a teardown would regress the legitimate host-process reuse path. Left out deliberately; hence Refs (not Closes). Local pre-commit Test (CLI) hook skipped via --no-verify (operator-approved): it fails only on environment-flaky tests (ollama proxy 5s timeout, /tmp symlink) unrelated to this change; entry-options 9/9 and the onboard suite 1876/1876 pass, typecheck and biome clean. CI re-runs all hooks on the PR. Refs #5470 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesPersisted Session Auto-Resume
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: Dispatch required Vitest E2E scenarios:
Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
Contributor
PR Review AdvisorReview posture: Resolve findings before merge Review findings by urgency: 0 required fixes, 2 items to resolve/justify, 0 in-scope improvements🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
![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: 1
🧹 Nitpick comments (2)
src/lib/onboard/entry-options.ts (1)
21-21: ⚡ Quick winTighten
persistedSessionStatusto a literal union instead ofstring.Using
string | nullweakens the cross-file contract and allows typos to compile. Narrow this to known session status literals (or a shared exported status type) so auto-resume conditions stay type-safe.🤖 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/entry-options.ts` at line 21, The persistedSessionStatus field is typed as string | null, which is too permissive and allows any string value, including typos. Replace this broad string type with a literal union of specific session status values (for example, 'active' | 'paused' | 'resumed') or reference a shared exported status type that captures the valid session states. This ensures type safety at compile time and prevents invalid status values from being assigned to persistedSessionStatus.src/lib/onboard.ts (1)
4750-4752: Run the onboarding E2E suite for this core-flow change before merge.Given this touches onboarding entry behavior, run the recommended targeted nightly E2E jobs to validate resume/fresh interactions across lifecycle paths.
gh workflow run nightly-e2e.yaml --ref <branch> -f jobs=cloud-e2e,sandbox-operations-e2e,rebuild-openclaw-e2e,channels-stop-start-e2e,messaging-compatible-endpoint-e2e,bedrock-runtime-compatible-anthropic-e2e,hermes-discord-e2e,hermes-slack-e2e,openshell-gateway-upgrade-e2e,issue-3600-gpu-proof-optional-e2eAs per coding guidelines,
src/lib/onboard.tschanges should run the recommended onboarding E2E coverage set.🤖 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.ts` around lines 4750 - 4752, Before merging the changes to src/lib/onboard.ts that modify the onboarding entry behavior with persistedSessionStatus initialization for resume detection, you must run the recommended onboarding E2E test coverage to validate that resume and fresh interaction paths work correctly across different lifecycle scenarios. Execute the provided gh workflow command to run the nightly E2E jobs (cloud-e2e, sandbox-operations-e2e, rebuild-openclaw-e2e, channels-stop-start-e2e, messaging-compatible-endpoint-e2e, bedrock-runtime-compatible-anthropic-e2e, hermes-discord-e2e, hermes-slack-e2e, openshell-gateway-upgrade-e2e, and issue-3600-gpu-proof-optional-e2e) on your branch to ensure the onboarding resume/fresh interaction changes are validated before merge.Source: Coding guidelines
🤖 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.ts`:
- Around line 4750-4751: The file `src/lib/onboard.ts` has exceeded the growth
guard threshold with a net addition of 2 lines, which blocks CI from passing. To
fix this, you need to offset the 2 newly added lines (the comment and the
persistedSessionStatus assignment) by removing or trimming 2 equivalent lines of
non-functional content elsewhere in the same file, such as unnecessary comments,
blank lines, or redundant whitespace. Ensure the deletions do not affect any
functional code logic.
---
Nitpick comments:
In `@src/lib/onboard.ts`:
- Around line 4750-4752: Before merging the changes to src/lib/onboard.ts that
modify the onboarding entry behavior with persistedSessionStatus initialization
for resume detection, you must run the recommended onboarding E2E test coverage
to validate that resume and fresh interaction paths work correctly across
different lifecycle scenarios. Execute the provided gh workflow command to run
the nightly E2E jobs (cloud-e2e, sandbox-operations-e2e, rebuild-openclaw-e2e,
channels-stop-start-e2e, messaging-compatible-endpoint-e2e,
bedrock-runtime-compatible-anthropic-e2e, hermes-discord-e2e, hermes-slack-e2e,
openshell-gateway-upgrade-e2e, and issue-3600-gpu-proof-optional-e2e) on your
branch to ensure the onboarding resume/fresh interaction changes are validated
before merge.
In `@src/lib/onboard/entry-options.ts`:
- Line 21: The persistedSessionStatus field is typed as string | null, which is
too permissive and allows any string value, including typos. Replace this broad
string type with a literal union of specific session status values (for example,
'active' | 'paused' | 'resumed') or reference a shared exported status type that
captures the valid session states. This ensures type safety at compile time and
prevents invalid status values from being assigned to persistedSessionStatus.
🪄 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: 93057bc6-750d-48d8-9238-4ef0d841519d
📒 Files selected for processing (3)
src/lib/onboard.tssrc/lib/onboard/entry-options.test.tssrc/lib/onboard/entry-options.ts
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 27589221225
|
…ests - Offset the codebase-growth-guardrails check: src/lib/onboard.ts must be net-neutral. Drop the inline comment (the rationale already lives on entry-options.ts) and a decorative blank line so the file is +1/-1 (net 0) while keeping the persistedSessionStatus wiring. - Widen the non-resume boundary test to cover "pending" and "" in addition to complete/failed/null/undefined, so only an exact "in_progress" auto-resumes. Refs #5470 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
test-onboard-resume.sh only exercised explicit `nemoclaw onboard --resume`. Add a phase that re-marks the session in_progress and runs a plain `onboard` (no --resume), asserting the "(resume mode)" banner and skipped cached steps, then verifies `--fresh` suppresses the auto-resume (fail-fast at preflight to stay cheap). Closes the implicit-resume gap flagged by the e2e advisor on #5470. Refs #5470 Signed-off-by: Dongni Yang <dongniy@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 `@test/e2e/test-onboard-resume.sh`:
- Around line 375-390: The test in the --fresh branch injects a preflight
failure using NEMOCLAW_E2E_FORCE_FAIL_AT_STEP=preflight but never verifies the
injection actually occurred. The current code only checks that "(resume mode)"
is absent, which is insufficient. Modify the test to assert both that the
command exits with a non-zero status (indicating the preflight failure) and that
the expected failure marker appears in the output. Remove the || true from the
nemoclaw.js invocation to capture the exit code, then add assertions to verify
the non-zero exit code and check for the expected preflight failure marker in
fresh_output to ensure the injection regressed detection works correctly.
🪄 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: a093d1cc-2baf-4383-b3a0-75555d7a0f63
📒 Files selected for processing (3)
src/lib/onboard.tssrc/lib/onboard/entry-options.test.tstest/e2e/test-onboard-resume.sh
💤 Files with no reviewable changes (1)
- src/lib/onboard.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/onboard/entry-options.test.ts
…heck CodeRabbit: the --fresh branch only checked that "(resume mode)" was absent, which could pass vacuously if the run failed for an unrelated reason or never reached preflight. Drop the `|| true`, capture the exit code, and assert the run exited non-zero with the injected "Forced onboarding failure at step 'preflight'" marker, so the banner-absence assertion is meaningful. Refs #5470 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
Contributor
Selective E2E Results — ❌ Some jobs failedRun: 27591288160
|
The Phase 3.5 implicit-resume check flipped the post-Phase-3 session's status to in_progress but left resumable=false (set when onboarding completes), so the resume machine correctly rejected it as "no resumable session" and exited before the banner — failing the assertions. A genuinely interrupted session is resumable; reset resumable=true so the synthesized state matches and plain onboard auto-resumes it. (The fix itself was sound — auto-detect fired, which is why the resume guard ran at all.) Refs #5470 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 27591945815
|
Contributor
Author
E2E status
Scope reminderClosing keyword stays Signed-off-by: Dongni Yang dongniy@nvidia.com |
cv
approved these changes
Jun 22, 2026
This was referenced Jun 25, 2026
jyaunches
added a commit
that referenced
this pull request
Jun 25, 2026
## Summary Restore issue #5800 parity package `P0-B` for merged onboard/rebuild/lifecycle bash-suite deltas only. ## Related Issues Refs #5800 Refs #5098 Refs #5225 Refs #5487 Refs #5410 Refs #5760 ## Scope gate - Package: `P0-B — Onboard/rebuild/lifecycle parity` - Included PRs all merged and touched `test/e2e`: yes — #5225, #5487, #5410, #5760 - Out of scope: unmerged/non-bash PRs; shell lane retirement / PR #5756 cleanup ## Parity map | ID | Source PR | Contract | Inference classification | Vitest assertion / waiver | Status | | --- | --- | --- | --- | --- | --- | | B1 | #5225 | Persisted sandbox-entry gateway resolution is used by lifecycle commands. | `none` | Existing `src/lib/actions/sandbox/sandbox-gateway-routing.test.ts`, `src/lib/onboard/gateway-binding.test.ts`, `src/lib/onboard/sandbox-registration.test.ts` | covered | | B2 | #5225 | Onboard repair/double-onboard failures include captured onboard output diagnostics. | `none` | Existing live `test/e2e-scenario/live/onboard-repair.test.ts`, `test/e2e-scenario/live/double-onboard.test.ts`; diagnostics are bash-runner-only verbosity and not a durable Vitest assertion. | waived | | B3 | #5487 | Plain `nemoclaw onboard` auto-detects an `in_progress` session and resumes without `--resume`; `--fresh` suppresses auto-resume. | `hosted-compatible capable` | `test/e2e-scenario/live/onboard-resume.test.ts` Phase 3.5 mutates the completed session to `in_progress`, asserts `(resume mode)` + cached skips, then asserts `--fresh` fails at injected preflight without resume banner. | covered | | B4 | #5410 | Rebuild resumes messaging from `messaging.plan` rather than legacy `providerCredentialHashes`; stale top-level provider hash state is not used. | `none` | `test/e2e-scenario/live/rebuild-hermes.test.ts` curated registry omits `providerCredentialHashes`; `src/lib/onboard/machine/handlers/sandbox.test.ts` refreshes registry-plan credential hashes from env on rebuild resume. | covered | | B5 | #5410 | Empty/staged rebuild messaging plan is preserved and token-backed channels are not rediscovered. | `none` | Existing `src/lib/actions/sandbox/rebuild-messaging-stage.test.ts` and `src/lib/onboard/machine/handlers/sandbox.test.ts`. | covered | | B6 | #5760 | Hosted-inference/messaging rebuild and live answer assertions tolerate model whitespace around integer `42`. | `hosted-compatible capable` | Existing `test/helpers/e2e-answer-assertions.test.ts`, consumed by `agent-turn-latency`, `full-e2e`, `launchable-smoke`, and `sandbox-operations` live Vitests. | covered | | B7 | #5760 | Stabilized hosted inference and messaging rebuild remain validated by live full/sandbox/launchable/rebuild targets. | `hosted-compatible capable` | Existing live targets remain unchanged; local live execution blocked by Docker daemon unavailable. Selective workflow required after PR opens. | follow-up | ## Inference mode support - Default mode for touched live targets: `hosted-compatible capable` for onboard-resume/rebuild-hermes/full/sandbox/launchable answer paths; `none` for unit/process registry and gateway routing tests. - Real inference support preserved: yes for existing hosted-compatible live targets; no new inference adapter seam added here. - Modes validated in this PR: local unit/process Vitests only; live hosted-compatible validation needs GitHub runner/secrets because local Docker daemon is unavailable. - If not validated with real inference: local `NEMOCLAW_RUN_E2E_SCENARIOS=1 npx vitest run --project e2e-scenarios-live test/e2e-scenario/live/onboard-resume.test.ts test/e2e-scenario/live/rebuild-hermes.test.ts` failed at prereq Docker daemon check before scenario assertions. ## Validation - [x] `git diff --check` - [x] `npm test -- src/lib/onboard/machine/handlers/sandbox.test.ts test/helpers/e2e-answer-assertions.test.ts src/lib/actions/sandbox/sandbox-gateway-routing.test.ts src/lib/onboard/entry-options.test.ts src/lib/onboard/sandbox-registration.test.ts src/lib/actions/sandbox/rebuild-messaging-stage.test.ts` - [x] `npm run typecheck:cli` - [x] `npm run build:cli` - [x] `npm run test-size:check` - [ ] hosted-compatible selective E2E workflow: pending PR / runner dispatch ## Follow-ups / waivers - Waiver B2: bash-only diagnostic verbosity from #5225 is not a durable Vitest contract; existing live tests already preserve the functional repair/double-onboard behavior. - Follow-up B7: dispatch selective live Vitest scenarios on GitHub runner with Docker + hosted inference secret after PR opens. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added coverage for sandbox resume flows to verify updated Telegram credentials are picked up when resuming a rebuild. * Expanded end-to-end onboarding resume scenarios to confirm implicit resume behavior, including skipped cached steps and fresh runs starting from the expected point. * Strengthened rebuild scenario checks to ensure curated registry entries no longer include legacy credential-hash data. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
## Summary Restore issue NVIDIA#5800 parity package `P0-B` for merged onboard/rebuild/lifecycle bash-suite deltas only. ## Related Issues Refs NVIDIA#5800 Refs NVIDIA#5098 Refs NVIDIA#5225 Refs NVIDIA#5487 Refs NVIDIA#5410 Refs NVIDIA#5760 ## Scope gate - Package: `P0-B — Onboard/rebuild/lifecycle parity` - Included PRs all merged and touched `test/e2e`: yes — NVIDIA#5225, NVIDIA#5487, NVIDIA#5410, NVIDIA#5760 - Out of scope: unmerged/non-bash PRs; shell lane retirement / PR NVIDIA#5756 cleanup ## Parity map | ID | Source PR | Contract | Inference classification | Vitest assertion / waiver | Status | | --- | --- | --- | --- | --- | --- | | B1 | NVIDIA#5225 | Persisted sandbox-entry gateway resolution is used by lifecycle commands. | `none` | Existing `src/lib/actions/sandbox/sandbox-gateway-routing.test.ts`, `src/lib/onboard/gateway-binding.test.ts`, `src/lib/onboard/sandbox-registration.test.ts` | covered | | B2 | NVIDIA#5225 | Onboard repair/double-onboard failures include captured onboard output diagnostics. | `none` | Existing live `test/e2e-scenario/live/onboard-repair.test.ts`, `test/e2e-scenario/live/double-onboard.test.ts`; diagnostics are bash-runner-only verbosity and not a durable Vitest assertion. | waived | | B3 | NVIDIA#5487 | Plain `nemoclaw onboard` auto-detects an `in_progress` session and resumes without `--resume`; `--fresh` suppresses auto-resume. | `hosted-compatible capable` | `test/e2e-scenario/live/onboard-resume.test.ts` Phase 3.5 mutates the completed session to `in_progress`, asserts `(resume mode)` + cached skips, then asserts `--fresh` fails at injected preflight without resume banner. | covered | | B4 | NVIDIA#5410 | Rebuild resumes messaging from `messaging.plan` rather than legacy `providerCredentialHashes`; stale top-level provider hash state is not used. | `none` | `test/e2e-scenario/live/rebuild-hermes.test.ts` curated registry omits `providerCredentialHashes`; `src/lib/onboard/machine/handlers/sandbox.test.ts` refreshes registry-plan credential hashes from env on rebuild resume. | covered | | B5 | NVIDIA#5410 | Empty/staged rebuild messaging plan is preserved and token-backed channels are not rediscovered. | `none` | Existing `src/lib/actions/sandbox/rebuild-messaging-stage.test.ts` and `src/lib/onboard/machine/handlers/sandbox.test.ts`. | covered | | B6 | NVIDIA#5760 | Hosted-inference/messaging rebuild and live answer assertions tolerate model whitespace around integer `42`. | `hosted-compatible capable` | Existing `test/helpers/e2e-answer-assertions.test.ts`, consumed by `agent-turn-latency`, `full-e2e`, `launchable-smoke`, and `sandbox-operations` live Vitests. | covered | | B7 | NVIDIA#5760 | Stabilized hosted inference and messaging rebuild remain validated by live full/sandbox/launchable/rebuild targets. | `hosted-compatible capable` | Existing live targets remain unchanged; local live execution blocked by Docker daemon unavailable. Selective workflow required after PR opens. | follow-up | ## Inference mode support - Default mode for touched live targets: `hosted-compatible capable` for onboard-resume/rebuild-hermes/full/sandbox/launchable answer paths; `none` for unit/process registry and gateway routing tests. - Real inference support preserved: yes for existing hosted-compatible live targets; no new inference adapter seam added here. - Modes validated in this PR: local unit/process Vitests only; live hosted-compatible validation needs GitHub runner/secrets because local Docker daemon is unavailable. - If not validated with real inference: local `NEMOCLAW_RUN_E2E_SCENARIOS=1 npx vitest run --project e2e-scenarios-live test/e2e-scenario/live/onboard-resume.test.ts test/e2e-scenario/live/rebuild-hermes.test.ts` failed at prereq Docker daemon check before scenario assertions. ## Validation - [x] `git diff --check` - [x] `npm test -- src/lib/onboard/machine/handlers/sandbox.test.ts test/helpers/e2e-answer-assertions.test.ts src/lib/actions/sandbox/sandbox-gateway-routing.test.ts src/lib/onboard/entry-options.test.ts src/lib/onboard/sandbox-registration.test.ts src/lib/actions/sandbox/rebuild-messaging-stage.test.ts` - [x] `npm run typecheck:cli` - [x] `npm run build:cli` - [x] `npm run test-size:check` - [ ] hosted-compatible selective E2E workflow: pending PR / runner dispatch ## Follow-ups / waivers - Waiver B2: bash-only diagnostic verbosity from NVIDIA#5225 is not a durable Vitest contract; existing live tests already preserve the functional repair/double-onboard behavior. - Follow-up B7: dispatch selective live Vitest scenarios on GitHub runner with Docker + hosted inference secret after PR opens. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added coverage for sandbox resume flows to verify updated Telegram credentials are picked up when resuming a rebuild. * Expanded end-to-end onboarding resume scenarios to confirm implicit resume behavior, including skipped cached steps and fresh runs starting from the expected point. * Strengthened rebuild scenario checks to ensure curated registry entries no longer include legacy credential-hash data. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
62 of 82 tasks
wscurran
added
area: cli
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
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
Resume mode was keyed only off the
--resumeflag, so re-runningnemoclaw onboardafter an interrupted attempt — where~/.nemoclaw/onboard-session.jsonhasstatus:"in_progress"— started over instead of resuming (no(resume mode)banner, no resume preflight). This auto-detects resume from the persisted in_progress status.Related Issue
Refs #5470
Changes
src/lib/onboard/entry-options.ts:resolveOnboardEntryOptionsnow auto-detects resume from a persistedin_progresssession (new optionalpersistedSessionStatusinput) when--freshis not set.--freshalways wins; the explicit--resume/--freshconflict guard is unchanged (applies only to the explicit flags).src/lib/onboard.ts: pass the persisted session status into the resolver.src/lib/onboard/entry-options.test.ts: red→green coverage for auto-detect, plus no-regression guards (--freshwins; non-in_progressstatuses).test/e2e/test-onboard-resume.sh: add a Phase 3.5 implicit-resume scenario (plainonboardon anin_progresssession) and a--freshsuppression check.Type of Change
Verification
entry-options.test.ts)trueforin_progress;--freshand non-in_progress(incl.pending/"") → no resume. Core case is a true red→green. ✅gateway --helplacksstart/destroy⇒supportsLifecycleCommands=false(host-process model — the bug-capable branch). ✅classifyGatewayPortReuse(healthy, lifecycle=false, container=missing)⇒reuse; container never probed, no cleanup; reuse isopenshell status-driven, nocontainerIdread. ✅onboard(Ubuntu 20.04, containerized-compat gateway)onboard(no--resume) on anin_progresssession prints(resume mode)(the fix), then[resume] Skipping gateway (running)against a healthy:8080gateway with no stale-cleanup sequence — reproducing both the fix and the unfixed half end-to-end. ✅onboard-resume-e2e(Ubuntu 24.04 CI)--resumeplus the new Phase 3.5 implicit-resume +--fresh-suppression assertions all pass. ✅Scope / notes: NemoClaw writes/reads no
gateway.jsonorcontainerIdin the onboard reuse path (grep-confirmed) — that artifact is OpenShell-owned, so the stale-gateway cleanup symptom is upstream. It also can't be tightened naively without regressing #4520 (the inverse "false-stale →:8080collision" bug, whose fix deliberately made reuse more trusting). HenceRefs, notCloses. Thecloud-onboard-e2efailure is the inference-provider credential/env issue the report itself calls "environmental and separate from this bug" (it fails on a fresh onboard at[3/8], where this change's resume path never runs).Signed-off-by: Dongni Yang dongniy@nvidia.com
Summary by CodeRabbit
in_progress, reducing manual--resumeusage.--freshstill overrides auto-resume and prevents resume mode even if the persisted session isin_progress.in_progress,complete,failed,null/undefined) and validating--freshsuppression.--freshmode.