feat(cli): add resource profiles to nemoclaw onboard and nemoclaw resources command#3348
Open
nvshridhar wants to merge 10 commits into
Open
feat(cli): add resource profiles to nemoclaw onboard and nemoclaw resources command#3348nvshridhar wants to merge 10 commits into
nvshridhar wants to merge 10 commits into
Conversation
…ources command Add `nemoclaw resources [--json]` global command for hardware inventory and integrate resource profile selection into the onboard flow. Hardware inventory: - CPU cores, model, k8s node allocatable - RAM total, swap, k8s node allocatable - NVIDIA GPU name, count, VRAM - Available resource profiles from blueprint.yaml (with resolved values) Onboard integration: - Profile picker after messaging channels, before sandbox creation - Profiles use percentage values (e.g. "25%") resolved against detected hardware at runtime — same profile works on any machine - appendResourceFlags passes resolved --cpu-limit/--memory-limit to openshell sandbox create - Graceful degradation: silently skips if OpenShell lacks resource flags - Non-interactive: NEMOCLAW_RESOURCE_PROFILE env var selects profile, NEMOCLAW_CPU_LIMIT/NEMOCLAW_RAM_LIMIT override individual fields (supports both "25%" and absolute "4" / "8Gi" formats) Blueprint changes: - Added resource_profiles section with 4 profiles using percentages - Schema validates pattern: percentage or Kubernetes quantity Signed-off-by: Shridhar Damale <sdamale@nvidia.com> Co-authored-by: Cursor <cursoragent@cursor.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:
📝 WalkthroughWalkthroughAdds a ChangesResource Profiles and Discovery
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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 unit tests (beta)
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: 5
🧹 Nitpick comments (2)
src/lib/resources-cmd.ts (1)
111-134: ⚡ Quick winConsider extracting duplicated profile loading logic.
Profile loading appears in both
getHardwareResources()(lines 111-134) andloadResourceProfiles()(lines 302-326). Extract the common blueprint parsing logic into a private helper to maintain consistency and reduce duplication.♻️ Proposed refactor
+function parseResourceProfilesFromBlueprint(blueprintPath: string): Record<string, ResourceProfile> { + if (!fs.existsSync(blueprintPath)) return {}; + const content = fs.readFileSync(blueprintPath, "utf-8"); + const blueprint = YAML.parse(content); + const raw = blueprint?.components?.sandbox?.resource_profiles; + if (!raw || typeof raw !== "object") return {}; + const profiles: Record<string, ResourceProfile> = {}; + for (const [name, p] of Object.entries(raw)) { + const prof = p as Record<string, unknown>; + if (prof.cpu_request && prof.cpu_limit && prof.memory_request && prof.memory_limit) { + profiles[name] = { + cpu_request: String(prof.cpu_request), + cpu_limit: String(prof.cpu_limit), + memory_request: String(prof.memory_request), + memory_limit: String(prof.memory_limit), + }; + } + } + return profiles; +}Then call it from both functions:
const blueprintPath = path.join(__dirname, "..", "..", "nemoclaw-blueprint", "blueprint.yaml"); profiles = parseResourceProfilesFromBlueprint(blueprintPath);🤖 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/resources-cmd.ts` around lines 111 - 134, The resource-profile parsing logic is duplicated in getHardwareResources and loadResourceProfiles; extract the shared YAML/blueprint reading and parsing into a single private helper (e.g., parseResourceProfilesFromBlueprint) that accepts the blueprintPath (or derives it) and returns Record<string, ResourceProfile> | null, then replace the duplicated blocks in getHardwareResources and loadResourceProfiles to call that helper and handle its null/return value; ensure the helper preserves the existing behavior (fs.existsSync, YAML.parse, safe casting and string defaults) and keep exception handling consistent.nemoclaw-blueprint/blueprint.yaml (1)
39-58: ⚡ Quick winConsider consolidating or differentiating the
creatorandgame-developerprofiles.Both profiles define identical resource allocations (25% request / 50% limit for CPU and memory). If these personas have the same resource needs, consider using a single profile; otherwise, adjust the values to reflect their distinct workload characteristics.
🤖 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 `@nemoclaw-blueprint/blueprint.yaml` around lines 39 - 58, The blueprint defines identical resource allocations for the "creator" and "game-developer" profiles so either consolidate them into a single profile or give them distinct values to reflect different workloads; update the blueprint.yaml by either removing one profile and referencing the remaining profile where used (merge "creator" and "game-developer" into one named profile) or change the cpu_request/cpu_limit/memory_request/memory_limit values for "creator" and/or "game-developer" to reflect their actual resource needs (e.g., lower for authoring vs. higher for build-heavy developer tasks), and ensure any references to the removed/renamed profile are updated accordingly.
🤖 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/commands/resources.ts`:
- Around line 37-43: The run() method currently captures the return of
printHardwareResources(this.jsonEnabled()) into result but
printHardwareResources returns void; update the code flow so that when
this.jsonEnabled() is true the command returns the actual HardwareResources
object: change or overload printHardwareResources (in resources-cmd.ts) to
return a HardwareResources value when called with jsonEnabled=true (or add a new
function that returns the object), then in ResourcesCommand.run() use that
returned object (result) to return JSON mode output; ensure
printHardwareResources still prints to stdout in non-JSON mode and that
ResourcesCommand.run() calls this.jsonEnabled() to decide which form to return.
In `@src/lib/onboard.ts`:
- Around line 10262-10280: The code only resolves/validates cpu_limit and
memory_limit; also resolve and validate cpu_request and memory_request using
resolveResourceValue so invalid request values are caught early: after computing
cpuTotal = hw.cpu.cores and memTotal = hw.memory.totalMB call
resolveResourceValue(cpuReq, cpuTotal, "cpu") and resolveResourceValue(ramReq,
memTotal, "memory"), then log the resolved request and limit values (or compare
requests <= limits if desired) before continuing; update the block that builds
selectedProfile (cpu_request, cpu_limit, memory_request, memory_limit) to
perform these extra resolves and handle errors the same way as for limits.
- Around line 10253-10257: The selection parsing for profiles (where idx =
parseInt(choice.trim(), 10) - 1 and selectedProfile is set from
availableProfiles/profileNames) currently lets non-numeric or out-of-range
inputs fall through to using default resources; change this to fail-fast or
re-prompt: validate that choice is a numeric index within
1..profileNames.length-1 (or equal to profileNames.length for the explicit
option) and when parseInt yields NaN or idx is outside the permitted range,
throw an error or loop back to prompt the user (do not silently continue),
ensuring you reference the same variables (choice, idx, profileNames,
availableProfiles, selectedProfile) and update the control flow that currently
reaches the default-resources branch.
In `@src/lib/resources-cmd.ts`:
- Around line 148-187: printHardwareResources currently writes JSON to stdout
but returns void, while ResourcesCommand.run() expects the hardware data when
the json flag is set; update printHardwareResources(json: boolean) so that when
json === true it returns the hw object (the variable holding
getHardwareResources()) after writing JSON to stdout, keeping the existing
behavior for non-json paths and the function signature unchanged so callers like
ResourcesCommand.run() can receive the data.
- Around line 265-296: appendResourceFlags claims to "gracefully degrade" but
resolveProfile -> resolveResourceValue can throw on invalid percentage input;
wrap the resolveProfile(profile, hw) call (and the subsequent uses of resolved)
in a try-catch so any thrown error is caught and the function returns false
(i.e., skip adding resource flags) instead of crashing; reference
resolveProfile, resolveResourceValue and appendResourceFlags when making this
change so the behavior is clear and safe.
---
Nitpick comments:
In `@nemoclaw-blueprint/blueprint.yaml`:
- Around line 39-58: The blueprint defines identical resource allocations for
the "creator" and "game-developer" profiles so either consolidate them into a
single profile or give them distinct values to reflect different workloads;
update the blueprint.yaml by either removing one profile and referencing the
remaining profile where used (merge "creator" and "game-developer" into one
named profile) or change the cpu_request/cpu_limit/memory_request/memory_limit
values for "creator" and/or "game-developer" to reflect their actual resource
needs (e.g., lower for authoring vs. higher for build-heavy developer tasks),
and ensure any references to the removed/renamed profile are updated
accordingly.
In `@src/lib/resources-cmd.ts`:
- Around line 111-134: The resource-profile parsing logic is duplicated in
getHardwareResources and loadResourceProfiles; extract the shared YAML/blueprint
reading and parsing into a single private helper (e.g.,
parseResourceProfilesFromBlueprint) that accepts the blueprintPath (or derives
it) and returns Record<string, ResourceProfile> | null, then replace the
duplicated blocks in getHardwareResources and loadResourceProfiles to call that
helper and handle its null/return value; ensure the helper preserves the
existing behavior (fs.existsSync, YAML.parse, safe casting and string defaults)
and keep exception handling consistent.
🪄 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: 0d5a1f3e-6867-40a9-9a41-2e09c8126647
📒 Files selected for processing (9)
nemoclaw-blueprint/blueprint.yamlschemas/blueprint.schema.jsonsrc/lib/cli/command-display.tssrc/lib/cli/command-registry.test.tssrc/lib/cli/command-registry.tssrc/lib/cli/oclif-dispatch.tssrc/lib/commands/resources.tssrc/lib/onboard.tssrc/lib/resources-cmd.ts
wscurran
added
NemoClaw CLI
Contributor
|
✨ Related open issues: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/lib/onboard.ts (1)
10853-10869: Run the full onboarding E2E matrix for this path change.This touches core sandbox creation flow in onboarding; please run the recommended E2E jobs (
cloud-e2e,sandbox-operations-e2e,rebuild-openclaw-e2e,messaging-compatible-endpoint-e2e,hermes-discord-e2e,hermes-slack-e2e,openshell-gateway-upgrade-e2e) before merge.As per coding guidelines
src/lib/onboard.ts: "This file contains core onboarding logic. Changes here affect the full sandbox creation and configuration flow." and the listed "E2E test recommendation" jobs.🤖 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 10853 - 10869, This change touches the core sandbox creation flow around selectResourceProfileForSandbox, stopStaleDashboardListenersForSandbox (which uses registry.listSandboxes), and createSandbox; before merging, run the full onboarding E2E matrix (cloud-e2e, sandbox-operations-e2e, rebuild-openclaw-e2e, messaging-compatible-endpoint-e2e, hermes-discord-e2e, hermes-slack-e2e, openshell-gateway-upgrade-e2e) to validate the new path and ensure no regressions in sandbox creation, dashboard listener cleanup, and profile selection logic.
🤖 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 5014-5122: The new resource-profile selection helpers
(hasResourceEnvOverrides, applyResourceEnvOverrides,
exitWithResourceProfileError, printResolvedResourceProfile,
selectResourceProfileForSandbox and any helper callers like
loadResourceProfiles/getHardwareResources/promptOrDefault) are too large for
src/lib/onboard.ts and must be extracted into a new focused module (e.g.,
src/lib/resources-cmd.ts); move those functions into the new file, export
selectResourceProfileForSandbox (and any helpers used elsewhere), update
onboard.ts to import the exported symbol(s) and keep onboard.ts only
orchestrating calls to selectResourceProfileForSandbox, ensuring all references
(note, prompt, promptOrDefault, loadResourceProfiles, getHardwareResources,
resolveResourceValue) are imported or passed in as needed so compilation and
behavior remain unchanged.
- Around line 5061-5068: The code trusts availableProfiles[envProfile] which can
hit inherited properties; change the existence check to an own-key check using
Object.prototype.hasOwnProperty.call(availableProfiles, envProfile) (or
equivalent) in the conditional that sets selectedProfile (the block using
NEMOCLAW_RESOURCE_PROFILE, envProfile, availableProfiles, profileNames and
selectedProfile), so only true defined profile keys allow selection and fallback
to the unknown-profile error branch otherwise.
---
Nitpick comments:
In `@src/lib/onboard.ts`:
- Around line 10853-10869: This change touches the core sandbox creation flow
around selectResourceProfileForSandbox, stopStaleDashboardListenersForSandbox
(which uses registry.listSandboxes), and createSandbox; before merging, run the
full onboarding E2E matrix (cloud-e2e, sandbox-operations-e2e,
rebuild-openclaw-e2e, messaging-compatible-endpoint-e2e, hermes-discord-e2e,
hermes-slack-e2e, openshell-gateway-upgrade-e2e) to validate the new path and
ensure no regressions in sandbox creation, dashboard listener cleanup, and
profile selection logic.
🪄 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: bf106993-c19a-41bc-b59e-e8799bd7825a
📒 Files selected for processing (5)
docs/reference/commands.mdnemoclaw-blueprint/blueprint.yamlsrc/lib/cli/command-registry.test.tssrc/lib/cli/oclif-dispatch.tssrc/lib/onboard.ts
✅ Files skipped from review due to trivial changes (3)
- src/lib/cli/oclif-dispatch.ts
- docs/reference/commands.md
- src/lib/cli/command-registry.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- nemoclaw-blueprint/blueprint.yaml
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/lib/onboard/resource-profile-selection.ts (2)
38-47: ⚡ Quick winMutation risk: spread before modifying to avoid side effects.
If
selectedProfileis passed as a direct reference (e.g., from line 112 which doesn't spread), this function mutates the original object. While the current control flow prevents this scenario, the code is fragile. Making a defensive copy here ensures safety regardless of how callers evolve.♻️ Proposed fix
- const nextProfile = selectedProfile || { + const nextProfile = selectedProfile ? { ...selectedProfile } : { cpu_request: "", cpu_limit: "", memory_request: "", memory_limit: "", };🤖 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/resource-profile-selection.ts` around lines 38 - 47, The code mutates selectedProfile via nextProfile; make a defensive shallow copy before modifying by creating nextProfile from a spread of selectedProfile (or the default object) so changes to nextProfile do not affect the original selectedProfile; then apply the env overrides (env.NEMOCLAW_CPU_REQUEST, env.NEMOCLAW_CPU_LIMIT, env.NEMOCLAW_RAM_REQUEST, env.NEMOCLAW_RAM_LIMIT) to that copied nextProfile.
112-112: ⚡ Quick winAdd spread for consistency with line 80.
Line 80 creates a copy with
{ ...availableProfiles[envProfile] }, but this line assigns the reference directly. For consistency and to prevent potential future mutation issues (as noted above inapplyResourceEnvOverrides), spread here as well.♻️ Proposed fix
- selectedProfile = availableProfiles[profileNames[idx]]; + selectedProfile = { ...availableProfiles[profileNames[idx]] };🤖 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/resource-profile-selection.ts` at line 112, selectedProfile is assigned a direct reference from availableProfiles[profileNames[idx]] which differs from the earlier copy at line 80 and can allow mutations; change the assignment in resource-profile-selection.ts to create a shallow copy using object spread (e.g., selectedProfile = { ...availableProfiles[profileNames[idx]] }) so it matches the pattern used for envProfile and avoids accidental mutation (consistent with applyResourceEnvOverrides behavior).
🤖 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 `@src/lib/onboard/resource-profile-selection.ts`:
- Around line 38-47: The code mutates selectedProfile via nextProfile; make a
defensive shallow copy before modifying by creating nextProfile from a spread of
selectedProfile (or the default object) so changes to nextProfile do not affect
the original selectedProfile; then apply the env overrides
(env.NEMOCLAW_CPU_REQUEST, env.NEMOCLAW_CPU_LIMIT, env.NEMOCLAW_RAM_REQUEST,
env.NEMOCLAW_RAM_LIMIT) to that copied nextProfile.
- Line 112: selectedProfile is assigned a direct reference from
availableProfiles[profileNames[idx]] which differs from the earlier copy at line
80 and can allow mutations; change the assignment in
resource-profile-selection.ts to create a shallow copy using object spread
(e.g., selectedProfile = { ...availableProfiles[profileNames[idx]] }) so it
matches the pattern used for envProfile and avoids accidental mutation
(consistent with applyResourceEnvOverrides behavior).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e4fa60a1-4415-4460-b8ae-c479535550d0
📒 Files selected for processing (5)
nemoclaw-blueprint/blueprint.yamlsrc/lib/onboard.tssrc/lib/onboard/build-estimate.tssrc/lib/onboard/resource-profile-selection.tssrc/lib/resources-cmd.ts
✅ Files skipped from review due to trivial changes (1)
- src/lib/onboard/build-estimate.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- nemoclaw-blueprint/blueprint.yaml
- src/lib/resources-cmd.ts
- src/lib/onboard.ts
cv
approved these changes
May 14, 2026
cv
requested changes
May 14, 2026
Collaborator
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
Waiting for an OpenShell that gives us these flags, as their design might change slightly.
Collaborator
|
Changes merged into OpenShell and should be in the next release tag: NVIDIA/OpenShell#1376 This PR is ready to go once we upgrade to it. |
Author
|
Rebased, Updated as per NVIDIA/OpenShell#1376, tested NemoClaw. |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com> # Conflicts: # src/lib/onboard.ts # test/onboard.test.ts
Signed-off-by: Aaron Erickson <aerickson@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
4 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
Add
nemoclaw resources [--json]global command for hardware inventory and integrate percentage-based resource profile selection into thenemoclaw onboardflow. Resource limits (CPU/memory requests and limits) are applied to sandbox pods via OpenShell's resource flags when available, with graceful degradation when they are not.Related Issue
Closes #3336
Changes
src/lib/resources-cmd.ts(new): Hardware inventory (getHardwareResources), percentage resolution (resolveResourceValue), profile loading fromblueprint.yaml, andappendResourceFlagswith OpenShell CLI capability detectionsrc/lib/commands/resources.ts(new): Oclif command fornemoclaw resources [--json]with display metadatasrc/lib/cli/oclif-dispatch.ts: AddedresourcestoGLOBAL_ROUTESsrc/lib/cli/command-display.ts: Added"Resources"toCommandGrouptypesrc/lib/cli/command-registry.ts: Added"Resources"toGROUP_ORDERsrc/lib/cli/command-registry.test.ts: Updated GROUP_ORDER snapshotsrc/lib/onboard.ts: Profile picker (interactive + env var paths),appendResourceFlagsincreateSandboxnemoclaw-blueprint/blueprint.yaml: Addedresource_profileswith 4 percentage-based profilesschemas/blueprint.schema.json: Addedresource_profilesschema with percentage/K8s quantity pattern validationType of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Testing
Verified end-to-end on cgroup v2 (Docker Desktop + WSL2):
resources.requests.cpu=6, resources.limits.cpu=8, resources.requests.memory=9Gi, resources.limits.memory=12Gicpu.max=800000/100000(8 cores),memory.max=12884901888(12Gi)Dependencies
0.0.39as both min and max (scripts/install-openshell.sh,nemoclaw-blueprint/blueprint.yaml), so resource profiles gracefully fall back and will not apply CPU/RAM limits through the shipped installer path.v0.0.41, which addsopenshell sandbox create --cpuand--memory.Signed-off-by: Shridhar Damale sdamale@nvidia.com
Summary by CodeRabbit
New Features
Configuration
Documentation