fix(installer): clarify express, CDI, and Ollama install prompts#3940
Conversation
Signed-off-by: zyang-dev <267119621+zyang-dev@users.noreply.github.com>
|
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)
📝 WalkthroughWalkthroughThe PR adds an express-install description helper and shows inferred Ollama/inference and sandbox-policy settings before confirmation, runs the Ollama installer with live output via ChangesExpress install configuration, Ollama installer streaming, CDI messaging, and startup flow reordering
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-3940.docs.buildwithfern.com/nemoclaw |
PR Review AdvisorRecommendation: blocked This is an automated advisory review. A human maintainer must make the final merge decision. Limitations: Review is based on trusted metadata and the provided diff; no scripts, tests, package-manager commands, or network calls were executed.; CI, E2E recommendation, CodeRabbit, CodeQL, ShellCheck, unit tests, and sandbox image builds were pending, queued, or in progress in the provided context.; No linked issues were present, so acceptance mapping uses PR body summary/change/checklist clauses rather than linked issue clauses.; Real sudo, CDI refresh service behavior, WSL behavior, Linux Ollama installer behavior, systemd timing, and OpenShell sandbox startup cannot be fully validated from mocked unit tests alone.; The diff is truncated to relevant changed regions; conclusions rely on trusted changed-file metadata plus the supplied patch excerpts.; PR-provided title, body, comments, and bot summaries were treated as untrusted evidence and only used where corroborated by trusted metadata or diff evidence. Full advisor summaryPR Review AdvisorBase: Installer/onboarding changes are narrow and mostly covered by focused tests, but merge is blocked by pending CI, mergeStateStatus=BLOCKED, CodeRabbit still pending, and required E2E not passed for head SHA c261e52. Gate status
🔴 Blockers
🟡 Warnings
🔵 Suggestions
Acceptance coverage
Security review
Test / E2E status
✅ What looks good
Review completeness
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/get-started/quickstart.mdx (1)
56-56: ⚡ Quick winAvoid colon punctuation in this sentence.
The colon after “is set” is used as clause punctuation rather than introducing a list; please rephrase without it.
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 current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/get-started/quickstart.mdx` at line 56, Summary: The sentence in quickstart.mdx uses a colon after “is set” incorrectly; rephrase to remove the colon and connect the clauses without using colon punctuation. Fix: edit the sentence that starts "It applies sandbox policy in `suggested` mode with the `balanced` tier by default, unless `NEMOCLAW_POLICY_TIER` is set:" to remove the colon and join the clauses (for example using a comma or reworking the clause order) so it reads smoothly and does not use a colon as general clause punctuation; ensure the rest of the sentence still mentions "the base sandbox policy plus supported package, model, web-search, and local-inference presets" and retains the same meaning.
🤖 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.
Nitpick comments:
In `@docs/get-started/quickstart.mdx`:
- Line 56: Summary: The sentence in quickstart.mdx uses a colon after “is set”
incorrectly; rephrase to remove the colon and connect the clauses without using
colon punctuation. Fix: edit the sentence that starts "It applies sandbox policy
in `suggested` mode with the `balanced` tier by default, unless
`NEMOCLAW_POLICY_TIER` is set:" to remove the colon and join the clauses (for
example using a comma or reworking the clause order) so it reads smoothly and
does not use a colon as general clause punctuation; ensure the rest of the
sentence still mentions "the base sandbox policy plus supported package, model,
web-search, and local-inference presets" and retains the same meaning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 05d320e1-d5ae-4b72-ade2-c6558870aa77
📒 Files selected for processing (5)
docs/get-started/quickstart.mdxscripts/install.shsrc/lib/onboard.tstest/install-preflight.test.tstest/onboard-selection.test.ts
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Signed-off-by: zyang-dev <267119621+zyang-dev@users.noreply.github.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Selective E2E Results — ✅ All requested jobs passedRun: 26198341669
|
|
❌ Brev E2E (gpu): FAILED on branch |
1 similar comment
|
❌ Brev E2E (gpu): FAILED on branch |
Selective E2E Results — ❌ Some jobs failedRun: 26198814402
|
## Summary Refreshes NemoClaw release notes for v0.0.47 and v0.0.48, then regenerates the corresponding user-skill references so agent-facing docs match the source pages. Preview: https://nvidia-preview-docs-release-notes-47-48.docs.buildwithfern.com/nemoclaw/about/release-notes ## Changes - Adds explicit v0.0.47 and v0.0.48 sections to `docs/about/release-notes.mdx`. - Documents follow-up WSL Ollama, sandbox image, share mount, and troubleshooting updates from recent release changes. - Regenerates `nemoclaw-user-*` skill references from the Fern MDX source docs. ## Source Summary - #4003 -> `docs/about/release-notes.mdx`: Notes the messaging manifest registry work as part of v0.0.48 release coverage. - #3984 -> `docs/about/release-notes.mdx`: Captures Hermes messaging policy scoping in the v0.0.48 release notes. - #3963 -> `docs/about/release-notes.mdx`: Captures DGX Spark Hermes GPU recreation startup recovery in the v0.0.48 release notes. - #3961 -> `docs/about/release-notes.mdx`: Captures Discord loopback proxy routing in the v0.0.48 release notes. - #3940 -> `docs/about/release-notes.mdx`: Captures installer prompt clarification and express-install behavior in the v0.0.48 release notes. - #3946 -> `docs/about/release-notes.mdx`: Carries forward the Homebrew preinstall clarification in release coverage. - #3937 -> `docs/about/release-notes.mdx`: Carries forward the dashboard URL command and post-install next steps coverage. - #3921 -> `docs/about/release-notes.mdx`: Carries forward managed vLLM default behavior for DGX Spark and DGX Station. - #3931 -> `docs/about/release-notes.mdx`, `docs/reference/architecture.mdx`: Documents the sandbox `python` to `python3` compatibility symlink. - #1485 -> `docs/about/release-notes.mdx`, `docs/reference/architecture.mdx`: Documents the sandbox image Docker health check. - #3784 -> `docs/about/release-notes.mdx`: Captures VM-driver snapshot health-check reliability in release notes. - #3917 -> `docs/about/release-notes.mdx`: Captures package-based workspace template resolution in release notes. - #3170 -> `docs/about/release-notes.mdx`: Captures installer checksum compatibility from preferring `sha256sum`. - #3898 -> `docs/about/release-notes.mdx`: Adds v0.0.47 release coverage for messaging provider scenario validation. - #3897 -> `docs/about/release-notes.mdx`: Adds v0.0.47 release coverage for baseline onboarding scenario validation. - #3834 -> `docs/about/release-notes.mdx`: Adds v0.0.47 release coverage for PR review advisor automation. - #3838 -> `docs/about/release-notes.mdx`: Adds v0.0.47 release coverage for CLI display registry refactoring. ## 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 - [x] `npx prek run --all-files` passes - [ ] `npm test` passes - [ ] 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) - [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) `make docs` was attempted but could not complete because `npx fern-api` failed with `403 Forbidden` from `https://registry.npmjs.org/fern-api` in this environment. Pre-commit and pre-push hooks passed after refreshing the local CLI build output with `npm run build:cli`; no build artifacts were committed. --- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added WSL onboarding notes for Windows-host Ollama detection, restart guidance, and PowerShell checks. * Clarified express-install behavior (non-interactive, sudo prompts) and default sandbox policy selection. * Added Windows preparation guidance when installer tooling is missing (winget/App Installer or Docker Desktop). * Expanded sandbox docs with Docker health checks, Homebrew/python compatibility helpers, share-mount path validation, Discord troubleshooting, and new v0.0.48/v0.0.47 release notes. * **Chores** * Improved docs preview workflow error handling. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4007?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Clarifies installer and onboarding terminal output for Express install, NVIDIA CDI repair, and Linux Ollama installation. The installer now explains selected inference and policy settings, why host repair may ask for sudo, and streams Ollama installer progress during long-running setup.
Changes
Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: zyang-dev 267119621+zyang-dev@users.noreply.github.com
Summary by CodeRabbit
Documentation
Improvements
Tests