Skip to content

docs: add a troubleshooting entry for the contributor setup heap error - #8689

Merged
prekshivyas merged 11 commits into
NVIDIA:mainfrom
VishnuR23:docs/document-node-heap-requirement
Aug 12, 2026
Merged

docs: add a troubleshooting entry for the contributor setup heap error#8689
prekshivyas merged 11 commits into
NVIDIA:mainfrom
VishnuR23:docs/document-node-heap-requirement

Conversation

@VishnuR23

@VishnuR23 VishnuR23 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

./scripts/dev-setup.sh stops at the CLI type-check step on a host with 8 GB of RAM because tsc -p tsconfig.cli.json exceeds the default Node.js old-space limit of about 2.2 GB. The failure surfaces as a V8 abort and a native stack trace, and no page named the cause or the remedy. This PR adds a troubleshooting entry that names the limit, the failing step, and the NODE_OPTIONS remedy.

Related Issue

Reported in #8688.

This PR implements option 1 of the three directions listed in that issue. It does not close the issue because the maintainers may prefer option 2, which sets the limit in the type-check scripts, or option 3, which detects the failure in dev-setup.sh and the doctor.

Changes

  • Add Contributor Setup Fails with a JavaScript Heap Out-of-Memory Error to the Installation section of docs/reference/troubleshooting.mdx.
  • State that the entry applies to a source checkout rather than an installed release.
  • Give the NODE_OPTIONS value and the command to run setup again.

This PR adds no abstraction, configuration, fallback, migration, or compatibility path. It changes no source file and no workflow.

Review History

The first revision put this guidance in the CONTRIBUTING.md prerequisites. @cv asked for it in docs/reference/troubleshooting.mdx instead, so CONTRIBUTING.md is byte-identical to main and the entry lives on the troubleshooting page next to the other Installation entries. A later commit changed the new heading to title case to follow the heading rule in docs/CONTRIBUTING.md.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification: The change adds prose and one shell sample to docs/reference/troubleshooting.mdx. It changes no source file, script, workflow, command, or default.
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification:
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification:
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: docs-updated
  • Evidence: A documentation writer reviewed docs/reference/troubleshooting.mdx at the latest PR commit against WRITING.md, docs/CONTRIBUTING.md, current implementation, and all three generated guide variants. The reviewer confirmed the entry's placement, title case, source-checkout scope, numeric claims, and NODE_OPTIONS guidance. npm run docs passed with 0 errors and the same 2 warnings as main; the complete hook suite and git diff --check also passed.
  • Agent: Codex Desktop

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: Tests are marked not applicable above. The latest PR commit passed markdownlint-cli2, gitleaks, commitlint, the complete hook suite, and the pre-push TypeScript checks.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Notes on the unchecked verification items:

  • Broad gate: CONTRIBUTING.md states that a documentation-only change does not need the full test suite, so npm test and npm run check do not apply.
  • npm run docs: The current build reports 0 errors and 2 warnings. Both warnings are present on main and this PR changes one page under docs/reference/.
  • New-page item: This PR edits an existing page and adds no new page, so the SPDX and frontmatter item does not apply.

Signed-off-by: Vishnu Rajeev 19866703+VishnuR23@users.noreply.github.com

Summary by CodeRabbit

  • Documentation
    • Added troubleshooting guidance for setup failures caused by Node.js memory exhaustion.
    • Included steps to increase the Node.js heap limit and rerun the development setup process.

@copy-pr-bot

copy-pr-bot Bot commented Aug 10, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Added troubleshooting guidance for source-checkout setup failures caused by Node.js heap exhaustion on 8 GB hosts. The guidance sets a 5120 MB old-space limit and instructs contributors to rerun setup and subsequent commands.

Changes

Node.js heap guidance

Layer / File(s) Summary
Document heap workaround
docs/reference/troubleshooting.mdx
Added instructions to set the Node.js old-space limit before rerunning ./scripts/dev-setup.sh and subsequent commands.

Estimated code review effort: 1 (Trivial) | ~2 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the troubleshooting documentation added for the contributor setup heap error.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: Review the warnings below.
Findings: 0 blockers · 1 warning · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · medium confidence · 0 blockers · 1 warning · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Failed after a partial review · low confidence · 0 blockers · 0 warnings · 0 suggestions

Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests.

3 semantic terminology decisions

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

  • established — source checkout at docs/reference/troubleshooting.mdx:67: Keep `source checkout` for this contributor-installation scope.
  • justified — old-space limit at docs/reference/troubleshooting.mdx:69: Keep `old-space limit` because it identifies the specific Node.js heap limit.
  • justified — JavaScript heap out of memory at docs/reference/troubleshooting.mdx:71: Keep this diagnostic phrase to identify the reported Node.js failure.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite for the commit under review.

Recommended E2E: None

1 warning · 0 suggestions

Warnings

Warnings do not block.

