fix(install): refuse to auto-resume a failed onboarding session#2437
Merged
fix(install): refuse to auto-resume a failed onboarding session#2437
Conversation
The installer used to auto-attach --resume to nemoclaw onboard whenever a non-complete session file existed. A run that failed at step 3 or 4 (for example Ollama proxy unreachable) writes status=failed with resumable=true, which matched that predicate — so re-running curl | bash silently replayed the same failed choice. The user had no way to pick a different provider short of deleting ~/.nemoclaw/onboard-session.json. Classify the session into resume / failed / skip / corrupt and branch on it: - resume (status=in_progress): auto-attach --resume as before (legit Ctrl-C case). - failed: in a TTY, prompt R/f; in non-interactive mode, refuse with a non-zero exit so scripted re-runs cannot loop. The refusal message points at --fresh and 'nemoclaw onboard --resume' as the two explicit opt-ins. - skip (complete / resumable=false / missing): no --resume. - corrupt: warn and skip rather than auto-resume an unreadable file. Add --fresh (and NEMOCLAW_FRESH=1) as the scripted escape hatch. It short-circuits the classifier; the downstream onboard call already overwrites the session file via createSession + saveSession, so no cleanup is needed here. Docs: troubleshooting entry under Onboarding covers the new flows. Also pick up a pre-existing drift in the autogenerated .agents/skills/.../commands.md heading level for `openshell term` — the docs-to-skills pre-commit hook regenerates it whenever any docs/*.md changes, so it is unavoidable in this commit. Resolves #2430. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
|
Important Review skippedThis PR was authored by the user configured for CodeRabbit reviews. CodeRabbit does not review PRs authored by this user. It's recommended to use a dedicated user account to post CodeRabbit review feedback. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a Changes
Sequence DiagramsequenceDiagram
participant User
participant Installer as scripts/install.sh
participant SessionFile as ~/.nemoclaw/onboard-session.json
participant OnboardCmd as nemoclaw onboard
User->>Installer: Run installer (e.g., curl | bash)
Installer->>SessionFile: Attempt to read session JSON
alt session file exists
Installer->>Installer: Classify (resume / failed / skip / corrupt)
alt classified == failed and Non-Interactive
Installer->>User: Print error and exit non-zero (referencing --fresh)
else classified == failed and Interactive (TTY)
Installer->>User: Prompt "Resume [R]/Fresh [f]?"
User->>Installer: Choice
alt Resume
Installer->>OnboardCmd: nemoclaw onboard --resume
else Fresh
Installer->>OnboardCmd: nemoclaw onboard --fresh
OnboardCmd->>SessionFile: Clear or overwrite session
end
else classified == resume
Installer->>OnboardCmd: nemoclaw onboard --resume
end
else no session file or --fresh provided
Installer->>OnboardCmd: nemoclaw onboard (normal start)
end
OnboardCmd->>User: Execute onboarding flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 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. Comment |
![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: 2
🧹 Nitpick comments (3)
.agents/skills/nemoclaw-user-reference/references/commands.md (1)
538-538: Please apply this heading-level change in the source docs, not the generated skills file.This file is autogenerated and may be overwritten on regeneration; move this edit to the corresponding
docs/source path and re-generate skills artifacts.Based on learnings: markdown files under
.agents/skills/are autogenerated from corresponding content underdocs/bydocs-to-skills.pyand are regenerated on each run.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/nemoclaw-user-reference/references/commands.md at line 538, The heading level change for "openshell term" must be made in the original docs source (not the autogenerated .agents/skills file); locate the source Markdown entry that defines the "openshell term" heading in the docs/ content, adjust its heading level to the desired level, then re-run the docs-to-skills.py generator to regenerate the skills artifacts; do not edit the autogenerated `.agents/skills/nemoclaw-user-reference/references/commands.md` directly.test/install-preflight.test.ts (1)
737-832: Add one companion test for theNEMOCLAW_FRESH=1path.The new coverage only exercises the CLI flag.
main()has a separate env-var precedence branch, and the docs now advertise that path too, so a tiny sibling case would keep the scripted escape hatch from drifting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/install-preflight.test.ts` around lines 737 - 832, Add a sibling test that mirrors the "--fresh skips auto-resume..." case but uses the NEMOCLAW_FRESH environment variable path: copy the existing it(...) block and remove the "--fresh" argument from the spawnSync invocation, set env to include NEMOCLAW_FRESH: "1" (keep HOME, PATH, NEMOCLAW_NON_INTERACTIVE, NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE, NPM_PREFIX, NEMOCLAW_ONBOARD_LOG as in the original), then assert the same expectations (exit status 0, stdout/stderr contains "Starting a fresh onboarding session", does not contain "Found an interrupted onboarding session", onboard log matches the non-resume invocation and does not include "--resume") so the env-var branch in main() is covered.docs/reference/troubleshooting.md (1)
301-306: This section trips a few docs style rules.The new prose mixes future tense (
will not), repeated em dashes in a paragraph, and bolded lead-in labels with colon punctuation for routine list items. Please normalize this to plain present-tense prose.As per coding guidelines "Present tense. Flag future tense ('will') in descriptions of current behavior.", "Excessive em dashes. One per paragraph is fine", "Unnecessary bold on routine instructions", and "Colons should only introduce a list."
Also applies to: 320-320
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/troubleshooting.md` around lines 301 - 306, Revise the prose in the troubleshooting section that describes handling failed "nemoclaw onboard" sessions to use present tense, remove duplicated em-dashes, and convert bold lead-in labels with trailing colons into plain sentence lead-ins; specifically update the paragraph mentioning "~/.nemoclaw/onboard-session.json" and the two list items currently labeled "Interactive terminal:" and "Non-interactive (piped `curl | bash` with `NEMOCLAW_NON_INTERACTIVE=1`, CI, scripts):" so they read in present tense, use at most one em-dash in the paragraph, and present the list items without bold or colon punctuation (e.g., "Interactive terminal — the installer prompts..." and "Non-interactive (piped curl | bash with NEMOCLAW_NON_INTERACTIVE=1, CI, scripts) — the installer exits with non-zero status...").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/reference/troubleshooting.md`:
- Around line 314-317: The documented command incorrectly sets NEMOCLAW_FRESH
for curl rather than the installer; update the example so NEMOCLAW_FRESH is
applied to the bash process on the right side of the pipeline (i.e., move the
environment assignment from the curl side to the bash invocation) so the
installer script sees the variable when running the curl -fsSL
https://www.nvidia.com/nemoclaw.sh | bash pipeline; alternatively describe using
a subshell that runs bash with NEMOCLAW_FRESH set so the installer receives the
environment variable.
In `@scripts/install.sh`:
- Around line 1215-1220: The auto-resume logic currently treats any unknown or
empty status as resumable; change the branch that sets out to "resume" so it
only does so when data.status === "in_progress" explicitly, and otherwise set
out to "failed" (or "corrupt") for any missing/unknown/stale status or malformed
session metadata; update the condition near the existing data/status checks (the
block that assigns out based on data.resumable and data.status) so it references
explicit "in_progress" only, and add a fallback path for unexpected values
(aligning with normalizeSession() and the statuses written by functions in
src/lib/onboard-session.ts).
---
Nitpick comments:
In @.agents/skills/nemoclaw-user-reference/references/commands.md:
- Line 538: The heading level change for "openshell term" must be made in the
original docs source (not the autogenerated .agents/skills file); locate the
source Markdown entry that defines the "openshell term" heading in the docs/
content, adjust its heading level to the desired level, then re-run the
docs-to-skills.py generator to regenerate the skills artifacts; do not edit the
autogenerated `.agents/skills/nemoclaw-user-reference/references/commands.md`
directly.
In `@docs/reference/troubleshooting.md`:
- Around line 301-306: Revise the prose in the troubleshooting section that
describes handling failed "nemoclaw onboard" sessions to use present tense,
remove duplicated em-dashes, and convert bold lead-in labels with trailing
colons into plain sentence lead-ins; specifically update the paragraph
mentioning "~/.nemoclaw/onboard-session.json" and the two list items currently
labeled "Interactive terminal:" and "Non-interactive (piped `curl | bash` with
`NEMOCLAW_NON_INTERACTIVE=1`, CI, scripts):" so they read in present tense, use
at most one em-dash in the paragraph, and present the list items without bold or
colon punctuation (e.g., "Interactive terminal — the installer prompts..." and
"Non-interactive (piped curl | bash with NEMOCLAW_NON_INTERACTIVE=1, CI,
scripts) — the installer exits with non-zero status...").
In `@test/install-preflight.test.ts`:
- Around line 737-832: Add a sibling test that mirrors the "--fresh skips
auto-resume..." case but uses the NEMOCLAW_FRESH environment variable path: copy
the existing it(...) block and remove the "--fresh" argument from the spawnSync
invocation, set env to include NEMOCLAW_FRESH: "1" (keep HOME, PATH,
NEMOCLAW_NON_INTERACTIVE, NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE, NPM_PREFIX,
NEMOCLAW_ONBOARD_LOG as in the original), then assert the same expectations
(exit status 0, stdout/stderr contains "Starting a fresh onboarding session",
does not contain "Found an interrupted onboarding session", onboard log matches
the non-resume invocation and does not include "--resume") so the env-var branch
in main() is covered.
🪄 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: 76d6df9a-a158-473c-8fd3-04690b10fa1d
📒 Files selected for processing (6)
.agents/skills/nemoclaw-user-reference/references/commands.md.agents/skills/nemoclaw-user-reference/references/troubleshooting.mddocs/reference/troubleshooting.mdinstall.shscripts/install.shtest/install-preflight.test.ts
Two follow-ups from the CodeRabbit pass on PR #2437: - scripts/install.sh: only classify a session as 'resume' when status is literally 'in_progress'. Previously any unknown or missing status (empty string, stale enum value, hand-edited file) fell through the else branch and auto-attached --resume. onboard-session.ts only writes in_progress / failed / complete, so anything else should be treated as 'corrupt' rather than silently resumed. - docs/reference/troubleshooting.md: env var assignments on the left of a pipe only apply to the left-hand process. 'NEMOCLAW_FRESH=1 curl ... | bash' set the variable for curl, which ignores it. Move the assignment to the bash side and add a note explaining why. Resolves #2430. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docs/reference/troubleshooting.md (2)
301-303: Use present tense and trim the em dashes in this section.This new prose switches to future tense (
will not) and uses multiple em dashes in a paragraph. LLM pattern detected.As per coding guidelines, "Present tense. Flag future tense ('will') in descriptions of current behavior." and "Excessive em dashes. One per paragraph is fine; multiple per paragraph or em dashes used instead of commas/periods should be flagged."
Also applies to: 320-320
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/troubleshooting.md` around lines 301 - 303, The paragraph about failed onboarding uses future tense and multiple em dashes; change "will not silently retry" to present tense "does not silently retry" and replace extra em dashes with commas or separate sentences so there is at most one em dash per paragraph; apply the same tense and punctuation fixes to the similar sentence noted at the other occurrence (the line referencing ~/.nemoclaw/onboard-session.json and the 'nemoclaw onboard' command).
305-306: Drop the bold lead-ins in these list items.
**Interactive terminal:**and**Non-interactive ...:**read as routine labels, not UI text or genuine warnings. LLM pattern detected.As per coding guidelines, "Unnecessary bold on routine instructions ... Bold is reserved for UI labels, parameter names, and genuine warnings."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/troubleshooting.md` around lines 305 - 306, Remove the unnecessary bold formatting from the lead-in labels in the two list items: replace "**Interactive terminal:**" with "Interactive terminal:" and "**Non-interactive (piped `curl | bash` with `NEMOCLAW_NON_INTERACTIVE=1`, CI, scripts):**" with "Non-interactive (piped `curl | bash` with `NEMOCLAW_NON_INTERACTIVE=1`, CI, scripts):" so the labels read as plain text rather than emphasized UI-style tokens; update the corresponding lines in docs/reference/troubleshooting.md containing those exact phrases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/install.sh`:
- Around line 1202-1204: The installer branch that checks FRESH only suppresses
auto-resume but does not forward the flag to the onboarding command; update the
install.sh logic so when FRESH (or --fresh) is set it adds the --fresh argument
to the subsequent call that runs "nemoclaw onboard" (the same invocation that
currently uses session_file and command_exists node) so the onboarding code
receives freshStart=true and will delete ~/.nemoclaw/onboard-session.json;
ensure the flag is appended consistently where the onboarding command is
constructed/executed so downstream src/lib/onboard-command.ts sees the
freshStart flag.
---
Nitpick comments:
In `@docs/reference/troubleshooting.md`:
- Around line 301-303: The paragraph about failed onboarding uses future tense
and multiple em dashes; change "will not silently retry" to present tense "does
not silently retry" and replace extra em dashes with commas or separate
sentences so there is at most one em dash per paragraph; apply the same tense
and punctuation fixes to the similar sentence noted at the other occurrence (the
line referencing ~/.nemoclaw/onboard-session.json and the 'nemoclaw onboard'
command).
- Around line 305-306: Remove the unnecessary bold formatting from the lead-in
labels in the two list items: replace "**Interactive terminal:**" with
"Interactive terminal:" and "**Non-interactive (piped `curl | bash` with
`NEMOCLAW_NON_INTERACTIVE=1`, CI, scripts):**" with "Non-interactive (piped
`curl | bash` with `NEMOCLAW_NON_INTERACTIVE=1`, CI, scripts):" so the labels
read as plain text rather than emphasized UI-style tokens; update the
corresponding lines in docs/reference/troubleshooting.md containing those exact
phrases.
🪄 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: f2baca7d-ddf8-4a58-86b5-4716c6841203
📒 Files selected for processing (3)
.agents/skills/nemoclaw-user-reference/references/troubleshooting.mddocs/reference/troubleshooting.mdscripts/install.sh
Follow-up on PR #2437 review feedback: - The --fresh flow stopped at install.sh: we suppressed auto-resume there but the downstream 'nemoclaw onboard' invocation did not know the user had asked for a fresh start. Add a --fresh flag on nemoclaw onboard that explicitly clearSession()s before createSession + saveSession. That makes the fresh intent explicit on disk (the old file is unlinked outright rather than overwritten in place) and lets the install.sh wrapper forward intent through to the CLI. - --fresh and --resume are mutually exclusive; both the CLI parser and onboard() guard against the combination. - install.sh forwards --fresh to nemoclaw onboard from both the FRESH=1 path and the interactive 'f' answer at the failed-session prompt. - Doc nitpicks on the troubleshooting section: present tense instead of future ('does not silently retry'), drop the bold from routine list labels (bold is reserved for UI labels and warnings), and trim em dashes to at most one per paragraph. Resolves #2430. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/reference/troubleshooting.md (2)
301-306: Multiple sentences per line violate documentation style guide.Lines 301, 303, 305, and 306 each contain multiple sentences on a single line. The style guide requires one sentence per line to make diffs more readable.
For example, line 301 should be split:
If a previous `nemoclaw onboard` attempt fails partway through (for example, a provider or inference-setup step reporting an error), NemoClaw records the failure in `~/.nemoclaw/onboard-session.json`.becomes:
If a previous `nemoclaw onboard` attempt fails partway through (for example, a provider or inference-setup step reporting an error), NemoClaw records the failure in `~/.nemoclaw/onboard-session.json`.Wait, that's already one sentence. Let me re-check line 303:
Line 303 has two sentences: "When you re-run the installer, it detects the failed session and does not silently retry it." and "Silent retry would loop on the same failure if your original choice, such as an unreachable provider, was the cause."
These should be on separate lines.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/troubleshooting.md` around lines 301 - 306, The documentation lines in docs/reference/troubleshooting.md violate the one-sentence-per-line rule: split any lines that contain multiple sentences so each sentence is on its own line. Specifically, edit the paragraph containing "If a previous `nemoclaw onboard`..." and the following sentence starting "When you re-run the installer..." so that the two sentences are on separate lines, and likewise ensure the sentences describing interactive and non-interactive behavior (the lines mentioning "Silent retry would loop..." and the non-interactive explanation about `NEMOCLAW_NON_INTERACTIVE=1`) are each placed on their own lines; keep the exact wording but break lines so each line contains exactly one sentence.
303-303: Split multiple sentences onto separate lines for diff readability.Per the documentation style guide, each sentence should be on its own line. Lines 303, 314, and 320 each contain two sentences that should be split.
Example fix for line 303
-When you re-run the installer, it detects the failed session and does not silently retry it. Silent retry would loop on the same failure if your original choice, such as an unreachable provider, was the cause. +When you re-run the installer, it detects the failed session and does not silently retry it. +Silent retry would loop on the same failure if your original choice, such as an unreachable provider, was the cause.Also applies to: 314-314, 320-320
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/troubleshooting.md` at line 303, Split sentences onto separate lines for improved diff readability: in the sentence starting "When you re-run the installer, it detects the failed session and does not silently retry it." break that into two lines so each sentence is on its own line; do the same for the paragraph containing the two sentences that start with the phrases found around lines 314 and 320 (each of those lines currently contains two sentences—ensure each sentence is placed on its own line). Ensure you only adjust line breaks (no wording changes) so each sentence becomes its own line in the docs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/reference/troubleshooting.md`:
- Around line 301-306: The documentation lines in
docs/reference/troubleshooting.md violate the one-sentence-per-line rule: split
any lines that contain multiple sentences so each sentence is on its own line.
Specifically, edit the paragraph containing "If a previous `nemoclaw
onboard`..." and the following sentence starting "When you re-run the
installer..." so that the two sentences are on separate lines, and likewise
ensure the sentences describing interactive and non-interactive behavior (the
lines mentioning "Silent retry would loop..." and the non-interactive
explanation about `NEMOCLAW_NON_INTERACTIVE=1`) are each placed on their own
lines; keep the exact wording but break lines so each line contains exactly one
sentence.
- Line 303: Split sentences onto separate lines for improved diff readability:
in the sentence starting "When you re-run the installer, it detects the failed
session and does not silently retry it." break that into two lines so each
sentence is on its own line; do the same for the paragraph containing the two
sentences that start with the phrases found around lines 314 and 320 (each of
those lines currently contains two sentences—ensure each sentence is placed on
its own line). Ensure you only adjust line breaks (no wording changes) so each
sentence becomes its own line in the docs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4933ac10-08e9-4073-b3a4-e12245647eb6
📒 Files selected for processing (7)
.agents/skills/nemoclaw-user-reference/references/troubleshooting.mddocs/reference/troubleshooting.mdscripts/install.shsrc/lib/onboard-command.test.tssrc/lib/onboard-command.tssrc/lib/onboard.tstest/install-preflight.test.ts
✅ Files skipped from review due to trivial changes (1)
- .agents/skills/nemoclaw-user-reference/references/troubleshooting.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
docs/reference/troubleshooting.md (2)
334-336: Avoid colon-separated label clauses in prose bullets.
Interactive terminal:andNon-interactive ...:use colons as clause separators rather than to introduce lists.As per coding guidelines, "Colons should only introduce a list. Flag colons used as general punctuation between clauses."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/troubleshooting.md` around lines 334 - 336, The two bullet labels currently use colon-separated clause forms ("Interactive terminal:" and "Non-interactive (piped `curl | bash` with `NEMOCLAW_NON_INTERACTIVE=1`, CI, scripts):"); change each bullet to remove the clause-colon by rephrasing them as normal prose or using parentheses/dashes (e.g., "Interactive terminal — the installer prompts..." or "In non-interactive mode (piped `curl | bash`, NEMOCLAW_NON_INTERACTIVE=1, CI, scripts) the installer refuses..."), updating the lines containing those exact label strings so colons are only used to introduce real lists.
332-334: Use one sentence per source line in this section.A couple of lines currently combine two sentences, which makes doc diffs harder to review.
As per coding guidelines, "One sentence per line in source (makes diffs readable). Flag paragraphs where multiple sentences appear on the same line."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/troubleshooting.md` around lines 332 - 334, The two source lines "When you re-run the installer, it detects the failed session and does not silently retry it. Silent retry would loop on the same failure if your original choice, such as an unreachable provider, was the cause." and the bullet "- Interactive terminal: the installer prompts whether to resume the failed session or start fresh. Press `R` (or Enter) to retry the same session, or `f` to discard it and make fresh choices." each contain two sentences on one line—split each sentence onto its own source line so there is exactly one sentence per line (e.g., break the first into two lines after "does not silently retry it." and break the bullet into two lines after "start fresh.") to satisfy the one-sentence-per-line guideline.src/lib/onboard-command.test.ts (1)
145-147: Replaceas nevercasts with existing typed exit helpers.Lines 145-147 can use
exitWithCode, and lines 172-174 can useexitWithPrefixedCodeinstead of inline arrow functions withas nevercasts. This removes unnecessary type assertions and improves consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/onboard-command.test.ts` around lines 145 - 147, Replace the inline exit arrow functions that use "as never" casts with the existing typed helpers: swap the anonymous exit: (code => { throw new Error(String(code)); }) as never with the exitWithCode helper, and similarly replace the other inline exit that throws a prefixed code with exitWithPrefixedCode; locate usages in this test file around the current exit callbacks and call exitWithCode(exit) or exitWithPrefixedCode(exit) as appropriate to remove the unsafe type assertions and use the typed helpers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/onboard.ts`:
- Around line 7266-7270: The code reads opts.fresh and assigns const fresh =
opts.fresh === true but the OnboardOptions type does not declare fresh; update
the OnboardOptions type (the interface/type that defines opts used by the
Onboard flow) to include fresh?: boolean so TypeScript knows this property
exists, then ensure any callers construct opts with the optional boolean or rely
on the existing default handling in the Onboard function (no runtime changes
required).
---
Nitpick comments:
In `@docs/reference/troubleshooting.md`:
- Around line 334-336: The two bullet labels currently use colon-separated
clause forms ("Interactive terminal:" and "Non-interactive (piped `curl | bash`
with `NEMOCLAW_NON_INTERACTIVE=1`, CI, scripts):"); change each bullet to remove
the clause-colon by rephrasing them as normal prose or using parentheses/dashes
(e.g., "Interactive terminal — the installer prompts..." or "In non-interactive
mode (piped `curl | bash`, NEMOCLAW_NON_INTERACTIVE=1, CI, scripts) the
installer refuses..."), updating the lines containing those exact label strings
so colons are only used to introduce real lists.
- Around line 332-334: The two source lines "When you re-run the installer, it
detects the failed session and does not silently retry it. Silent retry would
loop on the same failure if your original choice, such as an unreachable
provider, was the cause." and the bullet "- Interactive terminal: the installer
prompts whether to resume the failed session or start fresh. Press `R` (or
Enter) to retry the same session, or `f` to discard it and make fresh choices."
each contain two sentences on one line—split each sentence onto its own source
line so there is exactly one sentence per line (e.g., break the first into two
lines after "does not silently retry it." and break the bullet into two lines
after "start fresh.") to satisfy the one-sentence-per-line guideline.
In `@src/lib/onboard-command.test.ts`:
- Around line 145-147: Replace the inline exit arrow functions that use "as
never" casts with the existing typed helpers: swap the anonymous exit: (code =>
{ throw new Error(String(code)); }) as never with the exitWithCode helper, and
similarly replace the other inline exit that throws a prefixed code with
exitWithPrefixedCode; locate usages in this test file around the current exit
callbacks and call exitWithCode(exit) or exitWithPrefixedCode(exit) as
appropriate to remove the unsafe type assertions and use the typed helpers.
🪄 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: dd2a4c09-f59e-433c-9ba5-db702ec883c4
📒 Files selected for processing (5)
.agents/skills/nemoclaw-user-reference/references/troubleshooting.mddocs/reference/troubleshooting.mdsrc/lib/onboard-command.test.tssrc/lib/onboard.tstest/install-preflight.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- .agents/skills/nemoclaw-user-reference/references/troubleshooting.md
- test/install-preflight.test.ts
The TS2339 from npm run build:cli on macos-e2e and wsl-e2e was caused by
this branch: src/lib/onboard.ts reads opts.fresh on line 7266 but the
OnboardOptions type didn't declare it, so tsc -p tsconfig.src.json
failed before the e2e step could run. Add fresh?: boolean to the type.
While here, pick up the remaining CodeRabbit nits on the latest review:
- docs/reference/troubleshooting.md: split multi-sentence lines so each
sentence sits on its own line and rephrase the colon-as-clause
labels in the bullet list ("Interactive terminal:" /
"Non-interactive (...):") into prose sentences, since colons are
reserved for introducing real lists.
- src/lib/onboard-command.test.ts: replace the inline (code => { throw
new Error(...) }) as never casts with the existing exitWithCode and
exitWithPrefixedCode helpers.
The autogenerated .agents/skills/.../troubleshooting.md is regenerated
to match by the docs-to-skills pre-commit hook.
Resolves #2430.
Signed-off-by: Tinson Lai <tinsonl@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 the current code and only fix it if needed.
Inline comments:
In @.agents/skills/nemoclaw-user-reference/references/troubleshooting.md:
- Around line 302-339: The new "Previous onboarding session failed" section was
added to the generated file
.agents/skills/nemoclaw-user-reference/references/troubleshooting.md but this
repo generates that file from docs/ via docs-to-skills.py; update the canonical
source instead (likely docs/troubleshooting.md) by adding the new section text
(interactive vs non-interactive notes, --fresh examples, NEMOCLAW_FRESH env var
placement note, --resume and rm ~/.nemoclaw/onboard-session.json), then run
docs-to-skills.py to regenerate the .agents/.../troubleshooting.md so the change
persists and the generated file is not overwritten on the next run.
🪄 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: da65329a-465d-45b8-9e0c-859d823890a8
📒 Files selected for processing (4)
.agents/skills/nemoclaw-user-reference/references/troubleshooting.mddocs/reference/troubleshooting.mdsrc/lib/onboard-command.test.tssrc/lib/onboard.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/reference/troubleshooting.md
- src/lib/onboard-command.test.ts
cv
approved these changes
Apr 25, 2026
3 tasks
13 tasks
miyoungc
added a commit
that referenced
this pull request
Apr 28, 2026
## Summary Refreshes user-facing docs for the last 24 hours of merged NemoClaw history and bumps the docs metadata to 0.0.29, the next version after v0.0.28. The updates are limited to behavior supported by merged PR descriptions and diffs. ## Changes - `docs/reference/commands.md`: documented `nemoclaw <name> policy-add --from-file` and `--from-dir`, including custom preset review guidance, from #2077 / commit `7720b175`. - `docs/deployment/deploy-to-remote-gpu.md`: clarified that non-loopback `CHAT_UI_URL` disables OpenClaw device pairing for remote browser-only deployments, from #2449 / commit `f5ee8a4d`. - `docs/inference/inference-options.md`: documented provider-aware credential retry validation and the NVIDIA-only `nvapi-` prefix check, from #2389 / commit `6f7f0c6d`. - `docs/inference/switch-inference-providers.md`: documented `NEMOCLAW_INFERENCE_INPUTS` for text/image-capable model metadata baked into `openclaw.json`, from #2441 / commit `f4391892`. - `docs/reference/troubleshooting.md`: added the Git certificate verification entry for proxy CA propagation through `GIT_SSL_CAINFO`, `GIT_SSL_CAPATH`, `CURL_CA_BUNDLE`, and `REQUESTS_CA_BUNDLE`, from #2345 / commit `fa0dc1ab`. - `docs/versions1.json` and `docs/project.json`: promoted docs version `0.0.29`; `docs/versions1.json` omits unpublished `0.0.26`, `0.0.27`, and `0.0.28` entries. - `.agents/skills/nemoclaw-user-*`: regenerated derived user skill references from the updated docs. - Reviewed with no extra doc changes: #2575 / `d392ec07`, #2565 / `a3231049`, #1965 / `db1ef3ca`, #1990 / `db665834`, #2495 / `7da86fa3`, #2496 / `3192f4f4`, #2490 / `8c209058`, #2487 / `1f615e2f`, #2483 / `5653d33a`, #2482 / `31c782c0`, #2464 / `23bb5703`, #2472 / `a54f9a34`, and #2437 / `6bc860d7`. - Skipped per docs policy: #2420 / `7b76df6b` touched the experimental sandbox config path listed in `docs/.docs-skip`; #2466 / `cc15689c` touched a skipped term and CI-only sandbox image files. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] Doc only (includes code sample changes) ## Verification <!-- Check each item you ran and confirmed. Leave unchecked items you skipped. --> - [x] `npx prek run --all-files` passes - [ ] `npm test` passes — failed locally in installer-integration tests and one onboard helper timeout; the doc-scoped hook test projects passed under `prek`. - [ ] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) — build succeeded, but local Sphinx emitted the existing version-switcher file read message. - [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) ## AI Disclosure <!-- If an AI agent authored or co-authored this PR, check the box and name the tool. Remove this section for fully human-authored PRs. --> - [x] AI-assisted — tool: Codex --- <!-- DCO sign-off required by CI. Run: git config user.name && git config user.email --> Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Support for custom YAML presets in policy configuration via --from-file and --from-dir. * New build-time inference input option to declare accepted modalities (text or text,image). * **Improvements** * Credential validation now offers interactive recovery: re-enter key, retry, choose another provider, or exit. * Clarified provider-specific API key prefix handling (nvapi- only applies to NVIDIA keys). * **Documentation** * TLS certificate troubleshooting for inspected networks. * Clarified remote dashboard security/device-pairing behavior; command docs updated; docs version bumped. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
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
2 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
The installer used to auto-attach
--resumetonemoclaw onboardwhenever a non-complete session file existed on disk. A run that failed at step 3 or 4 (for example, Ollama proxy unreachable) writesstatus="failed"withresumable=true, which matched that predicate, so re-runningcurl | bashsilently replayed the same failed choice. The user had no way to pick a different provider short of deleting~/.nemoclaw/onboard-session.json.Fixed by tightening the installer's session classifier and adding an end-to-end
--freshopt-in across both the installer and thenemoclaw onboardCLI.Related Issue
Resolves #2430.
Changes
Installer (
scripts/install.sh)The install script now classifies the on-disk session into one of four states and branches on it instead of treating "non-complete" as one bucket:
resume—status === "in_progress"only. Auto-attaches--resume(the legitimate Ctrl-C / interrupted-mid-step case).failed—status === "failed"or a populatedfailurefield. In a TTY, prompts[R/f]; in non-interactive mode, refuses with a non-zero exit so scripted re-runs cannot loop. The refusal message points at--freshandnemoclaw onboard --resumeas the two explicit opt-ins.skip— complete /resumable: false/ missing. No--resumeattached.corrupt— file exists but cannot be parsed, or status does not match any valueonboard-session.tsactually writes. Warns and starts fresh rather than auto-resuming an unreadable file.A new
--freshflag (andNEMOCLAW_FRESH=1env var) short-circuits the classifier entirely and is forwarded to the downstream CLI invocation.CLI (
nemoclaw onboard --fresh)onboard-command.tsnow accepts--fresh, exposed through the parser, the help text, and theOnboardCommandOptionstype. When set,onboard.tscallsonboardSession.clearSession()beforecreateSession + saveSession, so the previous session file is unlinked outright rather than overwritten in place.--freshand--resumeare mutually exclusive, validated at both the parser and the runtime entry point.Docs
New troubleshooting entry under Onboarding (
Previous onboarding session failed) explaining the interactive / non-interactive /--fresh/nemoclaw onboard --resumepaths and the~/.nemoclaw/onboard-session.jsonlast-resort delete.Tests
test/install-preflight.test.ts: two new cases — non-interactive refusal of a failed session, and--freshshort-circuiting auto-resume + forwarding to the CLI.src/lib/onboard-command.test.ts:--freshparsing and the--resume/--freshmutual-exclusion error.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)AI Disclosure
Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
New Features
--freshoption to start onboarding without resuming previous sessions; onboarding now avoids silent automatic retries after failures.R/Enter) or discard (f) a recorded failed session; non-interactive runs refuse to retry and exit non-zero unless explicitly opted in.Documentation
--freshor--resume.Tests