Skip to content

fix: swap hard/soft ulimit settings in start script#951

Merged
kjw3 merged 2 commits intoNVIDIA:mainfrom
epwilkins:949-ulimit-bug
Mar 26, 2026
Merged

fix: swap hard/soft ulimit settings in start script#951
kjw3 merged 2 commits intoNVIDIA:mainfrom
epwilkins:949-ulimit-bug

Conversation

@epwilkins
Copy link
Contributor

@epwilkins epwilkins commented Mar 26, 2026

Fixes #949

Summary

Lowers soft limit before hard limit to avoid error noted in #949 and 0acb2e4

Related Issue

Fixes #949

Changes

Set ulimit -Su before setting ulimit -Hu in scripts/nemoclaw-start.sh

Type of Change

  • Code change for a new feature, bug fix, or refactor.
  • Code change with doc updates.
  • Doc only. Prose changes without code sample modifications.
  • Doc only. Includes code sample changes.

Testing

  • npx prek run --all-files passes (or equivalently make check).
  • npm test passes.
  • make docs builds without warnings. (for doc-only changes)

As this was a change to the script it is not tested by any of the above. Instead I rebuilt nemoclaw locally with the modified script and ran onboarding. Confirmed that the error was resolved and the sandbox was started successfully.

Checklist

General

Code Changes

  • Formatters applied — npx prek run --all-files auto-fixes formatting (or make format for targeted runs).
  • Tests added or updated for new or changed behavior.
  • No secrets, API keys, or credentials committed.
  • Doc pages updated for any user-facing behavior changes (new commands, changed defaults, new features, bug fixes that contradict existing docs).

Doc Changes

  • Follows the style guide. Try running the update-docs agent skill to draft changes while complying with the style guide. For example, prompt your agent with "/update-docs catch up the docs for the new changes I made in this PR."
  • New pages include SPDX license header and frontmatter, if creating a new page.
  • Cross-references and links verified.

Summary by CodeRabbit

  • Chores
    • Adjusted internal process limit configuration initialization order in startup script.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a5acd13c-cdf6-45d2-aed0-e019e98b9d0f

📥 Commits

Reviewing files that changed from the base of the PR and between 0acb2e4 and 3213c4b.

📒 Files selected for processing (1)
  • scripts/nemoclaw-start.sh

📝 Walkthrough

Walkthrough

Reordered the ulimit command execution in the nemoclaw startup script to set the soft nproc limit before the hard nproc limit, resolving an "Invalid argument" error that occurred when attempting to lower the hard limit below the existing soft limit.

Changes

Cohort / File(s) Summary
NemoClaw Startup Configuration
scripts/nemoclaw-start.sh
Reordered ulimit -Su 512 to execute before ulimit -Hu 512, allowing the hard process limit to be set after the soft limit is established.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐰 A rabbit learned a lesson well,
When limits must be set, oh tell:
Soft before the hard must go,
Like burrows built from top below!
Now sandboxes bloom without a care, 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: reordering ulimit hard and soft limit settings in the start script to fix the bug.
Linked Issues check ✅ Passed The PR successfully addresses issue #949 by swapping the order of ulimit calls so soft limit is set before hard limit, preventing the invalid-argument error.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing issue #949; no out-of-scope modifications are present in the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@epwilkins
Copy link
Contributor Author

FWIW this is pretty easy to test on the fly:

  1. docker run --rm -it ghcr.io/nvidia/nemoclaw/sandbox-base:latest bash
  2. ulimit -Hu 512 (errors out)
  3. ulimit -Su 512
  4. ulimit -Hu 512 (succeeds)

@kjw3 kjw3 self-assigned this Mar 26, 2026
@kjw3 kjw3 merged commit f0f53e4 into NVIDIA:main Mar 26, 2026
8 checks passed
TSavo pushed a commit to wopr-network/nemoclaw that referenced this pull request Mar 26, 2026
* fix: improve gateway lifecycle recovery (NVIDIA#953)

* fix: improve gateway lifecycle recovery

* docs: fix readme markdown list spacing

* fix: tighten gateway lifecycle review follow-ups

* fix: simplify tokenized control ui output

* fix: restore chat route in control ui urls

* refactor: simplify ansi stripping in onboard

* fix: shorten control ui url output

* fix: move control ui below cli next steps

* fix: swap hard/soft ulimit settings in start script (NVIDIA#951)

Fixes NVIDIA#949

Co-authored-by: KJ <kejones@nvidia.com>

---------

Co-authored-by: KJ <kejones@nvidia.com>
Co-authored-by: Emily Wilkins <80470879+epwilkins@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sandbox creation fails during onboarding

2 participants