PRA-1 Warning — Identify the setup step that exhausts the heap

  • Location: docs/reference/troubleshooting.mdx:71
  • Category: docs
  • Problem: The page states that the CLI type check needs more heap. The setup script starts TypeScript compilation during the preceding CLI build and plugin build steps, so checked-in code does not identify the CLI type check as the first failing operation.
  • Impact: A contributor whose setup fails during an earlier build step can misdiagnose the failure and receive an inaccurate explanation.
  • Recommendation: Verify the failing operation on the affected host. Then name that operation, or change the text to state that setup can fail during TypeScript compilation when the Node.js heap limit is too low.
  • Verification: Read `scripts/dev-setup.sh:361-369` and compare the documented step with captured output from the reported heap-exhaustion failure.
  • Test coverage: Add a documentation-source assertion that the troubleshooting entry describes the verified setup operation, or use step-neutral wording that matches the complete setup sequence.
  • Evidence: docs/reference/troubleshooting.mdx:71 states that the CLI type check needs more heap. scripts/dev-setup.sh:361-369 runs the CLI build and plugin build before the CLI type check. No checked-in measurement or test identifies `npm run typecheck:cli` as the affected operation.

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

cv
cv previously requested changes Aug 10, 2026

@cv cv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like a very specific troubleshooting issue. Can you put it in docs/reference/troubleshooting.mdx?

@wscurran wscurran added area: docs Documentation, examples, guides, or docs build chore Build, CI, dependency, or tooling maintenance labels Aug 10, 2026
@VishnuR23 VishnuR23 changed the title docs: document the Node.js heap limit for contributor setup docs: add a troubleshooting entry for the contributor setup heap error Aug 11, 2026
@VishnuR23

Copy link
Copy Markdown
Contributor Author

Moved it to docs/reference/troubleshooting.mdx as you asked. It now sits in the Installation section next to Node.js version is too old and Image push fails with out-of-memory errors, and CONTRIBUTING.md is byte-identical to main again.

Two notes:

  • The entry opens by stating that it applies to a source checkout rather than an installed release, since the rest of that section addresses installed releases.
  • A follow-up commit puts the heading in title case to follow the heading rule in docs/CONTRIBUTING.md. The neighbouring entries are mixed on this, so tell me if you would rather match the sentence-case ones.

npm run docs reports 0 errors. Happy to add a pointer from the CONTRIBUTING.md prerequisites to the new entry if you want contributors to find it before they run setup, but I left it out to keep the diff to one file.

@apurvvkumaria

Copy link
Copy Markdown
Collaborator

Maintainer readiness update

The requested relocation to docs/reference/troubleshooting.mdx is implemented, the documentation writer review is recorded, the docs build reports 0 errors with only the 2 warnings already present on main, contributor compliance passes, and there are no unresolved major or critical automated-review findings.

Two blockers remain:

  • The formal maintainer change-request is still active. I am preserving repository-owned reviewer routing and will not dismiss or re-request that review.
  • The secondary PR Review Advisor lane failed again during analysis after one safe rerun. The deterministic repository gate therefore remains fail-closed on CI and advisor provenance.

No approval, bypass, or merge was attempted. The PR remains open pending normal reviewer-state and advisor-automation recovery.

@cv
cv dismissed their stale review August 11, 2026 08:37

The latest PR commit moves the guidance to docs/reference/troubleshooting.mdx as requested.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@apurvvkumaria

Copy link
Copy Markdown
Collaborator

Security and documentation review for the latest PR commit is complete.

Security categories:

  1. Secrets and credentials: The change adds no credential value, credential path, or secret-handling behavior.
  2. Input validation and data sanitization: The documented NODE_OPTIONS value is a fixed numeric heap setting. The command does not interpolate untrusted input.
  3. Authentication and authorization: No identity, permission, or authorization boundary changes.
  4. Dependencies and third-party libraries: No dependency, lockfile, download source, or install hook changes.
  5. Error handling and logging: The entry explains the existing V8 failure and a bounded rerun. It does not suppress or reclassify a failure.
  6. Cryptography and data protection: No cryptographic or protected-data path changes.
  7. Configuration and security controls: The environment variable applies to the documented source-checkout setup command. It does not change an installed NemoClaw default.
  8. Security testing: The docs build, complete repository hook suite, secret scan, Markdown checks, and diff whitespace check pass.
  9. System security: The change has no runtime, network, filesystem-policy, process-privilege, or lifecycle effect.

Documentation writer result: docs-updated. The writer verified the source page and the generated OpenClaw, Hermes, and Deep Agents variants after the current-main refresh. The docs build reports 0 errors and the same 2 warnings as main.

Change size: +17/-0 in one documentation file. No security or documentation blocker was found. Human approval and every required GitHub check remain required before merge.

@prekshivyas prekshivyas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed latest PR commit 389854c0b; no substantive documentation finding.

The troubleshooting entry matches issue #8688's reproduced source-checkout failure and verified NODE_OPTIONS=--max-old-space-size=5120 workaround, scopes the guidance away from installed releases, and records that the variable must remain set for later heavy Node commands. The requested move into the troubleshooting page is complete.

No approval because base aaaf5c150 is behind current main and the exact-commit required check set is no longer current.

@prekshivyas prekshivyas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the current head and found no documentation correctness or style issue. I am withholding approval because this branch does not contain current main; refresh the branch and rerun the required checks so approval can bind to the integrated exact head.

@prekshivyas
prekshivyas merged commit 674c933 into NVIDIA:main Aug 12, 2026
26 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: docs Documentation, examples, guides, or docs build chore Build, CI, dependency, or tooling maintenance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants