Skip to content

feat: add MCP server for docs, policies, and source code#207

Closed
ac12644 wants to merge 4 commits intoNVIDIA:mainfrom
ac12644:feat/mcp-docs-server
Closed

feat: add MCP server for docs, policies, and source code#207
ac12644 wants to merge 4 commits intoNVIDIA:mainfrom
ac12644:feat/mcp-docs-server

Conversation

@ac12644
Copy link
Copy Markdown

@ac12644 ac12644 commented Mar 17, 2026

Summary

  • Add an MCP (Model Context Protocol) server (mcp-docs-server/) that exposes NemoClaw documentation, blueprint config, network policies, NIM model catalog, and source code as 12 searchable tools for AI assistants
  • Add 18 tests covering all tools, error cases, and search scope filtering
  • Add .mcp.json to .gitignore (contains user-specific paths)

Tools

Tool Description
list_docs List all documentation pages
read_doc Read a specific doc page by path
search Full-text search across docs, source code, policies, scripts, and configs
get_blueprint_config Blueprint YAML with inference profiles and sandbox config
get_baseline_policy Baseline sandbox network and filesystem policy
list_policy_presets Available network policy presets with endpoints
get_policy_preset Full YAML for a specific policy preset
get_nim_models NIM container image catalog with GPU memory requirements
read_source_file Read any source file from the repo
list_source_files Browse indexed source files by category
get_architecture_overview Architecture summary with components, flows, and models
get_dockerfile Sandbox container Dockerfile

Test plan

  • All 18 new tests pass (node --test test/mcp-docs-server.test.js)
  • All 51 existing tests still pass
  • Server initializes correctly over stdio
  • Verify MCP tools work end-to-end with .mcp.json config

Summary by CodeRabbit

  • New Features

    • Added a NemoClaw Docs MCP server exposing tools to browse/search docs, repository files, policies, blueprints, and NIM model catalogs.
  • Documentation

    • Added a README with setup and usage instructions for running the MCP server and integrating MCP clients.
  • Tests

    • Added end-to-end tests validating the MCP server tools and behaviors.
  • Chores

    • Added npm package metadata, updated CI to install server dependencies before tests, and updated ignore rules to exclude the MCP JSON config.

@wscurran wscurran added Getting Started Use this label to identify setup, installation, or onboarding issues. documentation Improvements or additions to documentation labels Mar 18, 2026
@cv
Copy link
Copy Markdown
Contributor

cv commented Mar 21, 2026

Thanks for working on the MCP server, @ac12644 — having docs, policies, and source code accessible that way is a cool idea. Quick note: main has evolved quite a bit since this PR was opened (CI pipeline, new features, etc.). We'd love to consider this, but a rebase onto the latest main would really help us give it a thorough review. Mind updating when you have a moment?

@ac12644 ac12644 force-pushed the feat/mcp-docs-server branch from f93021c to 4234a3d Compare March 22, 2026 04:08
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 22, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

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: f63a0530-fbe5-4494-914c-e29098fcae10

📥 Commits

Reviewing files that changed from the base of the PR and between 9b2007f and 7318bad.

📒 Files selected for processing (2)
  • .github/workflows/pr.yaml
  • .gitignore
✅ Files skipped from review due to trivial changes (1)
  • .gitignore
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/pr.yaml

📝 Walkthrough

Walkthrough

A new MCP server "nemoclaw-docs" is added; it indexes repository docs, policy presets, source/config files and exposes 12 MCP tools for listing, reading, searching, and retrieving blueprint/policy/models and architecture/Dockerfile content over stdio JSON-RPC.

Changes

Cohort / File(s) Summary
VCS & Repo Docs
/.gitignore, mcp-docs-server/README.md
Added .mcp.json to .gitignore; added README describing the NemoClaw Docs MCP Server, tool catalog, setup (Node.js 20+), and usage instructions.
MCP Server Package & Entrypoint
mcp-docs-server/index.js, mcp-docs-server/package.json
New MCP server that scans/indexes docs/, policy presets, source files, and key root files; implements unified keyword search and registers 12 tools (list_docs, read_doc, search, get_blueprint_config, get_baseline_policy, list_policy_presets, get_policy_preset, get_nim_models, list_source_files, read_source_file, get_architecture_overview, get_dockerfile). package.json defines module type, deps (@modelcontextprotocol/sdk, zod), Node>=20 requirement, and start script.
CI Workflow
.github/workflows/pr.yaml
Added an npm install step in the test-unit job to install mcp-docs-server dependencies before running tests.
Tests
test/mcp-docs-server.test.cjs
New end-to-end test suite that spawns the server over stdio JSON-RPC and validates tool listing, docs reading, search (with scopes), policy/blueprint endpoints, NIM catalog, source file listing/reading, architecture overview, Dockerfile reading, and package version.

