feat: add unattended/CI install support#225
Closed
rwipfelnv wants to merge 3 commits intoNVIDIA:mainfrom
Closed
Conversation
3 tasks
Member
jacobtomlinson
left a comment
jacobtomlinson
left a comment
There was a problem hiding this comment.
Looks great. One small comment.
| $ export NVIDIA_API_KEY=nvapi-xxx | ||
| $ export NEMOCLAW_NONINTERACTIVE=1 | ||
| $ export NEMOCLAW_SANDBOX_NAME=my-sandbox # optional, defaults to "my-assistant" | ||
| $ curl -fsSL https://nvidia.com/nemoclaw.sh | bash |
Member
There was a problem hiding this comment.
For CI we need to checkout the right branch and then run install.sh. We should probably mention that here. Dont want curl from main in CI.
rwipfelnv
force-pushed
the
rwipfelnv/unattended-install
branch
from
2094096 to
9c9b29f
Compare
Contributor
📝 WalkthroughWalkthroughAdds unattended/CI non-interactive install support: README documents env var options; Changes
Sequence DiagramsequenceDiagram
actor CI as CI/Deployment
participant Cred as Credentials
participant Onboard as Onboarding
participant Sandbox as SandboxService
participant Inference as InferenceProvider
CI->>Cred: run installer with env vars
Cred->>Cred: detect non-interactive (NEMOCLAW_NONINTERACTIVE or CI)
alt Non-interactive
Cred->>CI: supply default answers (no prompts)
else Interactive
Cred->>CI: prompt for inputs
end
Cred->>Onboard: start onboarding with resolved inputs
Onboard->>Onboard: check NEMOCLAW_SANDBOX_NAME and RECREATE flag
alt Dynamo endpoint provided
Onboard->>Inference: configure external vLLM (NEMOCLAW_DYNAMO_ENDPOINT / MODEL)
Onboard-->>CI: return early (external provider)
else Use local NIM
Onboard->>Sandbox: create or reuse sandbox (may recreate based on flags)
end
alt Non-interactive
Onboard->>Onboard: auto-apply policy presets and defaults
else Interactive
Onboard->>CI: prompt for preset selection
end
Onboard->>CI: log final configuration
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
bin/lib/onboard.js (1)
231-241: Consider validating the Dynamo endpoint URL format.The Dynamo endpoint is used directly without validation. A malformed URL could cause confusing failures later in
setupInference.🛡️ Proposed validation
// Check for Dynamo/external vLLM endpoint (for K8s/cloud deployments) const dynamoEndpoint = process.env.NEMOCLAW_DYNAMO_ENDPOINT; const dynamoModel = process.env.NEMOCLAW_DYNAMO_MODEL; if (dynamoEndpoint) { + try { + new URL(dynamoEndpoint); + } catch { + console.error(` Invalid NEMOCLAW_DYNAMO_ENDPOINT URL: ${dynamoEndpoint}`); + process.exit(1); + } console.log(` Using Dynamo endpoint: ${dynamoEndpoint}`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin/lib/onboard.js` around lines 231 - 241, Validate the Dynamo endpoint string before using it: check process.env.NEMOCLAW_DYNAMO_ENDPOINT (dynamoEndpoint) is a well-formed URL (scheme and host present) and reject or log an error if invalid; only set provider/model, call registry.updateSandbox(sandboxName, {...}) and return when the URL passes validation. Use the same variable names (dynamoEndpoint, dynamoModel, provider, model) and ensure any validation failure provides a clear message and prevents calling setupInference or registry.updateSandbox with a malformed endpoint.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@bin/lib/onboard.js`:
- Around line 231-241: Validate the Dynamo endpoint string before using it:
check process.env.NEMOCLAW_DYNAMO_ENDPOINT (dynamoEndpoint) is a well-formed URL
(scheme and host present) and reject or log an error if invalid; only set
provider/model, call registry.updateSandbox(sandboxName, {...}) and return when
the URL passes validation. Use the same variable names (dynamoEndpoint,
dynamoModel, provider, model) and ensure any validation failure provides a clear
message and prevents calling setupInference or registry.updateSandbox with a
malformed endpoint.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 904f4ccf-1967-4cde-aad6-b29d1786b969
📒 Files selected for processing (3)
README.mdbin/lib/credentials.jsbin/lib/onboard.js
Add support for non-interactive installation mode for automated deployments, CI pipelines, and containerized environments. New environment variables: - NEMOCLAW_NONINTERACTIVE: Skip all interactive prompts - NEMOCLAW_SANDBOX_NAME: Custom sandbox name (default: my-assistant) - NEMOCLAW_RECREATE_SANDBOX: Control sandbox recreation behavior - NEMOCLAW_DYNAMO_ENDPOINT: External vLLM/Dynamo endpoint URL - NEMOCLAW_DYNAMO_MODEL: Model name for Dynamo endpoint - CI=true: Auto-enables non-interactive mode Changes: - credentials.js: Add isNonInteractive() helper, update prompt() to accept defaults - onboard.js: Add Dynamo provider support, auto-apply policies in CI mode - README.md: Document unattended install workflow and env vars Signed-off-by: rwipfelnv
rwipfelnv
force-pushed
the
rwipfelnv/unattended-install
branch
from
9c9b29f to
06e020b
Compare
Contributor
Author
|
(Sorry, I amended the OG commit and should've pushed new ones to address the review feedback). |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
bin/lib/onboard.js (1)
483-488: Filter suggested presets before auto-apply to avoid brittle failures.In non-interactive mode, applying raw
suggestionscan fail if a preset is missing/renamed or already applied. UseallPresets/appliedto build a safe list first.♻️ Suggested patch
if (isNonInteractive()) { // In non-interactive mode, apply suggested presets automatically - console.log(` Applying suggested presets: ${suggestions.join(", ")}`); - for (const name of suggestions) { + const applicable = suggestions.filter( + (name) => allPresets.some((p) => p.name === name) && !applied.includes(name) + ); + console.log(` Applying suggested presets: ${applicable.join(", ") || "(none)"}`); + for (const name of applicable) { policies.applyPreset(sandboxName, name); } } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin/lib/onboard.js` around lines 483 - 488, When isNonInteractive() is true the code currently calls policies.applyPreset(sandboxName, name) for every item in suggestions which can fail if a preset doesn't exist or is already applied; update the logic to first build a safe list by intersecting suggestions with the available allPresets and excluding any already in applied, then iterate that filtered list and call policies.applyPreset(sandboxName, name) only for those valid, not-yet-applied presets to avoid brittle failures.README.md (1)
111-114: Consider removing shell prompts to satisfy markdownlint MD014.If markdownlint is enforced, the leading
$in console blocks will keep warning; dropping prompts avoids noisy doc checks.Also applies to: 119-120
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 111 - 114, The README code block uses leading "$" shell prompts which trigger markdownlint MD014; remove the leading "$" from each example line in that block (and the similar block at lines 119-120) so the snippet contains just the commands (e.g., export NVIDIA_API_KEY=..., export NEMOCLAW_NONINTERACTIVE=1, export NEMOCLAW_SANDBOX_NAME=..., curl -fsSL https://nvidia.com/nemoclaw.sh | bash) to satisfy markdownlint; ensure no other prompt characters remain in the same fenced code blocks.bin/lib/credentials.js (1)
14-18: Make non-interactive env parsing more tolerant.
CI/NEMOCLAW_NONINTERACTIVEcurrently only match exact"true"/"1"values. Supporting case-insensitive booleans avoids accidental interactive prompts in automation.♻️ Suggested patch
+function isTruthyEnv(value) { + return /^(1|true)$/i.test(String(value || "").trim()); +} + // Non-interactive mode: skip prompts and use defaults // Enabled via NEMOCLAW_NONINTERACTIVE=1 or CI=true function isNonInteractive() { - return process.env.NEMOCLAW_NONINTERACTIVE === "1" || - process.env.NEMOCLAW_NONINTERACTIVE === "true" || - process.env.CI === "true"; + return isTruthyEnv(process.env.NEMOCLAW_NONINTERACTIVE) || + isTruthyEnv(process.env.CI); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin/lib/credentials.js` around lines 14 - 18, The isNonInteractive function only checks exact "1" or "true" which is brittle; update it to normalize and tolerate common truthy values by reading process.env.NEMOCLAW_NONINTERACTIVE and process.env.CI into a safe string (e.g. (val||"").trim().toLowerCase()) and then compare against a set of truthy tokens such as "1", "true", "yes" (or use a regex like /^(1|true|yes)$/) so isNonInteractive returns true for case-insensitive and whitespace-padded boolean-like values; change references in the isNonInteractive function accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bin/lib/onboard.js`:
- Around line 234-247: The code interpolates dynamoEndpoint and dynamoModel into
shell commands causing shell-injection risk; instead of building a bash -c
string, pass the executable and arguments directly to child_process.spawnSync
(avoid running via shell) and validate/whitelist model names; specifically, stop
interpolating dynamoEndpoint/dynamoModel into command strings and change any
spawnSync("bash", ["-c", cmd], ...) calls to spawnSync(executable,
[dynamoEndpoint, dynamoModel, ...], { shell: false }) or use execFile, and add a
strict validation rule (e.g. allowlist or regex like /^[A-Za-z0-9._\-\/:]+$/)
for dynamoModel and reuse the existing new URL(dynamoEndpoint) validation to
reject unsafe values before calling registry.updateSandbox or invoking external
processes.
---
Nitpick comments:
In `@bin/lib/credentials.js`:
- Around line 14-18: The isNonInteractive function only checks exact "1" or
"true" which is brittle; update it to normalize and tolerate common truthy
values by reading process.env.NEMOCLAW_NONINTERACTIVE and process.env.CI into a
safe string (e.g. (val||"").trim().toLowerCase()) and then compare against a set
of truthy tokens such as "1", "true", "yes" (or use a regex like
/^(1|true|yes)$/) so isNonInteractive returns true for case-insensitive and
whitespace-padded boolean-like values; change references in the isNonInteractive
function accordingly.
In `@bin/lib/onboard.js`:
- Around line 483-488: When isNonInteractive() is true the code currently calls
policies.applyPreset(sandboxName, name) for every item in suggestions which can
fail if a preset doesn't exist or is already applied; update the logic to first
build a safe list by intersecting suggestions with the available allPresets and
excluding any already in applied, then iterate that filtered list and call
policies.applyPreset(sandboxName, name) only for those valid, not-yet-applied
presets to avoid brittle failures.
In `@README.md`:
- Around line 111-114: The README code block uses leading "$" shell prompts
which trigger markdownlint MD014; remove the leading "$" from each example line
in that block (and the similar block at lines 119-120) so the snippet contains
just the commands (e.g., export NVIDIA_API_KEY=..., export
NEMOCLAW_NONINTERACTIVE=1, export NEMOCLAW_SANDBOX_NAME=..., curl -fsSL
https://nvidia.com/nemoclaw.sh | bash) to satisfy markdownlint; ensure no other
prompt characters remain in the same fenced code blocks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: caa935c6-ad94-4568-ba1d-1a8feb1e6064
📒 Files selected for processing (3)
README.mdbin/lib/credentials.jsbin/lib/onboard.js
- Add shQuote() helper for shell-safe single-quote escaping - Validate URL protocol is http/https before use - Use sanitized URL from parser instead of raw input - Shell-escape dynamoEndpoint and model in openshell commands Addresses CodeRabbit security review comment. Signed-off-by: rwipfelnv
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
bin/lib/onboard.js (1)
498-503: Skip already-applied presets in non-interactive mode.This avoids redundant apply calls and keeps logs cleaner on reruns.
♻️ Suggested refactor
if (isNonInteractive()) { // In non-interactive mode, apply suggested presets automatically console.log(` Applying suggested presets: ${suggestions.join(", ")}`); for (const name of suggestions) { - policies.applyPreset(sandboxName, name); + if (!applied.includes(name)) { + policies.applyPreset(sandboxName, name); + } } } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin/lib/onboard.js` around lines 498 - 503, When running in non-interactive mode the loop always calls policies.applyPreset(sandboxName, name) for every item in suggestions, causing redundant applies and noisy logs; modify the loop to skip presets already applied by checking current state (e.g., via policies.hasPreset/ policies.getAppliedPresets or an equivalent method) before calling policies.applyPreset(sandboxName, name), and only log and apply when the preset is not present—use isNonInteractive, suggestions, sandboxName and policies.applyPreset references to locate and update the logic accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bin/lib/onboard.js`:
- Around line 236-255: The code currently logs the full Dynamo endpoint
(dynamoEndpoint/dynamoEndpointRaw), which may expose credentials or query
params; update the logging in the block that parses NEMOCLAW_DYNAMO_ENDPOINT to
never print the full URL: use the URL object (parsedUrl) to construct a
sanitized string that omits username, password, and search/query (e.g., log
protocol + host + pathname or just protocol + host), and if parsedUrl.username
or parsedUrl.password or parsedUrl.search are present log a redacted indicator
(e.g., "[REDACTED_CREDENTIALS]" or "[REDACTED_QUERY]") instead of their values;
keep the console.log for Model (dynamoModel) unchanged.
- Around line 439-443: When provider === "dynamo", validate the resolved
dynamoEndpoint (the value from registry.getSandbox(sandboxName)?.dynamoEndpoint
or process.env.NEMOCLAW_DYNAMO_ENDPOINT) before building configArg and invoking
run; if dynamoEndpoint is missing or falsy, fail fast by logging a clear error
via processLogger.error (or throw) and exit with non-zero status instead of
proceeding to create OPENAI_BASE_URL=undefined — update the check around the
dynamoEndpoint variable and the code that constructs configArg/modelArg and
calls run to enforce this validation.
---
Nitpick comments:
In `@bin/lib/onboard.js`:
- Around line 498-503: When running in non-interactive mode the loop always
calls policies.applyPreset(sandboxName, name) for every item in suggestions,
causing redundant applies and noisy logs; modify the loop to skip presets
already applied by checking current state (e.g., via policies.hasPreset/
policies.getAppliedPresets or an equivalent method) before calling
policies.applyPreset(sandboxName, name), and only log and apply when the preset
is not present—use isNonInteractive, suggestions, sandboxName and
policies.applyPreset references to locate and update the logic accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e3c4fd2b-5f28-4337-8135-1d1c8aceae47
📒 Files selected for processing (1)
bin/lib/onboard.js
Contributor
Author
Testing SummaryUnit TestsAll 43 unit tests pass: Tests cover: CLI dispatch, registry operations, NIM model listing, GPU detection, policy presets. Security Fix VerificationAddressed CodeRabbit's shell injection concern by adding:
// Shell-safe single-quote escaping to prevent injection
function shQuote(value) {
return `'${String(value).replace(/'/g, `'\"'\"'`)}'`;
}Kubernetes Integration TestTested on EKS with Dynamo vLLM endpoint:
Key environment variables tested:
Previous Successful Test (before security fix commit)Full 7-step onboard completed successfully on 2026-03-17: Final dashboard showed: |
- Sanitize URL logging to omit credentials and query params - Validate dynamoEndpoint before use, fail fast if missing - Skip already-applied presets in non-interactive mode Signed-off-by: rwipfelnv
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
bin/lib/onboard.js (1)
245-250:⚠️ Potential issue | 🟠 MajorAvoid logging raw endpoint values in validation error paths.
When URL parsing/protocol validation fails, the error message still includes the raw
NEMOCLAW_DYNAMO_ENDPOINT, which can leak credentials or tokenized query data to CI logs.🔐 Suggested fix
- console.error(` Invalid NEMOCLAW_DYNAMO_ENDPOINT URL: ${dynamoEndpointRaw}`); + console.error(" Invalid NEMOCLAW_DYNAMO_ENDPOINT URL."); process.exit(1); @@ - console.error(` NEMOCLAW_DYNAMO_ENDPOINT must use http or https: ${dynamoEndpointRaw}`); + console.error(" NEMOCLAW_DYNAMO_ENDPOINT must use http or https."); process.exit(1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin/lib/onboard.js` around lines 245 - 250, The validation error paths that currently log the raw NEMOCLAW_DYNAMO_ENDPOINT (variable dynamoEndpointRaw) should not print the full value; update the two console.error calls in the validation block that reference parsedUrl and parsedUrl.protocol to either omit the endpoint or log a redacted/limited form (for example parsedUrl.host or a fixed string like "<redacted NEMOCLAW_DYNAMO_ENDPOINT>") instead of dynamoEndpointRaw, while keeping the same exit behavior (process.exit(1)); locate the checks using dynamoEndpointRaw, parsedUrl, and the protocol comparison and replace the interpolated raw value with the redacted/host-only value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@bin/lib/onboard.js`:
- Around line 245-250: The validation error paths that currently log the raw
NEMOCLAW_DYNAMO_ENDPOINT (variable dynamoEndpointRaw) should not print the full
value; update the two console.error calls in the validation block that reference
parsedUrl and parsedUrl.protocol to either omit the endpoint or log a
redacted/limited form (for example parsedUrl.host or a fixed string like
"<redacted NEMOCLAW_DYNAMO_ENDPOINT>") instead of dynamoEndpointRaw, while
keeping the same exit behavior (process.exit(1)); locate the checks using
dynamoEndpointRaw, parsedUrl, and the protocol comparison and replace the
interpolated raw value with the redacted/host-only value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b75bd544-7baa-4706-9337-7ac462cd9607
📒 Files selected for processing (1)
bin/lib/onboard.js
Contributor
Author
|
Clarification on K8s testing: The OOM issue mentioned above occurred during a re-test with a pod that had memory limits configured. The full 7-step onboard completed successfully on 2026-03-17 when the pod had no memory limits (only requests). The fix is a test infrastructure configuration issue (remove memory limits on DinD pods), not a code issue. OpenShell builds Docker images inside k3s which requires significant memory headroom. Successful test configuration: resources:
requests:
memory: "8Gi" # dind
memory: "4Gi" # workspace
# No limits - sandbox image builds need headroomAll CI checks pass, including the upstream |
Contributor
Author
|
Re: splitting the PR The Dynamo/Kubernetes code is entirely optional and additive - it only activates when Core P0 unattended install (always works): export NEMOCLAW_NONINTERACTIVE=1
export NEMOCLAW_SANDBOX_NAME=my-sandbox
curl -fsSL https://nvidia.com/nemoclaw.sh | bash→ Uses NVIDIA cloud inference (existing behavior) Optional P1 Dynamo support (only if env var set): export NEMOCLAW_DYNAMO_ENDPOINT=http://vllm.svc:8000/v1 # ← triggers Dynamo path
export NEMOCLAW_DYNAMO_MODEL=meta-llama/Llama-3.1-8B-Instruct→ Uses external vLLM endpoint instead of cloud The Dynamo code doesn't block or change the P0 flow - it's a separate conditional branch that users opt into. Happy to split if preferred, but wanted to clarify it's not blocking. |
Contributor
Author
This was referenced Mar 19, 2026
mafueee
pushed a commit
to mafueee/NemoClaw
that referenced
this pull request
Mar 28, 2026
…lete --all, and improve spinner spacing (NVIDIA#225)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
New Environment Variables
NEMOCLAW_NONINTERACTIVE1to skip all interactive promptsNEMOCLAW_SANDBOX_NAMEmy-assistant)NEMOCLAW_RECREATE_SANDBOX0to keep existing sandboxNEMOCLAW_DYNAMO_ENDPOINTNEMOCLAW_DYNAMO_MODELCI=trueTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Bug Fixes