fix: preserve MCP server extensions and discover Hermes MCP tools - #173
Conversation
Capability planning dropped additive MCP fields (`env`, `args`) from `McpServerPlan`, and the Hermes adapter never called `discover_mcp_tools()` after Hermes 0.18 made discovery an explicit host step. MCP packages looked configured while tools never registered, and author-declared stdio credentials never reached Hermes (which only forwards explicit `mcp_servers.*.env`). * Preserve author MCP extensions through capability planning. * Discover MCP tools in `HermesRuntime.start` and shut them down on stop. * `crates/fabric-core/src/config.rs` * `adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py` - Relates to: none - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. Signed-off-by: Sam Oluwalana <soluwalana@nvidia.com>
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughMCP server plans now preserve additive configuration fields. Hermes discovers MCP tools and clears cached definitions during startup. Hermes shuts down configured MCP servers during cleanup. Tests cover configuration, tool execution, telemetry, lifecycle behavior, and attribution updates. ChangesMCP support
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant E2E_Test
participant HermesAdapter
participant MCPTimeServer
participant MockAPIServer
participant ATOFOutput
E2E_Test->>HermesAdapter: Configure MCP stdio transport
HermesAdapter->>MCPTimeServer: Discover and invoke time tool
HermesAdapter->>MockAPIServer: Request model completion
MockAPIServer-->>HermesAdapter: Return configured tool call
HermesAdapter->>ATOFOutput: Write tool start and end telemetry
ATOFOutput-->>E2E_Test: Provide OK records and timezone
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 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: 2
🤖 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 `@tests/adapters/test_hermes_adapter.py`:
- Around line 424-509: Update
test_runtime_start_discovers_mcp_tools_when_configured to make the mocked
shutdown_mcp_servers raise during runtime.stop(). Assert that AIAgent.close and
SessionDB.close are still called, and verify stop() raises the
hermes_runtime_stop_failed error. Preserve the existing discovery and successful
lifecycle assertions while covering this cleanup failure path.
- Line 467: Replace the direct TEST_API_KEY assignment in this test with the
pytest monkeypatch fixture’s setenv method, ensuring the environment variable is
automatically restored after the test.
🪄 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: 5ff8f5f2-44e4-418b-ab11-5302ca0afa94
📒 Files selected for processing (4)
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pycrates/fabric-core/src/config.rsschemas/run-plan.schema.jsontests/adapters/test_hermes_adapter.py
📜 Review details
⏰ Context from checks skipped due to timeout. (17)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.13, linux-amd64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.11, linux-amd64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Test (Python 3.14, linux-arm64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Pre-commit
⚠️ CI failures not shown inline (4)
GitHub Actions: Rust / Test (arm64): fix: preserve MCP server extensions and discover Hermes MCP tools
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
GitHub Actions: Rust / 1_Test (x86_64).txt: fix: preserve MCP server extensions and discover Hermes MCP tools
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
GitHub Actions: Rust / 0_Test (arm64).txt: fix: preserve MCP server extensions and discover Hermes MCP tools
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
GitHub Actions: Rust / Test (x86_64): fix: preserve MCP server extensions and discover Hermes MCP tools
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
🧰 Additional context used
📓 Path-based instructions (24)
**/*.{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:
schemas/run-plan.schema.jsontests/adapters/test_hermes_adapter.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pycrates/fabric-core/src/config.rs
**/*
📄 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:
schemas/run-plan.schema.jsontests/adapters/test_hermes_adapter.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pycrates/fabric-core/src/config.rs
**/*.{json,jsonschema}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Public contract changes must keep checked-in JSON Schema snapshots synchronized.
Files:
schemas/run-plan.schema.json
schemas/**/*
⚙️ CodeRabbit configuration file
schemas/**/*: Schemas are generated public contract snapshots. Check that schema diffs correspond to intentional Rust type changes and are covered by core tests.
Files:
schemas/run-plan.schema.json
**/*.{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:
tests/adapters/test_hermes_adapter.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pycrates/fabric-core/src/config.rs
**/*.{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:
tests/adapters/test_hermes_adapter.pyadapters/hermes/src/nemo_fabric_adapters/hermes/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:
tests/adapters/test_hermes_adapter.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pycrates/fabric-core/src/config.rs
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_hermes_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:
tests/adapters/test_hermes_adapter.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pycrates/fabric-core/src/config.rs
**/*.{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:
tests/adapters/test_hermes_adapter.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pycrates/fabric-core/src/config.rs
**/*.{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:
tests/adapters/test_hermes_adapter.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pycrates/fabric-core/src/config.rs
**/*.{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:
tests/adapters/test_hermes_adapter.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pycrates/fabric-core/src/config.rs
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_hermes_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_hermes_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_hermes_adapter.py
adapters/**/src/**/*.py
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
adapters/**/src/**/*.py: Use the existing Fabricpythonorprocessrunner and normalized request/result contracts; do not add a runner or abstraction for one adapter.
Consume the complete Fabric invocation when using normalized config or runtime context; treatconfig,capability_plan,telemetry_plan, andruntime_contextas authoritative, reservingharness.settingsfor harness-specific behavior.
Apply configuration precedence in this order: normalized config, Fabric-resolved plans and runtime context, harness-specific settings, then descriptor and adapter defaults. Reject conflicting duplicate declarations or unsupported behavior with actionable errors; never silently drop configuration.
Run dependency and authentication preflight before invoking the harness, and never expose credential values in output, errors, events, logs, or fixtures.
Forward only required system variables, selected credential variables, telemetry variables, and documented harness-specific environment; never forward or log unrelated environment values.
Start one local adapter host per Fabric runtime and keep it alive for orderedstart→invoke*→stopoperations.
Emit exactly one JSON lifecycle response per request on stdout and diagnostics on stderr. Represent harness-level invoke failures as successful lifecycle responses containingresponse: null,failed: true, and structurederrorfields.
Do not emit NeMo Relay stream records on adapter stdout; return exactly one terminal lifecycle response while Relay sends raw records through the SDK-owned out-of-band endpoint.
Scope workspace, generated config, state, sessions, and artifacts to the resolved runtime context, and isolate stateful adapters by Fabric runtime ID.
Keep stdout stable: emitresponseand adapter-specific extensions such aserror, harness events, usage, and session IDs; Fabric supplies top-level identity, lifecycle metadata, status, artifacts, and telemetry.
Claim only capabilities and policies that t...
Files:
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
adapters/**/src/nemo_fabric_adapters/**/*.py
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
For Python adapters, use the source entry-point layout
src/nemo_fabric_adapters/<name>/adapter.pyand keep the package independent and small.
Files:
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public NeMo Fabric contracts.
Files:
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Implement new runtime or binding behavior in the shared Rust core first.
Files:
crates/fabric-core/src/config.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For any Rust change, run
just test-rustandcargo fmt --all -- --check.For Rust core, CLI, or shared runtime semantic changes, run Rust formatting and tests, and add Python tests when behavior is exposed through the SDK.
Use Rust stable tooling; format Rust code with
cargo fmt --all, verify formatting withcargo fmt --all -- --check, and compile withcargo check --workspace --locked.
Files:
crates/fabric-core/src/config.rs
crates/fabric-core/**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes under
crates/fabric-coremust run both the Rust and Python test suites.
Files:
crates/fabric-core/src/config.rs
**/*.{rs,rmeta}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Rust code changes, run
cargo fmt --all -- --checkandjust test-rust.
Files:
crates/fabric-core/src/config.rs
crates/fabric-core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/fabric-corechanges in a way exposed through Python, run both the Rust and Python suites.For Rust API reference changes, update Rust documentation comments under
crates/fabric-core/instead of generated API reference files.
Files:
crates/fabric-core/src/config.rs
crates/fabric-core/src/**/*.rs
⚙️ CodeRabbit configuration file
crates/fabric-core/src/**/*.rs: Review the Rust core for runtime lifecycle correctness, handle validation, capability routing accuracy, schema stability, and error semantics.
Public API changes should match committed schemas, tests, and documentation.
Files:
crates/fabric-core/src/config.rs
🧠 Learnings (1)
📚 Learning: 2026-07-09T22:28:51.689Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 43
File: adapters/claude-sdk/src/nemo_fabric_adapters/claude_sdk/adapter.py:164-168
Timestamp: 2026-07-09T22:28:51.689Z
Learning: In the NeMo-Fabric adapters, treat path values used in Fabric adapter configuration (including logic like `_resolve_path` in adapter.py) as config-root-relative. Do not apply `Path.expanduser()` (or otherwise apply `~`/home or shell-style expansion), because it will make the resolved paths normalize inconsistently across adapters. Also, do not rely on or add any resolution behavior that uses `harness.settings.cwd` as an override point for these adapter paths—`harness.settings.cwd` is explicitly unsupported in this adapter context.
Applied to files:
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
🪛 GitHub Actions: Rust / Test (arm64)
crates/fabric-core/src/config.rs
[error] 2270-2275: cargo fmt formatting check failed. Run 'cargo fmt --all' to format the file.
🪛 GitHub Actions: Rust / Test (x86_64)
crates/fabric-core/src/config.rs
[error] 2270-2273: cargo fmt --all -- --check failed because this code is not formatted according to rustfmt. Run 'cargo fmt --all' to fix it.
🪛 GitHub Check: Test (arm64)
crates/fabric-core/src/config.rs
[warning] 2270-2270:
Diff in /home/runner/work/NeMo-Fabric/NeMo-Fabric/crates/fabric-core/src/config.rs
🪛 GitHub Check: Test (x86_64)
crates/fabric-core/src/config.rs
[warning] 2270-2270:
Diff in /home/runner/work/NeMo-Fabric/NeMo-Fabric/crates/fabric-core/src/config.rs
🪛 Ruff (0.16.1)
tests/adapters/test_hermes_adapter.py
[warning] 444-444: Prefer dict over useless lambda
Replace with lambda with dict
(PIE807)
[warning] 446-446: Unused lambda argument: force
(ARG005)
[warning] 447-447: Unused lambda argument: args
(ARG005)
[warning] 447-447: Unused lambda argument: kwargs
(ARG005)
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
[warning] 513-513: Do not catch blind exception: BaseException
(BLE001)
🔇 Additional comments (6)
crates/fabric-core/src/config.rs (2)
1734-1734: LGTM!Also applies to: 2254-2296
2169-2171: 🗄️ Data Integrity & IntegrationVerify generated-schema and Python binding parity.
McpServerPlannow changes the serialized public run-plan contract. Regenerateschemas/, review the schema diff, and confirm Python models and stubs retain flattened MCP extension fields. Runcargo fmt --all -- --check,just test-rust,cargo check -p fabric-python --locked,just build-python, andjust test-python.Sources: Coding guidelines, Path instructions
schemas/run-plan.schema.json (1)
810-810: LGTM!adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py (2)
480-480: LGTM!
303-311: 🩺 Stability & AvailabilityKeep the current MCP API imports. Hermes 0.17.0, 0.18.0, and 0.19.0 provide all three APIs used here.
> Likely an incorrect or invalid review comment.tests/adapters/test_hermes_adapter.py (1)
382-421: LGTM!
Signed-off-by: Sam Oluwalana <soluwalana@nvidia.com>
There was a problem hiding this comment.
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)
crates/fabric-core/src/config.rs (1)
2254-2294: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winTest the serialized run-plan shape.
This test checks
server.extensionsinside the native Rust value. It does not check the JSON consumed bypython/src/nemo_fabric/client.pyand Hermes. A regression could therefore produce a nestedextensionsobject or omitenvandargswhile this test still passes.Serialize
planand assert the flattened fields undercapability_plan.native.mcp_servers.analyzer. Regenerate the committed schema and run the Rust and Python suites.Suggested assertions
+ let value = serde_json::to_value(&plan).expect("run plan JSON"); + assert_eq!( + value["capability_plan"]["native"]["mcp_servers"]["analyzer"]["env"], + serde_json::json!({"NVIDIA_API_KEY": "${NVIDIA_API_KEY}"}) + ); + assert_eq!( + value["capability_plan"]["native"]["mcp_servers"]["analyzer"]["args"], + serde_json::json!(["--stdio"]) + );As per path instructions, tests must cover the changed API surface and SDK/native parity, and schema-backed Rust changes require generated-schema validation.
🤖 Prompt for 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. In `@crates/fabric-core/src/config.rs` around lines 2254 - 2294, Update mcp_server_extensions_survive_capability_planning to serialize the resolved plan and assert the JSON shape consumed by client.py, verifying analyzer’s flattened env and args fields under capability_plan.native.mcp_servers rather than only server.extensions. Regenerate the committed schema and run the relevant Rust and Python test suites to validate schema-backed API and SDK/native parity.Source: Path instructions
🤖 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 `@tests/adapters/test_hermes_adapter.py`:
- Around line 458-462: Define a test-specific McpShutdownError exception in the
test module and update shutdown_mcp_servers to raise that exception instead of
the generic RuntimeError, preserving the existing failure message and
shutdown_calls behavior.
---
Outside diff comments:
In `@crates/fabric-core/src/config.rs`:
- Around line 2254-2294: Update
mcp_server_extensions_survive_capability_planning to serialize the resolved plan
and assert the JSON shape consumed by client.py, verifying analyzer’s flattened
env and args fields under capability_plan.native.mcp_servers rather than only
server.extensions. Regenerate the committed schema and run the relevant Rust and
Python test suites to validate schema-backed API and SDK/native parity.
🪄 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: f1a6f65c-a92d-405a-9bde-a69d2341cc93
📒 Files selected for processing (2)
crates/fabric-core/src/config.rstests/adapters/test_hermes_adapter.py
📜 Review details
⏰ Context from checks skipped due to timeout. (19)
- GitHub Check: Test (arm64)
- GitHub Check: Test (x86_64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Test (Python 3.14, linux-arm64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.13, linux-amd64)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.11, linux-amd64)
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (19)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Implement new runtime or binding behavior in the shared Rust core first.
Files:
crates/fabric-core/src/config.rs
**/*.{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:
crates/fabric-core/src/config.rstests/adapters/test_hermes_adapter.py
**/*
📄 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:
crates/fabric-core/src/config.rstests/adapters/test_hermes_adapter.py
**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For any Rust change, run
just test-rustandcargo fmt --all -- --check.For Rust core, CLI, or shared runtime semantic changes, run Rust formatting and tests, and add Python tests when behavior is exposed through the SDK.
Use Rust stable tooling; format Rust code with
cargo fmt --all, verify formatting withcargo fmt --all -- --check, and compile withcargo check --workspace --locked.
Files:
crates/fabric-core/src/config.rs
**/*.{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:
crates/fabric-core/src/config.rstests/adapters/test_hermes_adapter.py
crates/fabric-core/**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes under
crates/fabric-coremust run both the Rust and Python test suites.
Files:
crates/fabric-core/src/config.rs
**/*.{rs,rmeta}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Rust code changes, run
cargo fmt --all -- --checkandjust test-rust.
Files:
crates/fabric-core/src/config.rs
crates/fabric-core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/fabric-corechanges in a way exposed through Python, run both the Rust and Python suites.For Rust API reference changes, update Rust documentation comments under
crates/fabric-core/instead of generated API reference files.
Files:
crates/fabric-core/src/config.rs
**/*.{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:
crates/fabric-core/src/config.rstests/adapters/test_hermes_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:
crates/fabric-core/src/config.rstests/adapters/test_hermes_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:
crates/fabric-core/src/config.rstests/adapters/test_hermes_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:
crates/fabric-core/src/config.rstests/adapters/test_hermes_adapter.py
**/*.{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:
crates/fabric-core/src/config.rstests/adapters/test_hermes_adapter.py
crates/fabric-core/src/**/*.rs
⚙️ CodeRabbit configuration file
crates/fabric-core/src/**/*.rs: Review the Rust core for runtime lifecycle correctness, handle validation, capability routing accuracy, schema stability, and error semantics.
Public API changes should match committed schemas, tests, and documentation.
Files:
crates/fabric-core/src/config.rs
**/*.{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:
tests/adapters/test_hermes_adapter.py
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_hermes_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_hermes_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_hermes_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_hermes_adapter.py
🪛 Ruff (0.16.1)
tests/adapters/test_hermes_adapter.py
[warning] 460-460: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (5)
crates/fabric-core/src/config.rs (2)
1734-1734: LGTM!
2169-2171: 🗄️ Data Integrity & IntegrationConfirm the public Rust construction contract.
McpServerPlanis a public Rust struct. Addingextensionsis source-breaking for downstream struct literals that omit the new field unless the type is already#[non_exhaustive]or construction is restricted to a constructor. Confirm the supported API policy and document any intentional breaking change.As per path instructions, public API changes should match committed schemas, tests, and documentation.
Source: Path instructions
tests/adapters/test_hermes_adapter.py (3)
382-421: LGTM!
472-472: LGTM!Also applies to: 701-701
509-517: LGTM!
|
/ok to test 8c0faf2 |
|
Fern docs preview: https://nvidia-preview-pull-request-173.docs.buildwithfern.com/nemo/fabric |
… to hermes-agent v0.19 Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/e2e/test_hermes_e2e.py`:
- Around line 107-117: Update the asynchronous test flow around the
scenario_response request to avoid calling synchronous requests.post directly on
the event loop; either use the test suite’s async HTTP client or execute
requests.post via await asyncio.to_thread, while preserving the existing URL,
JSON payload, timeout, and raise_for_status behavior.
🪄 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: d65f355c-fbc3-4cd3-9d94-08711b1f2357
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pypyproject.tomltests/_utils/mock_api_server.pytests/e2e/test_hermes_e2e.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (24)
**/*.{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:
tests/e2e/test_hermes_e2e.pytests/_utils/mock_api_server.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
**/*
📄 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:
tests/e2e/test_hermes_e2e.pytests/_utils/mock_api_server.pypyproject.tomladapters/hermes/src/nemo_fabric_adapters/hermes/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:
tests/e2e/test_hermes_e2e.pytests/_utils/mock_api_server.pyadapters/hermes/src/nemo_fabric_adapters/hermes/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:
tests/e2e/test_hermes_e2e.pytests/_utils/mock_api_server.pyadapters/hermes/src/nemo_fabric_adapters/hermes/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:
tests/e2e/test_hermes_e2e.pytests/_utils/mock_api_server.pyadapters/hermes/src/nemo_fabric_adapters/hermes/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:
tests/e2e/test_hermes_e2e.pytests/_utils/mock_api_server.pyadapters/hermes/src/nemo_fabric_adapters/hermes/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:
tests/e2e/test_hermes_e2e.pytests/_utils/mock_api_server.pypyproject.tomladapters/hermes/src/nemo_fabric_adapters/hermes/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:
tests/e2e/test_hermes_e2e.pytests/_utils/mock_api_server.pypyproject.tomladapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
**/*.{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:
tests/e2e/test_hermes_e2e.pytests/_utils/mock_api_server.pyadapters/hermes/src/nemo_fabric_adapters/hermes/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/e2e/test_hermes_e2e.pytests/_utils/mock_api_server.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/e2e/test_hermes_e2e.pytests/_utils/mock_api_server.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/e2e/test_hermes_e2e.pytests/_utils/mock_api_server.py
**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For any Rust change, run
just test-rustandcargo fmt --all -- --check.For Rust core, CLI, or shared runtime semantic changes, run Rust formatting and tests, and add Python tests when behavior is exposed through the SDK.
Use Rust stable tooling; format Rust code with
cargo fmt --all, verify formatting withcargo fmt --all -- --check, and compile withcargo check --workspace --locked.
Files:
pyproject.toml
**/{Cargo.toml,Cargo.lock,pyproject.toml,package.json}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For new or updated dependencies, document the functional need, alternatives considered, and why the selected dependency is the narrowest fit.
Files:
pyproject.toml
**/*.{toml,lock}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a manifest or lockfile changes, run the license-diff script against
origin/main, review transitive license changes, and run theattributions-rustandattributions-pythonpre-commit hooks.
Files:
pyproject.toml
**/*.{yml,yaml,toml,lock}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For CI or packaging changes, use
maintain-ciormaintain-packaging, then run recipes and checks whose behavior changed.
Files:
pyproject.toml
{pyproject.toml,adapters/**/pyproject.toml}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
{pyproject.toml,adapters/**/pyproject.toml}: Update the literalproject.versionin the root setuptools project and every adapterpyproject.toml.
Keep internal exact-version requirements aligned: rootnemo-fabric-* == <version>optional dependencies and each adapter'snemo-fabric-adapters-common == <version>dependency.
Files:
pyproject.toml
**/*.{toml,yaml,yml,sh,bash}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
TOML, YAML, and shell files must use the specified
#SPDX copyright and Apache-2.0 license headers.
Files:
pyproject.toml
pyproject.toml
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
pyproject.toml: Keepnemo-fabric-runtimeas an exact-version unconditional dependency of the rootnemo-fabricmetapackage, and give each installable adapter a canonical root extra delegating to the matching leaf harness package.
Install leaf adapters through theirharnessextras in the rootadapter-testsdependency group, with other test-only packages in a non-published group.Keep
nemo-fabricas a metapackage that unconditionally installs the exact-versionnemo-fabric-runtimedistribution; root harness extras must delegate to version-matched leaf adapterharnessextras, without root adapter-only aliases.
Files:
pyproject.toml
**/{Cargo.toml,pyproject.toml}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
**/{Cargo.toml,pyproject.toml}: Keep package names, dependency declarations, import paths, module names, and workspace or Python package metadata internally consistent.
Prefer the standard library, an existing dependency, or a small local implementation before adding a new direct dependency.
When multiple dependencies satisfy the requirement, prefer a maintained OSS option with clear SPDX metadata, a smaller transitive graph, and permissive licensing such as Apache-2.0, MIT, BSD, or ISC.
Files:
pyproject.toml
**/{Cargo.toml,pyproject.toml,Cargo.lock,uv.lock}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
**/{Cargo.toml,pyproject.toml,Cargo.lock,uv.lock}: For new dependencies, record the functional need, viable alternatives considered, why the selected dependency is the narrowest fit, and any unresolved licensing question.
After updating manifests or lockfiles, runuv run --no-project python scripts/licensing/license_diff.py --base-ref origin/mainand review added packages and license changes.
Keep workspace, Python package, and lockfile versions aligned wherever the packaging contract requires alignment.
Files:
pyproject.toml
adapters/**/src/**/*.py
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
adapters/**/src/**/*.py: Use the existing Fabricpythonorprocessrunner and normalized request/result contracts; do not add a runner or abstraction for one adapter.
Consume the complete Fabric invocation when using normalized config or runtime context; treatconfig,capability_plan,telemetry_plan, andruntime_contextas authoritative, reservingharness.settingsfor harness-specific behavior.
Apply configuration precedence in this order: normalized config, Fabric-resolved plans and runtime context, harness-specific settings, then descriptor and adapter defaults. Reject conflicting duplicate declarations or unsupported behavior with actionable errors; never silently drop configuration.
Run dependency and authentication preflight before invoking the harness, and never expose credential values in output, errors, events, logs, or fixtures.
Forward only required system variables, selected credential variables, telemetry variables, and documented harness-specific environment; never forward or log unrelated environment values.
Start one local adapter host per Fabric runtime and keep it alive for orderedstart→invoke*→stopoperations.
Emit exactly one JSON lifecycle response per request on stdout and diagnostics on stderr. Represent harness-level invoke failures as successful lifecycle responses containingresponse: null,failed: true, and structurederrorfields.
Do not emit NeMo Relay stream records on adapter stdout; return exactly one terminal lifecycle response while Relay sends raw records through the SDK-owned out-of-band endpoint.
Scope workspace, generated config, state, sessions, and artifacts to the resolved runtime context, and isolate stateful adapters by Fabric runtime ID.
Keep stdout stable: emitresponseand adapter-specific extensions such aserror, harness events, usage, and session IDs; Fabric supplies top-level identity, lifecycle metadata, status, artifacts, and telemetry.
Claim only capabilities and policies that t...
Files:
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
adapters/**/src/nemo_fabric_adapters/**/*.py
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
For Python adapters, use the source entry-point layout
src/nemo_fabric_adapters/<name>/adapter.pyand keep the package independent and small.
Files:
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public NeMo Fabric contracts.
Files:
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
🧠 Learnings (1)
📚 Learning: 2026-07-09T22:28:51.689Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 43
File: adapters/claude-sdk/src/nemo_fabric_adapters/claude_sdk/adapter.py:164-168
Timestamp: 2026-07-09T22:28:51.689Z
Learning: In the NeMo-Fabric adapters, treat path values used in Fabric adapter configuration (including logic like `_resolve_path` in adapter.py) as config-root-relative. Do not apply `Path.expanduser()` (or otherwise apply `~`/home or shell-style expansion), because it will make the resolved paths normalize inconsistently across adapters. Also, do not rely on or add any resolution behavior that uses `harness.settings.cwd` as an override point for these adapter paths—`harness.settings.cwd` is explicitly unsupported in this adapter context.
Applied to files:
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
🪛 ast-grep (0.45.0)
tests/e2e/test_hermes_e2e.py
[warning] 106-115: Request-controlled URL passed to requests; validate against an allowlist to prevent SSRF.
Context: requests.post(
f"{api_server}/_scenario",
json={
"tool_call": {
"name": tool_name,
"arguments": {"timezone": "America/Los_Angeles"},
}
},
timeout=5,
)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(ssrf-requests)
tests/_utils/mock_api_server.py
[info] 279-279: use jsonify instead of json.dumps for JSON output
Context: json.dumps(chunk)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 289-289: use jsonify instead of json.dumps for JSON output
Context: json.dumps(tool_call["arguments"])
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.16.1)
tests/e2e/test_hermes_e2e.py
[warning] 107-107: Async functions should not call blocking HTTP methods
(ASYNC210)
🔇 Additional comments (5)
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py (2)
303-312: 📐 Maintainability & Code QualityRun the required Python validation.
Run focused MCP discovery and lifecycle tests first. Then run
just test-python.As per coding guidelines, Python adapter changes require focused pytest tests followed by
just test-python.Sources: Coding guidelines, Path instructions
481-481: LGTM!Also applies to: 509-515
tests/_utils/mock_api_server.py (1)
33-34: LGTM!Also applies to: 61-71, 88-97, 211-292
pyproject.toml (1)
106-106: 📐 Maintainability & Code QualityValidate the new test dependency.
Record the functional need, alternatives considered, and why
mcp-server-timeis the narrowest fit. Runuv run --no-project python scripts/licensing/license_diff.py --base-ref origin/main, review license changes, and run theattributions-rustandattributions-pythonhooks.As per coding guidelines, new dependencies require rationale and license review.
Source: Coding guidelines
tests/e2e/test_hermes_e2e.py (1)
14-26: LGTM!
Signed-off-by: David Gardner <dagardner@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@ATTRIBUTIONS-Python.md`:
- Around line 5388-5394: Update both affected “### Licenses” sections in
ATTRIBUTIONS-Python.md by adding blank lines after each heading and immediately
before each fenced block, and change both fence markers to specify the text
language. Preserve the existing fenced content unchanged.
🪄 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: e61730b2-8807-4ff5-a94e-96c9bd2e6cd8
📒 Files selected for processing (1)
ATTRIBUTIONS-Python.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{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:
ATTRIBUTIONS-Python.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:
ATTRIBUTIONS-Python.md
**/*.{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:
ATTRIBUTIONS-Python.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:
ATTRIBUTIONS-Python.md
**/ATTRIBUTIONS-*.md
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Regenerate and include changed
ATTRIBUTIONS-*.mdfiles.
Files:
ATTRIBUTIONS-Python.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:
ATTRIBUTIONS-Python.md
**/*.{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:
ATTRIBUTIONS-Python.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:
ATTRIBUTIONS-Python.md
**/*.{html,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
HTML and Markdown files must use the specified SPDX HTML-comment header.
Files:
ATTRIBUTIONS-Python.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:
ATTRIBUTIONS-Python.md
🪛 markdownlint-cli2 (0.23.2)
ATTRIBUTIONS-Python.md
[warning] 5388-5388: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 5392-5392: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 5392-5392: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 10960-10960: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 10964-10964: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 10964-10964: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
ATTRIBUTIONS-Python.md (1)
3704-3704: LGTM!Also applies to: 5478-5478
Signed-off-by: David Gardner <dagardner@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/e2e/test_hermes_e2e.py`:
- Around line 167-168: Update the disabled-MCP branch in the relevant E2E test
to assert against a new all_tool_records collection filtered by
record["category"] == "tool", rather than the name-specific tool_records list.
Preserve tool_records for enabled assertions and verify that no tool telemetry
of any name is present when MCP is disabled.
🪄 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: 9651933f-82bf-42be-a07b-a771c471c91e
📒 Files selected for processing (1)
tests/e2e/test_hermes_e2e.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{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:
tests/e2e/test_hermes_e2e.py
**/*
📄 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:
tests/e2e/test_hermes_e2e.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:
tests/e2e/test_hermes_e2e.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:
tests/e2e/test_hermes_e2e.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:
tests/e2e/test_hermes_e2e.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:
tests/e2e/test_hermes_e2e.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:
tests/e2e/test_hermes_e2e.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:
tests/e2e/test_hermes_e2e.py
**/*.{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:
tests/e2e/test_hermes_e2e.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/e2e/test_hermes_e2e.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/e2e/test_hermes_e2e.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/e2e/test_hermes_e2e.py
🪛 Ruff (0.16.1)
tests/e2e/test_hermes_e2e.py
[warning] 105-105: Boolean-typed positional argument in function definition
(FBT001)
[warning] 171-171: Async functions should not use pathlib.Path methods, use trio.Path or anyio.path
(ASYNC240)
🔇 Additional comments (3)
tests/e2e/test_hermes_e2e.py (3)
14-26: LGTM!
101-105: 📐 Maintainability & Code QualityRun the required Python validation before merge.
Run the focused test first:
pytest -q tests/e2e/test_hermes_e2e.py -k test_mcp_stdio_transportThen run
just test-python. Formattests/e2e/test_hermes_e2e.pybefore the final test pass. The validation results are not included here.As per coding guidelines, Python changes require
just test-python, and changed files must be formatted before the final test pass.Source: Coding guidelines
131-133: LGTM!
Salonijain27
left a comment
Salonijain27
left a comment
There was a problem hiding this comment.
Approved from a dependency point of view
Signed-off-by: David Gardner <dagardner@nvidia.com>
…-assumptions Signed-off-by: David Gardner <dagardner@nvidia.com>
|
/ok to test 7804bf5 |
Signed-off-by: David Gardner <dagardner@nvidia.com>
|
/merge |
4 participants
Overview
Capability planning dropped additive MCP fields (
env,args) fromMcpServerPlan, and the Hermes adapter never calleddiscover_mcp_tools()after Hermes 0.18 made discovery an explicit host step. MCP packages looked configured while tools never registered, and author-declared stdio credentials never reached Hermes (which only forwards explicitmcp_servers.*.env).HermesRuntime.startand shut them down on stop.Where should the reviewer start?
crates/fabric-core/src/config.rsadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pyRelated Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Relates to: none
Closes FABRIC-167
I confirm this contribution is my own work, or I have the right to submit it under this project's license.
I searched existing issues and open pull requests, and this does not duplicate existing work.
Summary by CodeRabbit