Sequence Diagram

sequenceDiagram
    participant Client as MCP Client
    participant Server as MCP Server\n(nemoclaw-docs)
    participant FS as Repository\nFileSystem

    Client->>Server: initialize (MCP handshake)
    Server->>FS: scan docs/, policies/, sources, root files
    FS-->>Server: file contents
    Server->>Server: build in-memory indexes (docs, presets, sources, models)
    Server-->>Client: initialized

    Client->>Server: tools/list
    Server-->>Client: list of 12 tools

    Client->>Server: call tool (e.g., search | read_doc | get_nim_models)
    Server->>Server: query indexes or read fallback files (with path-deny checks)
    Server-->>Client: structured result or isError response
Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Poem

🐰 I hopped through docs and YAML trees,
Indexed each line on nimble knees,
Twelve tiny doors now open wide,
Read, search, and fetch — I beam with pride! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat: add MCP server for docs, policies, and source code' accurately and concisely summarizes the main objective of the changeset, which is adding a new MCP server with comprehensive tooling for documentation, policies, and source code access.

✏️ 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.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (4)
mcp-docs-server/index.js (1)

242-253: Consider adding minimum length validation for search query.

The zod schema allows empty strings for query, which combined with the search function issue could cause problems.

💡 Suggested fix
   {
-    query: z.string().describe("Search query (keywords or phrase)"),
+    query: z.string().min(1).describe("Search query (keywords or phrase)"),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mcp-docs-server/index.js` around lines 242 - 253, The search tool's Zod
schema currently allows empty queries because it uses z.string() for the "query"
field; update the schema used in server.tool for the "search" tool to enforce a
minimum length (e.g., use z.string().nonempty() or z.string().min(3)) so empty
strings are rejected and validation fails early before calling the search
handler; modify the schema where the "query" property is defined to include
.nonempty() or .min(n) to prevent empty queries.
test/mcp-docs-server.test.js (1)

202-214: Version test doesn't verify server-reported version.

The test reads package.json directly but doesn't assert that the server's initialize response contains the matching version. The comment on line 208 acknowledges this limitation.

💡 Suggestion to verify server version
   it("server version matches package.json", () => {
-    const res = mcpCall("initialize", {
-      protocolVersion: "2024-11-05",
-      capabilities: {},
-      clientInfo: { name: "test", version: "1.0" },
-    });
-    // The init response is id 1, but we sent it as id 2 in mcpCall wrapper
-    // Use a direct check instead
     const pkg = JSON.parse(
       fs.readFileSync(path.join(ROOT, "mcp-docs-server", "package.json"), "utf-8")
     );
-    assert.equal(pkg.version, "0.1.0", "package.json version mismatch");
+    // Get the initialize response (id 1) from a fresh server call
+    const res = mcpCall("tools/list"); // triggers initialize with id=1
+    // Parse output to find the initialize response
+    // For now, just verify package.json is consistent
+    assert.equal(pkg.version, "0.1.0", "package.json version mismatch");
   });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/mcp-docs-server.test.js` around lines 202 - 214, The test currently only
reads package.json (pkg) but never checks the server's initialize response;
update the test to capture the return value from mcpCall("initialize", ...) and
assert that the server-reported version equals pkg.version. Concretely, call
mcpCall("initialize", ...) into a variable (e.g., initRes), extract the
server-reported version from the response payload returned by mcpCall (the field
under the initialize result, e.g., initRes.result.serverInfo.version or
initRes.result.version depending on the response shape), and replace the lone
assert.equal(pkg.version, "0.1.0") with assert.equal(serverVersion, pkg.version)
to verify they match.
mcp-docs-server/README.md (1)

48-60: Consider removing leading $ from console commands.

The markdownlint MD014 rule flags dollar signs before commands when no output is shown. This is a stylistic preference to improve copy-paste UX.

📝 Suggested fix
 ```console
-$ cd /path/to/NemoClaw
-$ node mcp-docs-server/index.js
+cd /path/to/NemoClaw
+node mcp-docs-server/index.js

The server communicates over stdin/stdout using the MCP JSON-RPC protocol.

Install Dependencies

-$ cd mcp-docs-server
-$ npm install
+cd mcp-docs-server
+npm install
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @mcp-docs-server/README.md around lines 48 - 60, Remove the leading "$ "
prompt from the shell command lines in the README code blocks so they are plain,
copy-pastable commands; specifically update the command strings "cd
/path/to/NemoClaw", "node mcp-docs-server/index.js", "cd mcp-docs-server", and
"npm install" to have no leading dollar sign or space.


</details>

</blockquote></details>
<details>
<summary>mcp-docs-server/package.json (1)</summary><blockquote>

`1-23`: **Consider adding `"private": true` to prevent accidental npm publish.**

This internal package is unlikely to be published to npm. Adding `"private": true` prevents accidental publishes. The specified dependency versions (`@modelcontextprotocol/sdk` ^1.12.1 and zod ^3.24.0) are valid and recent. Note that zod v4.3.6 is available; if your code is compatible, consider updating to ^4.x for the latest features and improvements.

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @mcp-docs-server/package.json around lines 1 - 23, Add "private": true to the
package.json to prevent accidental npm publish; update the top-level object (the
same place as "name", "version", "description") and set "private": true so npm
will refuse publishing this internal package.


</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>🤖 Prompt for all review comments with AI agents</summary>

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @mcp-docs-server/index.js:

  • Around line 439-444: The mapping that builds rows from catalog.models uses
    m.minGpuMemoryMB without guarding against null/undefined, causing NaN or runtime
    errors; update the rows generation (the map that produces the string for rows)
    to defensively read m.minGpuMemoryMB (and optionally m.name/m.image) and fall
    back to a safe default (e.g., 0 MB or "N/A") before dividing/formatting so
    (Number(m.minGpuMemoryMB) || 0) (or an "N/A" branch) is used and the resulting
    string is always valid.
  • Around line 481-493: The fallback read uses path.join(ROOT, file_path) and
    readIfExists without verifying the resolved path, allowing path traversal;
    update the code around path.join/ROOT/file_path/readIfExists to resolve the
    target path (use path.resolve or path.relative) and ensure the resolved path is
    inside ROOT (e.g., reject if path.relative(ROOT, resolvedPath) starts with '..'
    or if resolvedPath does not start with ROOT), then only call readIfExists on
    validated paths and return the existing "File not found" error response for any
    disallowed/outside paths; reference the symbols path.join, ROOT, file_path,
    readIfExists, and sourceFiles to locate and change this logic.
  • Around line 161-176: The search function can infinite-loop when query yields
    an empty term (terms = [""]) because content.indexOf("") returns 0 repeatedly;
    update the logic in search to guard against empty terms by filtering out empty
    strings from terms (e.g., terms =
    query.toLowerCase().split(/\s+/).filter(Boolean)) or early-return an empty array
    when query.trim() is empty, and/or skip the indexOf loop when term === "" (check
    term before the while using content.indexOf); ensure the change references the
    existing symbols: search, terms, content.indexOf, idx so the fix is applied
    where the score and while loop are computed.
  • Line 14: The current ROOT computation uses new URL(import.meta.url).pathname
    which can produce a leading slash on Windows; replace that with Node's
    fileURLToPath(import.meta.url) to get a proper platform-correct file path, then
    pass that into path.dirname and path.resolve to compute ROOT (update the
    expression that defines ROOT to use fileURLToPath(import.meta.url) instead of
    new URL(import.meta.url).pathname and ensure fileURLToPath is imported from
    'url').

Nitpick comments:
In @mcp-docs-server/index.js:

  • Around line 242-253: The search tool's Zod schema currently allows empty
    queries because it uses z.string() for the "query" field; update the schema used
    in server.tool for the "search" tool to enforce a minimum length (e.g., use
    z.string().nonempty() or z.string().min(3)) so empty strings are rejected and
    validation fails early before calling the search handler; modify the schema
    where the "query" property is defined to include .nonempty() or .min(n) to
    prevent empty queries.

In @mcp-docs-server/package.json:

  • Around line 1-23: Add "private": true to the package.json to prevent
    accidental npm publish; update the top-level object (the same place as "name",
    "version", "description") and set "private": true so npm will refuse publishing
    this internal package.

In @mcp-docs-server/README.md:

  • Around line 48-60: Remove the leading "$ " prompt from the shell command lines
    in the README code blocks so they are plain, copy-pastable commands;
    specifically update the command strings "cd /path/to/NemoClaw", "node
    mcp-docs-server/index.js", "cd mcp-docs-server", and "npm install" to have no
    leading dollar sign or space.

In @test/mcp-docs-server.test.js:

  • Around line 202-214: The test currently only reads package.json (pkg) but
    never checks the server's initialize response; update the test to capture the
    return value from mcpCall("initialize", ...) and assert that the server-reported
    version equals pkg.version. Concretely, call mcpCall("initialize", ...) into a
    variable (e.g., initRes), extract the server-reported version from the response
    payload returned by mcpCall (the field under the initialize result, e.g.,
    initRes.result.serverInfo.version or initRes.result.version depending on the
    response shape), and replace the lone assert.equal(pkg.version, "0.1.0") with
    assert.equal(serverVersion, pkg.version) to verify they match.

</details>

---

<details>
<summary>ℹ️ Review info</summary>

<details>
<summary>⚙️ Run configuration</summary>

**Configuration used**: Path: .coderabbit.yaml

**Review profile**: CHILL

**Plan**: Pro

**Run ID**: `e6257fe7-efc9-4ee7-aefc-d7a888b53635`

</details>

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between b1fb0133b0b3ab6faf38f9f78f304a5fd7b2da7c and 4234a3d7f03f82005aa6d9777e6236f00dcdb8bc.

</details>

<details>
<summary>⛔ Files ignored due to path filters (1)</summary>

* `mcp-docs-server/package-lock.json` is excluded by `!**/package-lock.json`

</details>

<details>
<summary>📒 Files selected for processing (5)</summary>

* `.gitignore`
* `mcp-docs-server/README.md`
* `mcp-docs-server/index.js`
* `mcp-docs-server/package.json`
* `test/mcp-docs-server.test.js`

</details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

Comment thread mcp-docs-server/index.js Outdated
Comment thread mcp-docs-server/index.js
Comment thread mcp-docs-server/index.js
Comment thread mcp-docs-server/index.js Outdated
@ac12644 ac12644 force-pushed the feat/mcp-docs-server branch from 4234a3d to fd496d1 Compare March 22, 2026 04:20
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@mcp-docs-server/index.js`:
- Around line 560-564: The nimRows generation maps nimCatalog.models and assumes
m.minGpuMemoryMB is present, which can produce NaN; update the mapping in the
nimRows creation to defensively handle missing or non-finite m.minGpuMemoryMB
(e.g., check Number.isFinite(m.minGpuMemoryMB) or m.minGpuMemoryMB != null) and
use a safe fallback (like 0 or "N/A") before calling toFixed; reference the
nimRows variable and the m.minGpuMemoryMB property so the change is applied to
the map callback that formats `m.name` and GPU memory.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 00b1ddcd-1a34-4198-8f60-899f36b017b3

📥 Commits

Reviewing files that changed from the base of the PR and between 4234a3d and fd496d1.

⛔ Files ignored due to path filters (1)
  • mcp-docs-server/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (5)
  • .gitignore
  • mcp-docs-server/README.md
  • mcp-docs-server/index.js
  • mcp-docs-server/package.json
  • test/mcp-docs-server.test.js
✅ Files skipped from review due to trivial changes (3)
  • .gitignore
  • mcp-docs-server/package.json
  • test/mcp-docs-server.test.js

Comment thread mcp-docs-server/index.js
Comment on lines +560 to +564
const nimRows = nimCatalog?.models
? nimCatalog.models
.map((m) => `| \`${m.name}\` | ${(m.minGpuMemoryMB / 1024).toFixed(0)} GB |`)
.join("\n")
: "| (catalog unavailable) | |";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Missing null check for minGpuMemoryMB in architecture overview.

The get_nim_models tool (line 445) defensively handles missing minGpuMemoryMB, but this code path does not, risking NaN output.

🛡️ Proposed fix for consistency
     const nimRows = nimCatalog?.models
       ? nimCatalog.models
-          .map((m) => `| \`${m.name}\` | ${(m.minGpuMemoryMB / 1024).toFixed(0)} GB |`)
+          .map((m) => `| \`${m.name || "unknown"}\` | ${m.minGpuMemoryMB ? (m.minGpuMemoryMB / 1024).toFixed(0) : "?"} GB |`)
           .join("\n")
       : "| (catalog unavailable) | |";
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const nimRows = nimCatalog?.models
? nimCatalog.models
.map((m) => `| \`${m.name}\` | ${(m.minGpuMemoryMB / 1024).toFixed(0)} GB |`)
.join("\n")
: "| (catalog unavailable) | |";
const nimRows = nimCatalog?.models
? nimCatalog.models
.map((m) => `| \`${m.name || "unknown"}\` | ${m.minGpuMemoryMB ? (m.minGpuMemoryMB / 1024).toFixed(0) : "?"} GB |`)
.join("\n")
: "| (catalog unavailable) | |";
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mcp-docs-server/index.js` around lines 560 - 564, The nimRows generation maps
nimCatalog.models and assumes m.minGpuMemoryMB is present, which can produce
NaN; update the mapping in the nimRows creation to defensively handle missing or
non-finite m.minGpuMemoryMB (e.g., check Number.isFinite(m.minGpuMemoryMB) or
m.minGpuMemoryMB != null) and use a safe fallback (like 0 or "N/A") before
calling toFixed; reference the nimRows variable and the m.minGpuMemoryMB
property so the change is applied to the map callback that formats `m.name` and
GPU memory.

@ac12644
Copy link
Copy Markdown
Author

ac12644 commented Mar 22, 2026

Thanks @cv ! Just rebased onto the latest main, should be ready for review now.
Let me know if anything needs adjusting.

@cv
Copy link
Copy Markdown
Contributor

cv commented Mar 22, 2026

CI fix needed: missing mcp-docs-server dependencies

test-unit is failing because the mcp-docs-server/ subdirectory has its own package.json with @modelcontextprotocol/sdk and zod as dependencies, but the CI workflow (.github/workflows/pr.yaml) only runs npm install at the root — it never installs mcp-docs-server/node_modules.

Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@modelcontextprotocol/sdk'
  imported from mcp-docs-server/index.js

A couple of options to fix this:

Option A — Add an install step for the MCP server in the test-unit job (after the existing root install):

      - name: Install MCP docs server dependencies
        working-directory: mcp-docs-server
        run: npm install

Option B — Add @modelcontextprotocol/sdk and zod as root devDependencies so the existing npm install covers it. This avoids a second install step but couples the root package to MCP server deps.

Option A keeps the dependency scope isolated to mcp-docs-server/ and matches the pattern already used for nemoclaw/, so it's probably the cleaner path.

@ac12644 ac12644 force-pushed the feat/mcp-docs-server branch from fd496d1 to 2b09053 Compare March 23, 2026 03:47
Add an MCP (Model Context Protocol) server that exposes NemoClaw
documentation, blueprint config, network policies, NIM model catalog,
and source code as 12 searchable tools for AI assistants.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Abhishek Chauhan <ac12644@gmail.com>
@ac12644 ac12644 force-pushed the feat/mcp-docs-server branch from 2b09053 to c661b07 Compare March 23, 2026 03:48
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
mcp-docs-server/index.js (1)

560-564: ⚠️ Potential issue | 🟡 Minor

Missing null check for minGpuMemoryMB in architecture overview.

The get_nim_models tool (line 445) defensively handles missing minGpuMemoryMB, but this code path does not, risking NaN output. This inconsistency was flagged in a prior review.

🛡️ Proposed fix for consistency
     const nimRows = nimCatalog?.models
       ? nimCatalog.models
-          .map((m) => `| \`${m.name}\` | ${(m.minGpuMemoryMB / 1024).toFixed(0)} GB |`)
+          .map((m) => `| \`${m.name || "unknown"}\` | ${m.minGpuMemoryMB ? (m.minGpuMemoryMB / 1024).toFixed(0) : "?"} GB |`)
           .join("\n")
       : "| (catalog unavailable) | |";
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mcp-docs-server/index.js` around lines 560 - 564, nimRows builds a table from
nimCatalog.models but doesn't guard against missing minGpuMemoryMB, which can
produce NaN; update the mapping in nimRows to check m.minGpuMemoryMB != null (or
!== undefined) and render a safe fallback (e.g., "unknown" or an empty value)
when it's missing instead of dividing and calling toFixed on undefined,
mirroring the defensive handling used in get_nim_models.
🧹 Nitpick comments (2)
test/mcp-docs-server.test.cjs (1)

202-214: Version test doesn't verify the server's reported version.

The test reads package.json directly and asserts it equals "0.1.0", but it doesn't check that the server actually returns this version in its initialize response. The mcpCall helper always looks for response id: 2, but the initialize response has id: 1.

Consider either:

  1. Extracting and verifying serverInfo.version from the id: 1 response
  2. Renaming the test to clarify it only validates the package.json value
♻️ Proposed fix to verify server-reported version
   it("server version matches package.json", () => {
-    const res = mcpCall("initialize", {
-      protocolVersion: "2024-11-05",
-      capabilities: {},
-      clientInfo: { name: "test", version: "1.0" },
-    });
-    // The init response is id 1, but we sent it as id 2 in mcpCall wrapper
-    // Use a direct check instead
+    // Get the initialize response (id: 1) from the mcpCall output
+    const init = JSON.stringify({
+      jsonrpc: "2.0",
+      id: 1,
+      method: "initialize",
+      params: {
+        protocolVersion: "2024-11-05",
+        capabilities: {},
+        clientInfo: { name: "test", version: "1.0" },
+      },
+    });
+    const output = execSync(`node "${SERVER}"`, {
+      input: `${init}\n`,
+      encoding: "utf-8",
+      timeout: 15000,
+      cwd: ROOT,
+    });
+    const response = JSON.parse(output.trim().split("\n")[0]);
     const pkg = JSON.parse(
       fs.readFileSync(path.join(ROOT, "mcp-docs-server", "package.json"), "utf-8")
     );
-    assert.equal(pkg.version, "0.1.0", "package.json version mismatch");
+    assert.equal(response.result.serverInfo.version, pkg.version, "server version mismatch");
   });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/mcp-docs-server.test.cjs` around lines 202 - 214, The test currently
