-
Notifications
You must be signed in to change notification settings - Fork 487
Add serve and client for nat mcp
#811
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add serve and client for nat mcp
#811
Conversation
Signed-off-by: Daniel Wang <[email protected]>
WalkthroughAdds a top-level Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as nat CLI
participant MCPGroup as nat mcp
participant Serve as "serve"
participant Start as start_command (MCP)
participant MCPServer as MCP Server Runtime
User->>CLI: nat mcp serve [OPTIONS]
CLI->>MCPGroup: dispatch "mcp"
MCPGroup->>Serve: invoke "serve"
Note right of Serve: serve delegates to existing start logic
Serve->>Start: get_command(None,"mcp") / invoke
Start->>MCPServer: initialize & run server
MCPServer-->>User: server ready / responses
sequenceDiagram
autonumber
actor User
participant CLI as nat CLI
participant MCPClient as nat mcp client
participant ClientLib as MCP client wrappers
participant MCPServer as MCP Server Runtime
User->>CLI: nat mcp client tool list / ping / call
CLI->>MCPClient: parse options (url, transport, direct, command, args)
MCPClient->>ClientLib: list / ping / call (stdio / sse / http)
ClientLib->>MCPServer: transport-specific interaction
MCPServer-->>ClientLib: response / tool list / call result
ClientLib-->>MCPClient: formatted result
MCPClient-->>User: text or JSON output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/nat/cli/entrypoint.py (1)
1-24: Conflicting dual license headers (Apache-2.0 and NvidiaProprietary). Remove the proprietary block.Repo policy says Apache-2.0. Keep a single SPDX header.
-# SPDX-FileCopyrightText: Copyright (c) 2024-2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: LicenseRef-NvidiaProprietary -# -# NVIDIA CORPORATION, its affiliates and licensors retain all intellectual -# property and proprietary rights in and to this material, related -# documentation and any modifications thereto. Any use, reproduction, -# disclosure or distribution of this material and related documentation -# without an express license agreement from NVIDIA CORPORATION or -# its affiliates is strictly prohibited. +# (Removed proprietary header; file licensed under Apache-2.0 above.)
🧹 Nitpick comments (3)
docs/source/workflows/mcp/mcp-auth.md (1)
29-31: CLI switch to “nat mcp serve” looks correct. Add a clear production-safety warning.This page is an experimental scratch pad and uses a stub TokenVerifier that accepts any token. Add an explicit warning to avoid accidental production use.
Suggested addition near the top:
+:::{warning} +Experimental content. The sample MCP server uses a stub TokenVerifier that accepts any token. Do NOT use in production. +:::src/nat/cli/entrypoint.py (1)
59-64: Optional: add return type hints for functions.Add return annotations to meet the project’s typing guideline.
-def setup_logging(log_level: str): +def setup_logging(log_level: str) -> int: @@ -def get_version(): +def get_version() -> str:Also applies to: 66-74, 76-85
src/nat/cli/commands/mcp/mcp.py (1)
25-35: Minor polish: set explicit group name, add return type, and drop unused logger.
- Use name="mcp" for clarity (registration name matches).
- Add -> None to satisfy typing guideline.
- Remove unused logger to avoid linter noise (or use it).
-import logging - import click @@ -from nat.cli.commands.start import start_command - -logger = logging.getLogger(__name__) +from nat.cli.commands.start import start_command @@ -@click.group(name=__name__, invoke_without_command=False, help="MCP-related commands.") -def mcp_command(): +@click.group(name="mcp", invoke_without_command=False, help="MCP-related commands.") +def mcp_command() -> None: """ MCP-related commands. """ return None
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/source/reference/cli.md(2 hunks)docs/source/workflows/mcp/mcp-auth.md(1 hunks)docs/source/workflows/mcp/mcp-client.md(1 hunks)docs/source/workflows/mcp/mcp-server.md(2 hunks)examples/MCP/simple_calculator_mcp/README.md(5 hunks)src/nat/cli/commands/mcp/__init__.py(1 hunks)src/nat/cli/commands/mcp/mcp.py(1 hunks)src/nat/cli/entrypoint.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
src/nat/cli/commands/mcp/__init__.pydocs/source/workflows/mcp/mcp-auth.mddocs/source/workflows/mcp/mcp-client.mddocs/source/workflows/mcp/mcp-server.mdexamples/MCP/simple_calculator_mcp/README.mdsrc/nat/cli/commands/mcp/mcp.pydocs/source/reference/cli.mdsrc/nat/cli/entrypoint.py
src/nat/**/*
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/cli/commands/mcp/__init__.pysrc/nat/cli/commands/mcp/mcp.pysrc/nat/cli/entrypoint.py
docs/source/**/*
⚙️ CodeRabbit configuration file
This directory contains the source code for the documentation. All documentation should be written in Markdown format. Any image files should be placed in the
docs/source/_staticdirectory.
Files:
docs/source/workflows/mcp/mcp-auth.mddocs/source/workflows/mcp/mcp-client.mddocs/source/workflows/mcp/mcp-server.mddocs/source/reference/cli.md
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: - This directory contains example code and usage scenarios for the toolkit, at a minimum an example should
contain a README.md or file README.ipynb.
- If an example contains Python code, it should be placed in a subdirectory named
src/and should
contain apyproject.tomlfile. Optionally, it might also contain scripts in ascripts/directory.- If an example contains YAML files, they should be placed in a subdirectory named
configs/. - If an example contains sample data files, they should be placed in a subdirectory nameddata/, and should
be checked into git-lfs.
Files:
examples/MCP/simple_calculator_mcp/README.md
🧠 Learnings (1)
📚 Learning: 2025-08-22T20:18:28.041Z
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/cursor-rules.mdc:0-0
Timestamp: 2025-08-22T20:18:28.041Z
Learning: Applies to .cursor/rules/**/*.mdc : Use consistent project terminology in descriptions (e.g., NAT workflows, NAT CLI commands)
Applied to files:
docs/source/workflows/mcp/mcp-server.mddocs/source/reference/cli.md
🔇 Additional comments (8)
src/nat/cli/commands/mcp/__init__.py (1)
1-15: SPDX header: LGTM.examples/MCP/simple_calculator_mcp/README.md (1)
38-39: Docs updated to “nat mcp serve”: looks good.Nice consistency across examples.
Also applies to: 81-83, 101-102, 110-111, 119-120
docs/source/reference/cli.md (2)
169-170: Example updated to “nat mcp serve”: LGTM.
148-164: Sync CLI docs with implementation: reconcile --tool_names and --transport
- No click.option matching --tool_names was found in the codebase; confirm the actual flag name and whether it accepts repeated flags (multiple=True) or a CSV, then update the docs to match.
- @click.option('--transport', type=click.Choice(['sse','stdio','streamable-http']), default='streamable-http') appears in src/nat/cli/commands/info/list_mcp.py (around lines ~343 and ~414). Confirm whether nat mcp serve supports --transport — if yes add it to the docs, if not remove references.
src/nat/cli/entrypoint.py (2)
36-37: Registering the new top-level “mcp” group: LGTM.
115-116: Command wiring complete: LGTM.Adds
mcpwithout reusing the old alias. Good.docs/source/workflows/mcp/mcp-client.md (1)
205-206: Align package-name terminology (nvidia-nat-mcpvsnvidia-nat[mcp]) across docsInvocation
nat mcp serveis correct; search returned no matches fornvidia-nat-mcpornvidia-nat[mcp]in docs — verify repo‑wide and standardize install instructions/docs to a single canonical form (e.g.,nvidia-nat[mcp]if pip extras are intended).Location: docs/source/workflows/mcp/mcp-client.md (lines 205–206).
docs/source/workflows/mcp/mcp-server.md (1)
26-39: No changes required: --transport and repeated --tool_names behave as documented.start_command dynamically generates CLI options from the MCP front-end config: MCPFrontEndConfig defines transport (Literal['sse','streamable-http']) and tool_names (list[str]); StartCommandGroup._build_params creates --transport and --tool_names, and tool_names is registered with multiple=True so repeated --tool_names flags (not a CSV) are correct. Note: list_mcp (info command) also exposes --transport (includes 'stdio' for client use).
Files of interest:
- src/nat/front_ends/mcp/mcp_front_end_config.py (defines transport, tool_names)
- src/nat/cli/commands/start.py (StartCommandGroup._build_params — dynamic CLI option creation)
- src/nat/cli/commands/mcp/mcp.py (mcp -> serve wiring via start_command.get_command)
- src/nat/front_ends/mcp/mcp_front_end_plugin_worker.py (uses front_end_config.tool_names to filter)
- docs/source/workflows/mcp/mcp-server.md (examples match current CLI)
Signed-off-by: Zhongxuan (Daniel) Wang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
docs/source/workflows/mcp/mcp-server.md (4)
18-22: Fix branding on first mention and casing of “server”.Follow the style guide: first use “NVIDIA NeMo Agent toolkit”; subsequent uses “NeMo Agent toolkit”. Also use “server” (lowercase).
-# NeMo Agent Toolkit as an MCP Server +# NVIDIA NeMo Agent toolkit as an MCP server @@ -This guide will cover how to use NeMo Agent toolkit as an MCP Server to publish tools using MCP. +This guide will cover how to use NeMo Agent toolkit as an MCP server to publish tools using MCP.
79-87: Don’t expose debug routes publicly.Add a short note that the debug endpoint is for development and should be firewalled/disabled in production to avoid leaking tool schemas.
153-157: Grammar: “an MCP”.Use “an MCP” before vowel sound.
-In this example, we will use NeMo Agent toolkit as both a MCP client and a MCP server. +In this example, we will use NeMo Agent toolkit as both an MCP client and an MCP server.
63-69: Unify MCP install instructions — prefer the extras formnvidia-nat[mcp]Docs currently say
nvidia-nat-mcpbut the install command and most docs usenvidia-nat[mcp]. Update the doc and the CLI message to use the extras form (files to change: docs/source/workflows/mcp/mcp-server.md lines 63–69 and src/nat/cli/commands/info/list_mcp.py:162).-**Note:** The `nat info mcp` command requires the `nvidia-nat-mcp` package. If you encounter an error about missing MCP client functionality, install it with `uv pip install nvidia-nat[mcp]`. +**Note:** The `nat info mcp` command requires MCP client support. Install it with: +`uv pip install 'nvidia-nat[mcp]'`
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/source/workflows/mcp/mcp-client.md(1 hunks)docs/source/workflows/mcp/mcp-server.md(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/source/workflows/mcp/mcp-client.md
🧰 Additional context used
📓 Path-based instructions (5)
docs/source/**/*.md
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
docs/source/**/*.md: Use the official naming: first use “NVIDIA NeMo Agent toolkit”; subsequent uses “NeMo Agent toolkit”; never use deprecated names in documentation
Documentation sources must be Markdown under docs/source; keep docs in sync and fix Sphinx errors/broken links
Documentation must be clear, comprehensive, free of TODO/FIXME/placeholders/offensive/outdated terms; fix spelling; adhere to Vale vocab allow/reject lists
Files:
docs/source/workflows/mcp/mcp-server.md
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}: Every file must start with the standard SPDX Apache-2.0 header; keep copyright years up‑to‑date
All source files must include the SPDX Apache‑2.0 header; do not bypass CI header checks
Files:
docs/source/workflows/mcp/mcp-server.md
**/*.{py,md}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never hard‑code version numbers in code or docs; versions are derived by setuptools‑scm
Files:
docs/source/workflows/mcp/mcp-server.md
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
docs/source/workflows/mcp/mcp-server.md
docs/source/**/*
⚙️ CodeRabbit configuration file
This directory contains the source code for the documentation. All documentation should be written in Markdown format. Any image files should be placed in the
docs/source/_staticdirectory.
Files:
docs/source/workflows/mcp/mcp-server.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI Pipeline / Check
🔇 Additional comments (5)
docs/source/workflows/mcp/mcp-server.md (5)
26-34: CLI rename reflected correctly.Docs now use “nat mcp serve …” and describe default endpoint/transport consistently. Looks good.
36-41: SSE transport example looks correct.Backward‑compat flag and endpoint path are clear.
42-48: Options example LGTM.Host/port/name flags match the new CLI structure.
53-59: Tool filtering example LGTM; clarify repetition vs list if supported.If CLI accepts only repeated flags (as shown), this is fine. If comma‑separated lists are also accepted, consider noting it for parity with the debug route section.
117-121: Verify flags for “nat info mcp”.The text mentions a
--detailflag; confirm the exact flag names and defaults for the CLI and adjust if needed.
Signed-off-by: Daniel Wang <[email protected]>
|
@coderabbitai review |
|
@zhongxuanwang-nv I'll review the changes in this PR that introduces the new 🧠 Learnings used✅ Actions performedReview triggered.
|
Signed-off-by: Daniel Wang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/nat/cli/commands/info/info.py (1)
26-31: Add type hints to Click group.Per guidelines, annotate parameters and return type.
-@click.group(name=__name__, invoke_without_command=False, help="Provide information about the local NAT environment.") -def info_command(**kwargs): +@click.group(name=__name__, invoke_without_command=False, help="Provide information about the local NAT environment.") +def info_command(**kwargs: object) -> None: """ Provide information about the local NAT environment. """ passdocs/source/workflows/mcp/mcp-client.md (1)
28-33: Replace installation command with the standalone packagenvidia-nat-mcp.File: docs/source/workflows/mcp/mcp-client.md (lines 28–33) — docs show
nvidia-nat[mcp]but PyPI and runtime messages usenvidia-nat-mcp; update the command.-uv pip install nvidia-nat[mcp] +uv pip install nvidia-nat-mcp
🧹 Nitpick comments (9)
src/nat/cli/commands/info/info.py (1)
16-24: Remove unused logger import/variable.
loggeris not used in this module.-import logging @@ -logger = logging.getLogger(__name__)src/nat/cli/commands/mcp/mcp.py (4)
54-56: Silence ARG001: mark unusedinclude_traceback.- def format_mcp_error(error, include_traceback=False): - click.echo(f"Error: {error}", err=True) + def format_mcp_error(error: Exception, include_traceback: bool = False) -> None: + _ = include_traceback + click.echo(f"Error: {error}", err=True)
303-307: Remove unused F821noqa; ExceptionGroup is built‑in on 3.11+.- if isinstance(e, ExceptionGroup): # noqa: F821 + if isinstance(e, ExceptionGroup): primary_exception = extract_primary_exception(list(e.exceptions)) mcp_error = convert_to_mcp_error(primary_exception, url)
614-619: Remove unused F821noqa; ExceptionGroup is built‑in on 3.11+.- if isinstance(e, ExceptionGroup): # noqa: F821 + if isinstance(e, ExceptionGroup): primary_exception = extract_primary_exception(list(e.exceptions)) mcp_error = convert_to_mcp_error(primary_exception, endpoint) else: mcp_error = convert_to_mcp_error(e, endpoint)
776-777: Log unexpected exceptions with stack traces (CLI guidance).When not re‑raising, prefer
logger.exceptionto capture trace.- except Exception as e: - click.echo(f"[ERROR] {e}", err=True) + except Exception as e: + logger.exception("Unhandled error in mcp_client_tool_call") + click.echo(f"[ERROR] {e}", err=True)docs/source/workflows/mcp/mcp-client.md (2)
35-39: Grammar nit.-The library handles the MCP server connection, tool discovery, and function registration. This allow the workflow to use MCP tools as regular functions. +The library handles the MCP server connection, tool discovery, and function registration. This allows the workflow to use MCP tools as regular functions.
262-267: Troubleshooting: suggestpingexplicitly.You already mention ping; consider adding an explicit command example for clarity.
- Verify the MCP server is running and accessible via the `nat mcp client ping` command + - Verify the MCP server is running and accessible, e.g.: + ```bash + nat mcp client ping --url http://localhost:9901/mcp + ```tests/nat/cli/commands/mcp/test_mcp_cli.py (2)
464-470: Avoid unusednoqaand dead params in test helpers.Drop unused
# noqa: ARG002and unused args to keep Ruff quiet.- def _broken_list_tools(*args, **kwargs): # noqa: ARG002 + def _broken_list_tools(*_args, **_kwargs): raise RuntimeError("boom")
505-513: Prefer logging over baretry/except/passin timeout shim.Minor, but add a comment or log to explain intentional swallow.
- async def _raise_timeout(coro, timeout=None, **_kwargs): # noqa: ARG002 + async def _raise_timeout(coro, timeout=None, **_kwargs): del timeout, _kwargs # Dispose the passed coroutine to avoid "never awaited" warnings try: coro.close() - except Exception: - pass + except Exception: # pragma: no cover + # Best-effort cleanup; ignore errors in test shim + pass
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
docs/source/reference/cli.md(3 hunks)docs/source/workflows/mcp/mcp-client.md(4 hunks)docs/source/workflows/mcp/mcp-server.md(6 hunks)src/nat/cli/commands/info/info.py(1 hunks)src/nat/cli/commands/info/list_mcp.py(0 hunks)src/nat/cli/commands/mcp/mcp.py(1 hunks)src/nat/cli/entrypoint.py(2 hunks)tests/nat/cli/commands/info/test_list_mcp.py(0 hunks)tests/nat/cli/commands/mcp/test_mcp_cli.py(1 hunks)
💤 Files with no reviewable changes (2)
- tests/nat/cli/commands/info/test_list_mcp.py
- src/nat/cli/commands/info/list_mcp.py
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/source/workflows/mcp/mcp-server.md
- src/nat/cli/entrypoint.py
- docs/source/reference/cli.md
🧰 Additional context used
📓 Path-based instructions (4)
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
src/nat/cli/commands/info/info.pytests/nat/cli/commands/mcp/test_mcp_cli.pysrc/nat/cli/commands/mcp/mcp.pydocs/source/workflows/mcp/mcp-client.md
src/nat/**/*
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/cli/commands/info/info.pysrc/nat/cli/commands/mcp/mcp.py
tests/**/*.py
⚙️ CodeRabbit configuration file
tests/**/*.py: - Ensure that tests are comprehensive, cover edge cases, and validate the functionality of the code. - Test functions should be named using thetest_prefix, using snake_case. - Any frequently repeated code should be extracted into pytest fixtures. - Pytest fixtures should define the name argument when applying the pytest.fixture decorator. The fixture
function being decorated should be named using thefixture_prefix, using snake_case. Example:
@pytest.fixture(name="my_fixture")
def fixture_my_fixture():
pass
Files:
tests/nat/cli/commands/mcp/test_mcp_cli.py
docs/source/**/*
⚙️ CodeRabbit configuration file
This directory contains the source code for the documentation. All documentation should be written in Markdown format. Any image files should be placed in the
docs/source/_staticdirectory.
Files:
docs/source/workflows/mcp/mcp-client.md
🧠 Learnings (1)
📚 Learning: 2025-08-22T20:18:28.041Z
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/cursor-rules.mdc:0-0
Timestamp: 2025-08-22T20:18:28.041Z
Learning: Applies to .cursor/rules/**/*.mdc : Use consistent project terminology in descriptions (e.g., NAT workflows, NAT CLI commands)
Applied to files:
docs/source/workflows/mcp/mcp-client.md
🪛 Ruff (0.13.1)
src/nat/cli/commands/info/info.py
46-47: Avoid specifying long messages outside the exception class
(TRY003)
tests/nat/cli/commands/mcp/test_mcp_cli.py
243-243: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
249-249: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
271-271: Avoid specifying long messages outside the exception class
(TRY003)
280-280: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
286-286: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
439-439: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
464-464: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
467-467: Unused function argument: args
(ARG001)
467-467: Unused function argument: kwargs
(ARG001)
467-467: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
505-505: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
510-511: try-except-pass detected, consider logging the exception
(S110)
510-510: Do not catch blind exception: Exception
(BLE001)
594-594: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
618-618: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
672-672: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
675-675: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
684-684: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
723-723: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
726-726: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
759-759: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
src/nat/cli/commands/mcp/mcp.py
54-54: Unused function argument: include_traceback
(ARG001)
207-207: Do not catch blind exception: Exception
(BLE001)
289-289: Consider moving this statement to an else block
(TRY300)
290-290: Do not catch blind exception: Exception
(BLE001)
303-303: Unused noqa directive (unused: F821)
Remove unused noqa directive
(RUF100)
339-339: Avoid specifying long messages outside the exception class
(TRY003)
368-368: Do not catch blind exception: Exception
(BLE001)
559-559: Abstract raise to an inner function
(TRY301)
559-559: Avoid specifying long messages outside the exception class
(TRY003)
569-569: Abstract raise to an inner function
(TRY301)
569-569: Avoid specifying long messages outside the exception class
(TRY003)
577-577: Abstract raise to an inner function
(TRY301)
577-577: Avoid specifying long messages outside the exception class
(TRY003)
597-597: Abstract raise to an inner function
(TRY301)
614-614: Unused noqa directive (unused: F821)
Remove unused noqa directive
(RUF100)
683-683: Avoid specifying long messages outside the exception class
(TRY003)
776-776: Do not catch blind exception: Exception
(BLE001)
🪛 markdownlint-cli2 (0.18.1)
docs/source/workflows/mcp/mcp-client.md
221-221: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (6)
src/nat/cli/commands/info/info.py (2)
38-51: Good deprecation shim preserving UX, message is clear.Raising a UsageError that points users to
nat mcp clientis the right call for this breaking change.
50-51: No other references tonat info mcp; only the deprecated shim remains.
Ripgrep across the repo found matches only in src/nat/cli/commands/info/info.py (deprecated command + guidance at ~lines 39–51); no occurrences in docs or tests.src/nat/cli/commands/mcp/mcp.py (2)
40-45: Wiringnat mcp serveto existing start command looks correct.Reusing
start_command.get_command(..., "mcp")keeps behavior consistent.
445-446: Potential asyncio.run() in running loop.If these CLI handlers are ever invoked from a running loop (e.g., notebooks),
asyncio.runwill fail. Consider an executor that detects/bridges a running loop.Would you like a small helper (
run_async) that usesasyncio.get_running_loop()andnest_asynciofallback to avoid runtime errors?docs/source/workflows/mcp/mcp-client.md (1)
176-179: Nice: updated tonat mcp serve.Example matches the new CLI shape.
tests/nat/cli/commands/mcp/test_mcp_cli.py (1)
221-229: Good: verifies MCPError formatting path.Solid unit test coverage for the error surface.
nat mcp serve (replace direct nat mcp) and align CLI structureserve and 'client' for nat mcp
serve and 'client' for nat mcpserve and client for nat mcp
Signed-off-by: Daniel Wang <[email protected]>
Signed-off-by: Daniel Wang <[email protected]>
serve and client for nat mcpserve and client for nat mcp
Signed-off-by: Daniel Wang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (4)
tests/nat/cli/commands/mcp/test_mcp_cli.py (2)
505-514: Replace try/except/pass with contextlib.suppress for cleaner timeout test.Removes S110/BLE001 concerns while keeping behavior.
@@ - async def _raise_timeout(coro, timeout=None, **_kwargs): + async def _raise_timeout(coro, timeout=None, **_kwargs): del timeout, _kwargs - # Dispose the passed coroutine to avoid "never awaited" warnings - try: - coro.close() - except Exception: - pass + # Dispose the passed coroutine to avoid "never awaited" warnings + from contextlib import suppress + with suppress(Exception): + coro.close() # Simulate asyncio.wait_for timing out raise asyncio.TimeoutError
467-469: Silence ARG001 without noqa by naming unused parameters with underscores.Prevents RUF100/ARG001 and keeps intent obvious.
-def _broken_list_tools(*args, **kwargs): +def _broken_list_tools(*_args, **_kwargs): raise RuntimeError("boom")src/nat/cli/commands/mcp/mcp.py (2)
54-56: Fix unused parameter to satisfy Ruff ARG001 in fallback formatter.-def format_mcp_error(error, include_traceback=False): +def format_mcp_error(error, include_traceback: bool = False): + del include_traceback click.echo(f"Error: {error}", err=True)
439-441: Remove no-op subcommand check.This is a command, not a group; the guard never triggers.
- if ctx.invoked_subcommand is not None: - return + # No subcommands under this command.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/source/workflows/mcp/mcp-client.md(5 hunks)src/nat/cli/commands/info/info.py(1 hunks)src/nat/cli/commands/mcp/mcp.py(1 hunks)tests/nat/cli/commands/mcp/test_mcp_cli.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/source/workflows/mcp/mcp-client.md
🧰 Additional context used
📓 Path-based instructions (9)
src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
src/**/*.py: All importable Python code must live under src/
All public APIs in src/ require Python 3.11+ type hints on parameters and return values; prefer typing/collections.abc abstractions; use typing.Annotated when useful
Provide Google-style docstrings for every public module, class, function, and CLI command; first line concise with a period; surround code entities with backticks
Files:
src/nat/cli/commands/info/info.pysrc/nat/cli/commands/mcp/mcp.py
src/nat/**/*
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Core functionality under src/nat should prioritize backward compatibility when changed
Files:
src/nat/cli/commands/info/info.pysrc/nat/cli/commands/mcp/mcp.py
⚙️ CodeRabbit configuration file
This directory contains the core functionality of the toolkit. Changes should prioritize backward compatibility.
Files:
src/nat/cli/commands/info/info.pysrc/nat/cli/commands/mcp/mcp.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.py: Follow PEP 8/20 style; format with yapf (column_limit=120) and use 4-space indentation; end files with a single newline
Run ruff (ruff check --fix) per pyproject.toml; fix warnings unless explicitly ignored; ruff is linter-only
Use snake_case for functions/variables, PascalCase for classes, and UPPER_CASE for constants
Treat pyright warnings as errors during development
Exception handling: preserve stack traces and avoid duplicate logging
When re-raising exceptions, use bareraiseand log with logger.error(), not logger.exception()
When catching and not re-raising, log with logger.exception() to capture stack trace
Validate and sanitize all user input; prefer httpx with SSL verification and follow OWASP Top‑10
Use async/await for I/O-bound work; profile CPU-heavy paths with cProfile/mprof; cache with functools.lru_cache or external cache; leverage NumPy vectorization when beneficial
**/*.py: Programmatic use: create TestLLMConfig(response_seq=[...], delay_ms=...), add with builder.add_llm("", cfg).
When retrieving the test LLM wrapper, use builder.get_llm(name, wrapper_type=LLMFrameworkEnum.) and call the framework’s method (e.g., ainvoke, achat, call).
Files:
src/nat/cli/commands/info/info.pysrc/nat/cli/commands/mcp/mcp.pytests/nat/cli/commands/mcp/test_mcp_cli.py
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{py,sh,md,yml,yaml,toml,ini,json,ipynb,txt,rst}: Every file must start with the standard SPDX Apache-2.0 header; keep copyright years up‑to‑date
All source files must include the SPDX Apache‑2.0 header; do not bypass CI header checks
Files:
src/nat/cli/commands/info/info.pysrc/nat/cli/commands/mcp/mcp.pytests/nat/cli/commands/mcp/test_mcp_cli.py
**/*.{py,md}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Never hard‑code version numbers in code or docs; versions are derived by setuptools‑scm
Files:
src/nat/cli/commands/info/info.pysrc/nat/cli/commands/mcp/mcp.pytests/nat/cli/commands/mcp/test_mcp_cli.py
**/*.{py,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/nat-test-llm.mdc)
**/*.{py,yaml,yml}: Configure response_seq as a list of strings; values cycle per call, and [] yields an empty string.
Configure delay_ms to inject per-call artificial latency in milliseconds for nat_test_llm.
Files:
src/nat/cli/commands/info/info.pysrc/nat/cli/commands/mcp/mcp.pytests/nat/cli/commands/mcp/test_mcp_cli.py
**/*
⚙️ CodeRabbit configuration file
**/*: # Code Review Instructions
- Ensure the code follows best practices and coding standards. - For Python code, follow
PEP 20 and
PEP 8 for style guidelines.- Check for security vulnerabilities and potential issues. - Python methods should use type hints for all parameters and return values.
Example:def my_function(param1: int, param2: str) -> bool: pass- For Python exception handling, ensure proper stack trace preservation:
- When re-raising exceptions: use bare
raisestatements to maintain the original stack trace,
and uselogger.error()(notlogger.exception()) to avoid duplicate stack trace output.- When catching and logging exceptions without re-raising: always use
logger.exception()
to capture the full stack trace information.Documentation Review Instructions - Verify that documentation and comments are clear and comprehensive. - Verify that the documentation doesn't contain any TODOs, FIXMEs or placeholder text like "lorem ipsum". - Verify that the documentation doesn't contain any offensive or outdated terms. - Verify that documentation and comments are free of spelling mistakes, ensure the documentation doesn't contain any
words listed in the
ci/vale/styles/config/vocabularies/nat/reject.txtfile, words that might appear to be
spelling mistakes but are listed in theci/vale/styles/config/vocabularies/nat/accept.txtfile are OK.Misc. - All code (except .mdc files that contain Cursor rules) should be licensed under the Apache License 2.0,
and should contain an Apache License 2.0 header comment at the top of each file.
- Confirm that copyright years are up-to date whenever a file is changed.
Files:
src/nat/cli/commands/info/info.pysrc/nat/cli/commands/mcp/mcp.pytests/nat/cli/commands/mcp/test_mcp_cli.py
tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Unit tests must live under tests/ and use configured markers (e2e, integration, etc.)
Files:
tests/nat/cli/commands/mcp/test_mcp_cli.py
⚙️ CodeRabbit configuration file
tests/**/*.py: - Ensure that tests are comprehensive, cover edge cases, and validate the functionality of the code. - Test functions should be named using thetest_prefix, using snake_case. - Any frequently repeated code should be extracted into pytest fixtures. - Pytest fixtures should define the name argument when applying the pytest.fixture decorator. The fixture
function being decorated should be named using thefixture_prefix, using snake_case. Example:
@pytest.fixture(name="my_fixture")
def fixture_my_fixture():
pass
Files:
tests/nat/cli/commands/mcp/test_mcp_cli.py
**/tests/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/tests/**/*.py: Test functions must use the test_ prefix and snake_case
Extract repeated test code into pytest fixtures; fixtures should set name=... in @pytest.fixture and functions named with fixture_ prefix
Mark expensive tests with @pytest.mark.slow or @pytest.mark.integration
Use pytest with pytest-asyncio for async code; mock external services with pytest_httpserver or unittest.mock
Files:
tests/nat/cli/commands/mcp/test_mcp_cli.py
🧠 Learnings (1)
📚 Learning: 2025-08-28T23:22:41.742Z
Learnt from: CR
PR: NVIDIA/NeMo-Agent-Toolkit#0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-08-28T23:22:41.742Z
Learning: Applies to **/tests/**/*.py : Extract repeated test code into pytest fixtures; fixtures should set name=... in pytest.fixture and functions named with fixture_ prefix
Applied to files:
tests/nat/cli/commands/mcp/test_mcp_cli.py
🧬 Code graph analysis (2)
src/nat/cli/commands/mcp/mcp.py (5)
src/nat/cli/commands/start.py (2)
start_command(255-257)get_command(242-244)packages/nvidia_nat_mcp/src/nat/plugins/mcp/exceptions.py (1)
MCPError(30-43)tests/nat/cli/commands/mcp/test_mcp_cli.py (10)
schema_json(233-235)add_function_group(675-676)add_function_group(726-727)get_accessible_functions(662-663)get_accessible_functions(713-714)initialize(252-253)list_tools(255-263)send_ping(269-271)call_tool(265-267)acall_invoke(656-658)packages/nvidia_nat_mcp/src/nat/plugins/mcp/client_impl.py (1)
MCPServerConfig(42-85)src/nat/data_models/api_server.py (1)
TextContent(92-96)
tests/nat/cli/commands/mcp/test_mcp_cli.py (1)
src/nat/cli/commands/mcp/mcp.py (10)
MCPPingResult(83-95)call_tool_and_print(631-696)call_tool_direct(525-628)format_tool(98-135)list_tools_direct(234-319)mcp_client_tool_call(716-786)mcp_client_tool_list(416-466)ping_mcp_server(322-378)print_tool(138-157)validate_transport_cli_args(58-80)
🪛 Ruff (0.13.1)
src/nat/cli/commands/info/info.py
42-43: Avoid specifying long messages outside the exception class
(TRY003)
src/nat/cli/commands/mcp/mcp.py
54-54: Unused function argument: include_traceback
(ARG001)
216-216: Do not catch blind exception: Exception
(BLE001)
298-298: Consider moving this statement to an else block
(TRY300)
299-299: Do not catch blind exception: Exception
(BLE001)
348-348: Avoid specifying long messages outside the exception class
(TRY003)
377-377: Do not catch blind exception: Exception
(BLE001)
568-568: Abstract raise to an inner function
(TRY301)
568-568: Avoid specifying long messages outside the exception class
(TRY003)
578-578: Abstract raise to an inner function
(TRY301)
578-578: Avoid specifying long messages outside the exception class
(TRY003)
586-586: Abstract raise to an inner function
(TRY301)
586-586: Avoid specifying long messages outside the exception class
(TRY003)
606-606: Abstract raise to an inner function
(TRY301)
692-692: Avoid specifying long messages outside the exception class
(TRY003)
785-785: Do not catch blind exception: Exception
(BLE001)
tests/nat/cli/commands/mcp/test_mcp_cli.py
243-243: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
249-249: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
271-271: Avoid specifying long messages outside the exception class
(TRY003)
280-280: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
286-286: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
439-439: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
464-464: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
467-467: Unused function argument: args
(ARG001)
467-467: Unused function argument: kwargs
(ARG001)
510-511: try-except-pass detected, consider logging the exception
(S110)
510-510: Do not catch blind exception: Exception
(BLE001)
594-594: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
618-618: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
672-672: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
675-675: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
684-684: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
723-723: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
726-726: Unused noqa directive (unused: ARG002)
Remove unused noqa directive
(RUF100)
759-759: Unused noqa directive (unused: BLE001)
Remove unused noqa directive
(RUF100)
🔇 Additional comments (6)
src/nat/cli/commands/info/info.py (1)
34-44: Deduplicate message and add return type to satisfy Ruff TRY003 and typing policy.Move the long message to a constant and annotate the function. This also aligns with earlier feedback.
@@ @click.command( name="mcp", help="Removed. Use 'nat mcp client' instead.", ) -def info_mcp_deprecated(): +DEPRECATED_INFO_MCP_MSG = ( + "The 'nat info mcp' command has been removed. " + "Use the new 'nat mcp client' commands instead" +) +def info_mcp_deprecated() -> None: @@ - raise click.UsageError("The 'nat info mcp' command has been removed. " - "Use the new 'nat mcp client' commands instead") + raise click.UsageError(DEPRECATED_INFO_MCP_MSG)src/nat/cli/commands/mcp/mcp.py (5)
32-37: Add return type to Click group.Matches project typing standards and prior feedback.
-@click.group(name=__name__, invoke_without_command=False, help="MCP-related commands.") -def mcp_command(): +@click.group(name=__name__, invoke_without_command=False, help="MCP-related commands.") +def mcp_command() -> None: @@ - return None + return None
381-387: Annotate nested Click groups with return types.Aligns with public-API typing policy and prior bot suggestion.
-@mcp_command.group(name="client", invoke_without_command=False, help="MCP client commands.") -def mcp_client_command(): +@mcp_command.group(name="client", invoke_without_command=False, help="MCP client commands.") +def mcp_client_command() -> None: @@ -@mcp_client_command.group(name="tool", invoke_without_command=False, help="Inspect and call MCP tools.") -def mcp_client_tool_group(): +@mcp_client_command.group(name="tool", invoke_without_command=False, help="Inspect and call MCP tools.") +def mcp_client_tool_group() -> None: @@ - return None + return NoneAlso applies to: 389-395
234-235: Type‑annotate direct listing API.Meets src/ typing requirements and improves IDE help.
-async def list_tools_direct(command, url, tool_name=None, transport='sse', args=None, env=None): +async def list_tools_direct( + command: str | None, + url: str | None, + tool_name: str | None = None, + transport: str = 'sse', + args: list[str] | None = None, + env: dict[str, str] | None = None, +) -> list[dict[str, str | None]]:
397-467: Type‑annotate CLI handler signature.Public CLI in src/ must be fully typed.
-@click.pass_context -def mcp_client_tool_list(ctx, direct, url, transport, command, args, env, tool, detail, json_output): +@click.pass_context +def mcp_client_tool_list( + ctx: click.Context, + direct: bool, + url: str, + transport: str, + command: str | None, + args: str | None, + env: str | None, + tool: str | None, + detail: bool, + json_output: bool, +) -> None:
40-41: Confirm start_command.get_command(None, "mcp") returns a click.Command (not None).
- No registration for "mcp" found in the repo; calling add_command with None will raise at runtime. Either register the "mcp" subcommand under start_command or check for None and surface a clear error before calling add_command. Location: src/nat/cli/commands/mcp/mcp.py:41.
|
The "Squash and merge" button is too tempting! |
## Description <!-- Note: The pull request title will be included in the CHANGELOG. --> <!-- Provide a standalone description of changes in this PR. --> <!-- Reference any issues closed by this PR with "closes NVIDIA#1234". All PRs should have an issue they close--> Closes AIQ-1885 - **Added**: - New CLI group and subcommands: - `nat mcp serve` wired to server start logic (`src/nat/cli/commands/mcp/mcp.py`) - `nat mcp client` with `ping`, `tool list`, `tool call` (`src/nat/cli/commands/mcp/mcp.py`) - `nat mcp client ping` -> ping the mcp server - `nat mcp client tool list` -> list all tools served by the server - `nat mcp client tool call` -> call a specific tool served by the server - `nat mcp client tool` -> will do nothing more than printing a help message asking for more options! - Entry-point registration for `mcp` (`src/nat/cli/entrypoint.py`) - Test suite for new MCP CLI (`tests/nat/cli/commands/mcp/test_mcp_cli.py`) - **Changed**: - Consolidated/relocated MCP CLI logic into `src/nat/cli/commands/mcp/mcp.py` - CLI reference updated to show nested structure (`nat mcp serve`, `nat mcp client ...`) in `docs/source/reference/cli.md` - **Removed**: - Legacy `nat info mcp` implementation (`src/nat/cli/commands/info/list_mcp.py`) and its tests (`tests/nat/cli/commands/info/test_list_mcp.py`) - **Why it matters**: - Clearer, future-proof CLI surface for MCP: explicit server (`serve`) and client operations; improves UX discoverability and keeps parity with existing start behavior. - **Breaking changes / Migrations**: - `nat mcp` (top-level) → `nat mcp serve` - `nat info mcp` removed; a shim now raises `click.UsageError` instructing users to use `nat mcp client ...` (`src/nat/cli/commands/info/info.py`) - **Docs**: - Updated MCP pages and examples: `docs/source/reference/cli.md`, `docs/source/workflows/mcp/mcp-auth.md`, `docs/source/workflows/mcp/mcp-client.md`, `docs/source/workflows/mcp/mcp-server.md`, `examples/MCP/simple_calculator_mcp/README.md` to use `nat mcp serve` and new client commands. ## By Submitting this PR I confirm: - I am familiar with the [Contributing Guidelines](https://github.com/NVIDIA/NeMo-Agent-Toolkit/blob/develop/docs/source/resources/contributing.md). - We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license. - Any contribution which contains commits that are not Signed-Off will not be accepted. - When the PR is ready for review, new or existing tests cover these changes. - When the PR is ready for review, the documentation is up to date with these changes. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Adds a dedicated top-level MCP CLI: "nat mcp serve" to start servers and "nat mcp client" with subcommands to list, ping, and call tools. * **Documentation** * Updates CLI reference, auth, client, server guides, and example README to use "nat mcp serve" and the new client command names. * **Chores** * Adds Apache-2.0 SPDX license header. * **Refactor** * Deprecates old "nat info mcp" and replaces previous alias with guidance. * **Tests** * Adds comprehensive tests for the new MCP CLI and removes legacy MCP info tests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Daniel Wang <[email protected]> Signed-off-by: Zhongxuan (Daniel) Wang <[email protected]>
Description
Closes AIQ-1885
Added:
nat mcp servewired to server start logic (src/nat/cli/commands/mcp/mcp.py)nat mcp clientwithping,tool list,tool call(src/nat/cli/commands/mcp/mcp.py)nat mcp client ping-> ping the mcp servernat mcp client tool list-> list all tools served by the servernat mcp client tool call-> call a specific tool served by the servernat mcp client tool-> will do nothing more than printing a help message asking for more options!mcp(src/nat/cli/entrypoint.py)tests/nat/cli/commands/mcp/test_mcp_cli.py)Changed:
src/nat/cli/commands/mcp/mcp.pynat mcp serve,nat mcp client ...) indocs/source/reference/cli.mdRemoved:
nat info mcpimplementation (src/nat/cli/commands/info/list_mcp.py) and its tests (tests/nat/cli/commands/info/test_list_mcp.py)Why it matters:
serve) and client operations; improves UX discoverability and keeps parity with existing start behavior.Breaking changes / Migrations:
nat mcp(top-level) →nat mcp servenat info mcpremoved; a shim now raisesclick.UsageErrorinstructing users to usenat mcp client ...(src/nat/cli/commands/info/info.py)Docs:
docs/source/reference/cli.md,docs/source/workflows/mcp/mcp-auth.md,docs/source/workflows/mcp/mcp-client.md,docs/source/workflows/mcp/mcp-server.md,examples/MCP/simple_calculator_mcp/README.mdto usenat mcp serveand new client commands.By Submitting this PR I confirm:
Summary by CodeRabbit
New Features
Documentation
Chores
Refactor
Tests