fix: preserve NAT stdio MCP arguments and environment - #178
Conversation
WalkthroughThis change adds a NeMo Fabric-to-NAT adapter. It translates Fabric configuration, manages persistent NAT runtime resources, supports MCP transports and tool policies, and validates lifecycle, invocation, cleanup, serialization, and failure handling. ChangesNAT adapter
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant FabricLifecycle
participant NatRuntime
participant NATWorkflowBuilder
participant NATSessionManager
FabricLifecycle->>NatRuntime: start lifecycle payload
NatRuntime->>NATWorkflowBuilder: build workflow from typed NAT configuration
NatRuntime->>NATSessionManager: create persistent session manager
FabricLifecycle->>NatRuntime: invoke request and context
NatRuntime->>NATSessionManager: create managed session
NatRuntime->>NATWorkflowBuilder: execute workflow with run context
NATWorkflowBuilder-->>NatRuntime: workflow result
NatRuntime-->>FabricLifecycle: JSON-compatible success or structured error
FabricLifecycle->>NatRuntime: shutdown
NatRuntime->>NATSessionManager: close runtime resources
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@external/nat/README.md`:
- Around line 49-51: Update the README’s NeMo Fabric-to-NAT mapping
documentation near the routed capability_plan.native.mcp_servers description to
state that structured stdio args are appended after arguments parsed from the
url, and that env entries are forwarded to NAT’s stdio mapping. Also document
that $VAR expansion applies only to url, not to args or env values.
In `@external/nat/src/nemo_fabric_adapters/nat/adapter.py`:
- Around line 346-347: Update the env handling in the nat_mcp_server_config
translation to validate that env is a string-to-string mapping, report invalid
input through nat_invalid_mcp_server, and deep-copy the validated mapping using
the module’s existing _mapping helper or equivalent. Add a rejection test
covering a non-mapping env value and preserve the existing behavior for valid
mappings.
In `@tests/adapters/test_external_nat_adapter.py`:
- Around line 710-716: Extend the adapter tests with focused coverage for the
subprocess entry point and credential-free doctor flow: patch lifecycle.serve,
call adapter.main(), and assert it receives NatRuntime, while preserving the
__main__ guard behavior; add a doctor assertion using the staged descriptor
alongside the existing plan coverage, and keep the credential-dependent run path
opt-in.
- Around line 959-986: Extend
test_mcp_stdio_expands_command_and_maps_structured_args_and_env to cover a bare
stdio command with no structured args, preferably by parametrizing the test.
Assert that the result contains the transport and command but omits the args key
entirely, preserving the existing merged-arguments case and its expected
mapping.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 7856b94c-0701-43fd-9915-1fb5ad8fc251
📒 Files selected for processing (9)
external/README.mdexternal/nat/README.mdexternal/nat/examples/calculator.pyexternal/nat/examples/calculator_mcp.pyexternal/nat/examples/email_phishing.pyexternal/nat/fabric-adapter.jsonexternal/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.pytests/adapters/test_external_nat_adapter.py
📜 Review details
⏰ Context from checks skipped due to timeout. (18)
- GitHub Check: Pre-commit
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.13, linux-amd64)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Test (Python 3.14, linux-arm64)
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Test (Python 3.11, linux-amd64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Test (x86_64)
🧰 Additional context used
📓 Path-based instructions (22)
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Update documentation and examples in the same branch as the public API change.
Files:
external/README.mdexternal/nat/README.md
**/*
📄 CodeRabbit inference engine (.agents/skills/karpathy-guidelines/SKILL.md)
**/*: Before implementing, explicitly state assumptions, surface ambiguity and tradeoffs, present multiple interpretations when relevant, and ask for clarification rather than silently deciding or proceeding when requirements are unclear.
Prefer the minimum code needed to solve the requested problem: avoid speculative features, unnecessary abstractions, unrequested flexibility, and handling of impossible scenarios; simplify overcomplicated solutions.
When editing existing code, make surgical changes only: do not modify unrelated code, comments, formatting, or pre-existing dead code; match the existing style, and remove only unused imports, variables, or functions introduced by your changes.
Define verifiable success criteria for each task, such as writing regression tests for bugs and invalid-input tests for validation, then verify the implementation against those criteria. For multi-step work, state a brief plan with a verification check for each step.
**/*: Always spellNVIDIAin all caps; do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun, because the name begins with an “en” sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company; use trademark symbols with product names only when required by the document type or legal guidance.
Verify official capitalization, spacing, hyphenation, and spelling for NVIDIA and third-party product names; do not rewrite official product names for grammar or title-case rules.
Precede NVIDIA product names withNVIDIAon first mention when natural and accurate, and link the first mention when the destination helps the reader.
On first use, include the company name and full model qualifier when it helps identify the model; preserve official capitalization and punctuation, and use shorter family names only after establishing the full name.
For learning-oriented and developer content, do not force trademark symbols unless explicitly required; for press, ...
Files:
external/README.mdexternal/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/examples/calculator_mcp.pyexternal/nat/README.mdexternal/nat/examples/email_phishing.pyexternal/nat/fabric-adapter.jsonexternal/nat/examples/calculator.pytests/adapters/test_external_nat_adapter.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.py
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,mdx,rst}: For NeMo Fabric documentation, verify technical claims against the current repository, public API, or documented command before reviewing style.
Always spellNVIDIAin all caps; do not useNvidia,nvidia, orNV.
Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text; avoid raw URLs and weak anchors such ashereorread more.
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative, parallel steps; split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English while preserving necessary technical precision.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce, and preferrefer tooverseewhen directing readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical documentation.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values; use numerals for 10 or greater and commas in thousands.
Do not add trademark symbols to learning-oriented documentation unless the source, platform, or legal guidance explicitly requires them.
Do not replace precise technical terms with simpler words when doing so would lose precision.
Do not flag passive voice when the actor is unknown or the action is the important part.
Do not rewrite API names, package names, command flags, or code literals for style.
**/*.{md,mdx,rst}: Use consistent title case for technical-document headings and table headers; avoid quotation marks, ampersands, and exclamation marks in headings, while preserving official product, event, research, and whitepaper title ...
Files:
external/README.mdexternal/nat/README.md
**/*.{md,rst,txt,adoc}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-language-mechanics.md)
**/*.{md,rst,txt,adoc}: For technical documentation, use professional, active, conversational, engaging, precise, and plain-English prose. Prefer active voice, present tense, short sentences, and scannable paragraphs. Avoid casual or imprecise language, swearing, threats, insults, jokes, puns, culture-specific idioms, marketing exaggeration, and unsupported third-party comparisons.
Usecanfor possibility and reservemayfor permission; useafterfor temporal order; userefer tofor cross-references; prefer short direct sentences and specific verbs; avoid unnecessarypleasein technical documentation.
Prefer active voice when the actor matters. Passive voice is acceptable when the actor is unknown or irrelevant, when the action or result is the focus, or in programmer documentation.
Use natural contractions in conversational technical prose, but do not force them in formal legal copy, API references, or generated text.
Prefer simpler English over Latinisms: usefor exampleorsuch asinstead ofe.g.,and so oninstead ofetc.,that isinstead ofi.e.,compared toinstead ofvs., andby,through, orusinginstead ofvia. Use industry-standard terms such as in silico, in vitro, and in vivo when appropriate, and italicize them in running text.
Usethatwithout commas for essential clauses, andwhichwith commas for nonessential clauses.
Format dates and times clearly: spell out months in body text; use forms such asJune 12, 2025; avoid numeric or ordinal dates; capitalize days; use 12-hour time when appropriate; include a space beforea.m.orp.m.; useETandPTfor needed time zones; avoid24/7; and preferfrom 12:30 to 1:00 p.m.for prose ranges.
Format numbers consistently: spell out zero through nine in body text, use numerals for 10 or greater and for technical values, use commas in thousands, do not begin a sentence with a numeral, spell out ordinals, and use numerals consistently within a category wh...
Files:
external/README.mdexternal/nat/README.md
**/*.{md,mdx,yml,py,rs,sh}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
Keep documentation aligned with current NeMo Fabric behavior, repository layout, entry points, commands, package names, APIs, bindings, and support claims.
Files:
external/README.mdexternal/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/examples/calculator_mcp.pyexternal/nat/README.mdexternal/nat/examples/email_phishing.pyexternal/nat/examples/calculator.pytests/adapters/test_external_nat_adapter.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.py
**/*.{md,mdx}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
**/*.{md,mdx}: Use the full product nameNVIDIA NeMo Fabricon first use, typically in the title and H1; useNeMo Fabricthereafter. Usefabricalone only for the CLI tool and surround it with backticks.
Treat incorrect or stale commands, package names, paths, APIs, support claims, procedures, examples, terminology, or public behavior documentation as blocking issues.
CapitalizeNVIDIAcorrectly and format code, commands, paths, and filenames as inline code where needed.
Use title case for technical-documentation headings.
Introduce code blocks, tables, and lists with complete lead-in sentences; ensure examples match current APIs and build commands.
Use descriptive anchor text, avoid raw URLs and generic labels such ashere, and use repository-relative.mdxpaths for links withindocs/.
Prefer active voice, present tense, short sentences, plain English, consistent terminology, and imperative, parallel, scannable procedures.
Useafterinstead ofoncewhen expressing temporal sequence, and usecanrather thanmaywhen describing possibility rather than permission.
Avoid ambiguous numeric dates and ordinal dates in body text.
For learning-oriented documentation, do not force trademark symbols unless the source document explicitly requires them.
When reporting documentation-review findings, lead withMust fix,Should fix, andNice to havecategories; include file path, line reference, current problem, rationale, and a concrete rewrite or direction.
Files:
external/README.mdexternal/nat/README.md
**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant SDK, API reference, adapter, example, integration, and support documentation when the corresponding public surface changes.
Files:
external/README.mdexternal/nat/README.md
**/*.{html,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
HTML and Markdown files must use the specified SPDX HTML-comment header.
Files:
external/README.mdexternal/nat/README.md
{*.md,**/*.md,**/*.mdx,**/*.ipynb}
⚙️ CodeRabbit configuration file
{*.md,**/*.md,**/*.mdx,**/*.ipynb}: Enforce the product name in user-facing prose: use "NVIDIA NeMo Fabric" on first use and "NeMo Fabric" thereafter. Flag standalone capitalized "Fabric" when it refers to the product. Do not flag the lowercasefabricCLI command, package/import/crate names, code identifiers, API symbols, configuration keys, file paths, or unrelated generic uses of the word.
Files:
external/README.mdexternal/nat/README.md
**/*.{rs,py,pyi,json,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Determine and update every affected public surface, including the CLI, PyO3 bindings, Python SDK, type stubs, schemas, and adapter contract, so they remain in parity.
Files:
external/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/examples/calculator_mcp.pyexternal/nat/examples/email_phishing.pyexternal/nat/fabric-adapter.jsonexternal/nat/examples/calculator.pytests/adapters/test_external_nat_adapter.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.py
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.Use
snake_casefor functions and variables; usePascalCasefor Rust types and Python classes.
Files:
external/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/examples/calculator_mcp.pyexternal/nat/examples/email_phishing.pyexternal/nat/examples/calculator.pytests/adapters/test_external_nat_adapter.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Python code or a Python-facing adapter changes, run
just test-python.In Python SDK, adapters, examples, and tests, follow the existing style, use type annotations for public APIs, and keep native binding declarations synchronized with their Rust implementations.
Files:
external/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/examples/calculator_mcp.pyexternal/nat/examples/email_phishing.pyexternal/nat/examples/calculator.pytests/adapters/test_external_nat_adapter.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.py
**/*.{rs,py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.{rs,py,pyi}: If public configuration types change, confirm schema snapshot tests injust test-rustpass and review generated schema diffs.
For schema or public contract changes, run both language suites and review changes underschemas/and generated API references.
Files:
external/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/examples/calculator_mcp.pyexternal/nat/examples/email_phishing.pyexternal/nat/examples/calculator.pytests/adapters/test_external_nat_adapter.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.py
**/*.{py,pyi,rs}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For Python SDK or PyO3 binding changes, use
python-tests, run focused pytest tests first, thenjust test-python; rebuild withjust build-pythonwhen native code or packaging changes.Public contract changes must keep native Python binding declarations synchronized with their Rust implementations.
Files:
external/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/examples/calculator_mcp.pyexternal/nat/examples/email_phishing.pyexternal/nat/examples/calculator.pytests/adapters/test_external_nat_adapter.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.py
**/*.{rs,py,toml}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
When editing version helpers, verify every
nemo-fabric-*workspace package through Cargo metadata and reject a static version inpython/pyproject.toml.
Files:
external/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/examples/calculator_mcp.pyexternal/nat/examples/email_phishing.pyexternal/nat/examples/calculator.pytests/adapters/test_external_nat_adapter.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.py
**/*.{toml,rs,py}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
Avoid blind repository-wide replacement of version-like strings; distinguish package-version references from examples and unrelated dependency versions.
Files:
external/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/examples/calculator_mcp.pyexternal/nat/examples/email_phishing.pyexternal/nat/examples/calculator.pytests/adapters/test_external_nat_adapter.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.py
**/*.{json,jsonschema}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Public contract changes must keep checked-in JSON Schema snapshots synchronized.
Files:
external/nat/fabric-adapter.json
tests/adapters/**/*.py
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
tests/adapters/**/*.py: If an adapter or integration changes, run its focused tests.
For adapter behavior changes, run focused adapter tests undertests/adapters, then runjust test-python.
Files:
tests/adapters/test_external_nat_adapter.py
tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or the relevant area under
tests/.
Files:
tests/adapters/test_external_nat_adapter.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorAsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
If a fixture is needed in multiple test files, define it once inconftest.pyrather than repeating it.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a<fixture_name>_fixturefunction; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its returned value is unused or it returns no value.
Avoid defensive programming in tests; access expected values directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
When adapter installation metadata changes, packaging metadata tests must directly assert that the root project depends unconditionally on the exact-versionnemo-fabric-runtimedistribution.
Packaging metadata tests must verify that each root harness extra delegates to the matching version of the leaf adapter'sharnessextra.
Packaging metadata tests must verify that bare leaf dependencies remain adapter-owned and that the rootadapter-testsdependency group installs each leaf through itsharnessextra.
Packaging metadata tests must verify that every leaf providesfull; only adapters importing NeMo Relay Python APIs providerelay, while adapters using an external Relay executable havefullequal toharness.
Files:
tests/adapters/test_external_nat_adapter.py
tests/adapters/test_*.py
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
tests/adapters/test_*.py: Add focused tests for descriptor shape and exact capabilities, positive normalized-surface mappings, rejection of unsupported values and unenforceable policies, result normalization without secret leakage, one-shot execution, continuation, runtime isolation, and the packaged subprocess entry point.
Provide a credential-free fixture coveringplan,doctor, andrun; keep credential-dependent live-harness tests opt-in while retaining a deterministic CI end-to-end path.
Files:
tests/adapters/test_external_nat_adapter.py
{tests/**,python/tests/**}
⚙️ CodeRabbit configuration file
{tests/**,python/tests/**}: Tests should cover the behavior promised by the changed API surface, including error paths, lifecycle cleanup, and SDK/native parity where relevant.
Files:
tests/adapters/test_external_nat_adapter.py
🧠 Learnings (1)
📚 Learning: 2026-06-29T22:34:52.407Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 27
File: adapters/codex-cli/fabric-adapter.json:13-15
Timestamp: 2026-06-29T22:34:52.407Z
Learning: In NeMo-Fabric adapter manifest files (e.g., `*/fabric-adapter.json`), keep `config.accepts` limited to the top-level Fabric capability sections that `resolve_capability_plan` consumes (such as `models`, `tools`, `mcp`, `skills`, `telemetry`). Do not add adapter-owned `harness.settings` keys to `config.accepts`; `harness.settings` should remain adapter-owned and be passed through unchanged.
Applied to files:
external/nat/fabric-adapter.json
🪛 ast-grep (0.45.0)
external/nat/examples/email_phishing.py
[info] 90-90: use jsonify instead of json.dumps for JSON output
Context: json.dumps(output.to_mapping(), indent=2)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
external/nat/examples/calculator.py
[info] 85-85: use jsonify instead of json.dumps for JSON output
Context: json.dumps(output.to_mapping(), indent=2)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
tests/adapters/test_external_nat_adapter.py
[info] 904-904: use jsonify instead of json.dumps for JSON output
Context: json.dumps(result)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 928-928: use jsonify instead of json.dumps for JSON output
Context: json.dumps(result)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.16.1)
external/nat/examples/calculator_mcp.py
[warning] 39-39: Avoid specifying long messages outside the exception class
(TRY003)
tests/adapters/test_external_nat_adapter.py
[warning] 33-33: Dynamically typed expressions (typing.Any) are disallowed in **settings
(ANN401)
[warning] 180-180: Dynamically typed expressions (typing.Any) are disallowed in input_value
(ANN401)
[warning] 182-182: Dynamically typed expressions (typing.Any) are disallowed in context
(ANN401)
[warning] 183-183: Dynamically typed expressions (typing.Any) are disallowed in raw_request
(ANN401)
external/nat/src/nemo_fabric_adapters/nat/adapter.py
[warning] 59-59: Dynamically typed expressions (typing.Any) are disallowed in **metadata
(ANN401)
[warning] 73-73: Dynamically typed expressions (typing.Any) are disallowed in value
(ANN401)
[warning] 176-176: Too many branches (13 > 12)
(PLR0912)
[warning] 288-288: Dynamically typed expressions (typing.Any) are disallowed in value
(ANN401)
[warning] 305-305: Dynamically typed expressions (typing.Any) are disallowed in server
(ANN401)
[warning] 551-551: Too many branches (14 > 12)
(PLR0912)
[warning] 705-705: Dynamically typed expressions (typing.Any) are disallowed in build_nat_config
(ANN401)
[warning] 733-733: Prefer TypeError exception for invalid type
(TRY004)
[warning] 733-733: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 745-747: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 752-752: Dynamically typed expressions (typing.Any) are disallowed in response
(ANN401)
[warning] 785-785: Do not catch blind exception: Exception
(BLE001)
[warning] 786-789: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
[warning] 865-869: Use a single with statement with multiple contexts instead of nested with statements
(SIM117)
[warning] 873-873: Do not catch blind exception: Exception
(BLE001)
[warning] 874-877: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
[warning] 888-891: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
🔇 Additional comments (15)
external/README.md (1)
1-19: LGTM!external/nat/fabric-adapter.json (1)
83-94: LGTM!external/nat/src/nemo_fabric_adapters/nat/__init__.py (1)
1-4: LGTM!external/nat/src/nemo_fabric_adapters/nat/adapter.py (5)
339-345: LGTM!
176-257: LGTM!
288-302: LGTM!Also applies to: 398-483, 508-684
780-914: LGTM!
705-725: LGTM!external/nat/README.md (1)
16-45: LGTM!Also applies to: 53-71, 73-118
tests/adapters/test_external_nat_adapter.py (2)
41-196: LGTM!
199-957: LGTM!Also applies to: 1015-1023
external/nat/examples/calculator_mcp.py (2)
14-44: LGTM!
8-13: 📐 Maintainability & Code QualityNo change is needed.
The locked MCP Python SDK 1.28.1 exports
FastMCPfrommcp.server.fastmcpand supports the calledtooldecorator form.> Likely an incorrect or invalid review comment.external/nat/examples/calculator.py (1)
27-90: LGTM!external/nat/examples/email_phishing.py (1)
26-95: LGTM!
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
f9f9699 to
3d9f36c
Compare
|
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. |
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
|
/merge |
2 participants
Overview
Fix NAT stdio MCP configuration so the structured
argsandenvfields in the capability plan are forwarded to NAT instead of being silently dropped. Document the resulting command, argument, and environment mapping in the NAT adapter README.The NAT adapter is now on
mainthrough #175; this draft contains the focused fix, regression coverage, and its adapter documentation.Details
Validation
uv run --no-sync pytest -q tests/adapters/test_external_nat_adapter.py— 52 passed, 2 skipped after rebasing.pre-commit run --files external/nat/README.md.just test-python— full Python suite passed before rebasing; the rebase introduced no code conflicts.uv run --with ruff ruff check ....git diff --check.Where should the reviewer start?
Start with
nat_mcp_server_configinexternal/nat/src/nemo_fabric_adapters/nat/adapter.py; the README directly below the MCP routing section and the regression coverage intests/adapters/test_external_nat_adapter.pyexplain and test the behavior.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
New Features
Reliability
Documentation