reads package.json (pkg) and asserts its version but never checks the server's
initialize response (which comes back with id: 1) because mcpCall wrapper
expects id: 2; update the test to capture the raw initialize response (call
"initialize" and read the response with id === 1 or adjust mcpCall to accept an
expectedId) and assert that response.result.serverInfo.version equals
pkg.version (or assert response.serverInfo.version depending on the shape),
using the existing mcpCall/initialize call and pkg variable to compare the two
versions so the test verifies the server-reported version matches package.json.
mcp-docs-server/README.md (1)

48-60: Consider removing leading $ from console commands.

The static analysis tool flags dollar signs used before commands without showing output. While this is a common convention, removing them improves copy-paste usability.

♻️ Proposed fix
 ```console
-$ cd /path/to/NemoClaw
-$ node mcp-docs-server/index.js
+cd /path/to/NemoClaw
+node mcp-docs-server/index.js

The server communicates over stdin/stdout using the MCP JSON-RPC protocol.

Install Dependencies

-$ cd mcp-docs-server
-$ npm install
+cd mcp-docs-server
+npm install
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @mcp-docs-server/README.md around lines 48 - 60, Remove the leading "$"
prompt characters from the shell command examples in README.md so they are
copy-pastable; update the code blocks containing "cd /path/to/NemoClaw", "node
mcp-docs-server/index.js", "cd mcp-docs-server", and "npm install" to omit the
"$" prefixes while leaving the rest of the text and formatting intact.


</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>🤖 Prompt for all review comments with AI agents</summary>

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In @mcp-docs-server/index.js:

  • Around line 560-564: nimRows builds a table from nimCatalog.models but doesn't
    guard against missing minGpuMemoryMB, which can produce NaN; update the mapping
    in nimRows to check m.minGpuMemoryMB != null (or !== undefined) and render a
    safe fallback (e.g., "unknown" or an empty value) when it's missing instead of
    dividing and calling toFixed on undefined, mirroring the defensive handling used
    in get_nim_models.

Nitpick comments:
In @mcp-docs-server/README.md:

  • Around line 48-60: Remove the leading "$" prompt characters from the shell
    command examples in README.md so they are copy-pastable; update the code blocks
    containing "cd /path/to/NemoClaw", "node mcp-docs-server/index.js", "cd
    mcp-docs-server", and "npm install" to omit the "$" prefixes while leaving the
    rest of the text and formatting intact.

In @test/mcp-docs-server.test.cjs:

  • Around line 202-214: The test currently reads package.json (pkg) and asserts
    its version but never checks the server's initialize response (which comes back
    with id: 1) because mcpCall wrapper expects id: 2; update the test to capture
    the raw initialize response (call "initialize" and read the response with id ===
    1 or adjust mcpCall to accept an expectedId) and assert that
    response.result.serverInfo.version equals pkg.version (or assert
    response.serverInfo.version depending on the shape), using the existing
    mcpCall/initialize call and pkg variable to compare the two versions so the test
    verifies the server-reported version matches package.json.

</details>

---

<details>
<summary>ℹ️ Review info</summary>

<details>
<summary>⚙️ Run configuration</summary>

**Configuration used**: Path: .coderabbit.yaml

**Review profile**: CHILL

**Plan**: Pro

**Run ID**: `4426a3c0-87ae-47f7-8a15-942c52fad75c`

</details>

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between fd496d1c1fcb93204b75869350da14bce7b82e43 and c661b07aed4ace88a893dcc7311cd524c75ed48c.

</details>

<details>
<summary>⛔ Files ignored due to path filters (1)</summary>

* `mcp-docs-server/package-lock.json` is excluded by `!**/package-lock.json`

</details>

<details>
<summary>📒 Files selected for processing (6)</summary>

* `.github/workflows/pr.yaml`
* `.gitignore`
* `mcp-docs-server/README.md`
* `mcp-docs-server/index.js`
* `mcp-docs-server/package.json`
* `test/mcp-docs-server.test.cjs`

</details>

<details>
<summary>✅ Files skipped from review due to trivial changes (2)</summary>

* .gitignore
* mcp-docs-server/package.json

</details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

@wscurran wscurran requested a review from cv March 23, 2026 17:40
mafueee pushed a commit to mafueee/NemoClaw that referenced this pull request Mar 28, 2026
…l inference (NVIDIA#209)

* feat(inference): add sandbox-system inference route for platform-level inference

Add a separate system-level inference route that the sandbox supervisor
can use in-process for platform functions (e.g., embedded agent harness
for policy analysis), distinct from the user-facing inference.local
endpoint. The system route is accessed via an in-process API on the
supervisor, ensuring userland code in the sandbox netns cannot reach it.

- Extend proto with route_name fields on Set/Get inference messages
- Add ResolvedRoute.name field to the router for route segregation
- Server resolves both user and sandbox-system routes in bundles
- Sandbox partitions routes into user/system caches on refresh
- Expose InferenceContext::system_inference() in-process API
- CLI --sandbox flag targets the system route on set/get/update
- Integration tests using mock:// routes for the full in-process path

Closes NVIDIA#207

* refactor(cli): rename --sandbox flag to --system for inference commands

The --sandbox flag could be misread as targeting a user-level sandbox
operation. Rename to --system to clearly indicate it configures the
platform-level system inference route.

* style(cli): collapse short if-else per rustfmt 1.94

---------

Co-authored-by: John Myers <johntmyers@users.noreply.github.com>
@ac12644 ac12644 force-pushed the feat/mcp-docs-server branch from 5c67671 to 7318bad Compare March 29, 2026 11:37
@ac12644
Copy link
Copy Markdown
Author

ac12644 commented Mar 29, 2026

@cv Rebased and merged with latest main. Also addressed all CodeRabbit feedback (path traversal fix, empty query guard, null checks, Windows path resolution) and added the MCP server install step to CI. Ready for review.

@cv
Copy link
Copy Markdown
Contributor

cv commented Apr 9, 2026

@ac12644 this is a good idea, but since the interfaces for the NemoClaw CLI are still very unstable, I think this work would be better done a little later. I'm going to close this for now, and once the UX is more stable, we can revisit. I'd like to have a nemoclaw mcp serve or something along those lines baked into the main code itself, if we are going to do this.

@cv cv closed this Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation Getting Started Use this label to identify setup, installation, or onboarding issues.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants