feat(tools): add blocked tools policy - #56
Conversation
Signed-off-by: Anuradha Karuppiah <26330987+AnuradhaKaruppiah@users.noreply.github.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:
WalkthroughThis change introduces a typed ChangesTools policy and Relay integration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Config
participant CapabilityPlanner
participant Adapter
participant Runtime
Config->>CapabilityPlanner: tools.blocked
CapabilityPlanner->>Adapter: tools.blocked route
Adapter->>Runtime: native enforcement or unsupported route
Runtime->>Runtime: validate blocked-tools support
🚥 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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (1)
214-229: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTreat an empty
toolsobject as unset
config.toolscan serialize to{}whenToolsConfig.blockedis empty, and this branch currently raises on that valid no-policy state. ReturnNonefor an empty dict before the type check.🤖 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 `@adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py` around lines 214 - 229, Update _allowed_tool_names so an empty config.tools dictionary is treated as unset and returns None before the type validation. Preserve the existing blocked-dictionary handling, list/string normalization, and AdapterConfigError behavior for other invalid shapes.python/src/nemo_fabric/types.py (1)
1143-1153: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep Relay artifact attribution bound to Relay.
With
relay_enabled=Trueandtelemetry_providers=["native", "relay"], this labels a reference withprovider="native"while its URI isrelay_output_dir. Prefer Relay whenever Relay produced the artifact, and add a mixed-provider regression test.Proposed attribution fix
if "relay_enabled" in data: providers = metadata.get("telemetry_providers") - provider = ( - providers[0] if isinstance(providers, list) and providers and isinstance(providers[0], str) else "relay" - ) + provider = "relay" if data["relay_enabled"] is True else ( + providers[0] + if isinstance(providers, list) and providers and isinstance(providers[0], str) + else "relay" + )As per path instructions, Python SDK results must retain parity with the native extension.
🤖 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 `@python/src/nemo_fabric/types.py` around lines 1143 - 1153, Update the provider selection in the trace result construction around metadata.get("telemetry_providers") so relay_enabled=True attributes Relay-produced artifacts to "relay", even when "native" appears first in the provider list; preserve the existing fallback for non-Relay cases and add a mixed-provider regression test covering ["native", "relay"] and relay_output_dir attribution.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 `@adapters/claude/src/nemo_fabric_adapters/claude/adapter.py`:
- Around line 243-249: Update _disallowed_tools to validate
settings.get("disallowed_tools") with _string_list(..., name="disallowed_tools")
before passing it to common_utils.merge_unique with
common_utils.blocked_tools(payload). Preserve the existing merge behavior while
ensuring malformed values raise claude_invalid_configuration instead of being
coerced.
In `@adapters/deepagents/README.md`:
- Around line 134-141: Revise the Native telemetry bullet in the README to add a
grammatical connector between “provider config” and “OpenTelemetry/OpenInference
exporter,” while preserving its meaning that the configured exporter sends spans
directly to the collector without relay artifacts.
In `@adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py`:
- Around line 270-297: Extract the duplicated AgentMiddleware construction used
by allowed_tools_middleware and blocked_tools_middleware into a shared factory
parameterized by the tool-membership predicate and rejection-message behavior.
Update both public middleware factories to supply their respective predicate and
message while preserving their existing async/sync handler delegation and
ToolMessage responses.
In `@examples/code_review_agent/config.py`:
- Around line 269-286: The relay and observability setup in the current
configuration flow contains repetitive runtime type-narrowing assertions that
are only needed for static typing. Simplify the handling around relay,
observability, and their atif/atof fields by using a small typed helper or
appropriate casts, while preserving the existing with_relay-created
configuration behavior and output-directory assignments.
In `@python/src/nemo_fabric/types.py`:
- Around line 378-385: Update the ToolsConfig constructor to reject blocked
values that are str or bytes before iterating, matching from_mapping()
validation. Preserve acceptance of valid string sequences and ensure scalar
inputs cannot be serialized as per-character tool names.
In `@tests/_utils/utils.py`:
- Around line 47-49: Update the telemetry assertion in the result-validation
helper so missing telemetry is handled explicitly rather than silently skipped.
Assert that telemetry is either absent from result or contains relay_enabled set
to False, preserving valid no-provider behavior while rejecting any enabled
relay value.
In `@tests/adapters/test_adapaters_common_hermes.py`:
- Around line 105-116: Merge
test_validate_hermes_telemetry_provider_rejects_native and
test_validate_hermes_telemetry_provider_rejects_mixed_native_and_relay into a
single pytest.mark.parametrize test, parameterizing the differing providers and
relay_enabled payload values while preserving the shared ValueError assertion
and message.
In `@tests/adapters/test_codex_cli.py`:
- Around line 560-561: Update the test around codex_payload and child_env to
assert that FABRIC_UNRELATED_SECRET is absent from child_env, while retaining
the assertion that CODEX_EXPLICIT is forwarded. Ensure the test verifies both
exclusion of ambient secrets and inclusion of explicitly configured environment
variables.
---
Outside diff comments:
In `@adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py`:
- Around line 214-229: Update _allowed_tool_names so an empty config.tools
dictionary is treated as unset and returns None before the type validation.
Preserve the existing blocked-dictionary handling, list/string normalization,
and AdapterConfigError behavior for other invalid shapes.
In `@python/src/nemo_fabric/types.py`:
- Around line 1143-1153: Update the provider selection in the trace result
construction around metadata.get("telemetry_providers") so relay_enabled=True
attributes Relay-produced artifacts to "relay", even when "native" appears first
in the provider list; preserve the existing fallback for non-Relay cases and add
a mixed-provider regression test covering ["native", "relay"] and
relay_output_dir attribution.
🪄 Autofix (Beta)
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: cf4dbfe4-d6af-451b-a728-a870067bc6f9
⛔ Files ignored due to path filters (2)
adapters/deepagents/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (110)
ATTRIBUTIONS-Python.mdREADME.mdadapters/claude/README.mdadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/codex-cli/src/nemo_fabric_adapters/codex_cli/adapter.pyadapters/common/src/nemo_fabric_adapters/common/hermes.pyadapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/deepagents/README.mdadapters/deepagents/pyproject.tomladapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/hermes-cli/README.mdadapters/hermes-cli/src/nemo_fabric_adapters/hermes_cli/adapter.pyadapters/hermes-sdk/README.mdadapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.pycrates/fabric-core/src/config.rscrates/fabric-core/src/lib.rscrates/fabric-core/src/runtime.rsdocs/reference/api/python-library-reference/index.mddocs/reference/api/python-library-reference/nemo_fabric.models.mddocs/reference/api/rust-library-reference/fabric-core/config/enum-capabilitykind.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-capabilitytarget.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatifstorageconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofendpointfieldnamepolicy.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofendpointtransport.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofmode.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayotlptransport.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayunsupportedbehavior.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-telemetryprovider.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-load-adapter-descriptor.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-load-fabric-document.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-effective-config-from-config.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-effective-config-with-profiles.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-effective-config.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-run-plan-from-config.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-run-plan-from-effective-config.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-run-plan-with-profiles.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-run-plan.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-validate-agent-directory.mdxdocs/reference/api/rust-library-reference/fabric-core/config/index.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-fabricconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayatifconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayatofconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayatofendpointconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relaycomponentconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayconfigpolicy.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayobservabilityconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayotlpconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-telemetryconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-telemetryplan.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-telemetryproviderconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/doctor/enum-doctorstatus.mdxdocs/reference/api/rust-library-reference/fabric-core/doctor/fn-doctor-plan.mdxdocs/reference/api/rust-library-reference/fabric-core/doctor/index.mdxdocs/reference/api/rust-library-reference/fabric-core/doctor/struct-doctorcheck.mdxdocs/reference/api/rust-library-reference/fabric-core/doctor/struct-doctorreport.mdxdocs/reference/api/rust-library-reference/fabric-core/error/enum-fabricerror.mdxdocs/reference/api/rust-library-reference/fabric-core/error/index.mdxdocs/reference/api/rust-library-reference/fabric-core/error/type-result.mdxdocs/reference/api/rust-library-reference/fabric-core/fn-version.mdxdocs/reference/api/rust-library-reference/fabric-core/index.mdxdocs/reference/api/rust-library-reference/fabric-core/runtime/index.mdxdocs/reference/api/rust-library-reference/fabric-core/schema/index.mdxdocs/sdk/python.mdxexamples/code_review_agent/config.pyexamples/harbor/README.mdexamples/harbor/demo/task/environment/fabric/configs/codex.yamlexamples/harbor/demo/task/environment/fabric/configs/hermes-relay.yamlexamples/harbor/demo/task/environment/fabric/configs/hermes.yamlexamples/harbor/demo/task/environment/fabric/configs/smoke.yamlpyproject.tomlpython/src/nemo_fabric/__init__.pypython/src/nemo_fabric/models.pypython/src/nemo_fabric/types.pyschemas/adapter-invocation.schema.jsonschemas/agent.schema.jsonschemas/effective-config.schema.jsonschemas/profile.schema.jsonschemas/run-plan.schema.jsontests/_utils/utils.pytests/adapters/test_adapaters_common_hermes.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_cli.pytests/adapters/test_deepagents.pytests/adapters/test_hermes_cli.pytests/adapters/test_hermes_sdk_adapter.pytests/e2e/test_cli.pytests/fixtures/file-config-agent/agent.yamltests/fixtures/file-config-agent/profiles/codex-cli.yamltests/fixtures/file-config-agent/profiles/env-local.yamltests/fixtures/file-config-agent/profiles/env-opensandbox.yamltests/fixtures/file-config-agent/profiles/hermes-cli.yamltests/fixtures/file-config-agent/profiles/hermes-sdk.yamltests/fixtures/file-config-agent/profiles/mcp-github.yamltests/fixtures/file-config-agent/profiles/native-otel.yamltests/fixtures/file-config-agent/profiles/relay-openinference.yamltests/fixtures/file-config-agent/profiles/relay-otel.yamltests/fixtures/file-config-agent/profiles/relay.yamltests/fixtures/hermes-cli-agent/agent.yamltests/fixtures/hermes-cli-agent/profiles/env-local.yamltests/fixtures/hermes-shim-agent/agent.yamltests/fixtures/hermes-shim-agent/profiles/env-local.yamltests/fixtures/hermes-shim-agent/profiles/harbor-swebench-django-13741.yamltests/fixtures/hermes-shim-agent/profiles/mcp-github.yamltests/fixtures/hermes-shim-agent/profiles/swebench-shim.yamltests/integrations/test_harbor_runner.pytests/python/test_code_review_example.pytests/python/test_native_sdk.pytests/python/test_sdk_contract.pytests/python/test_typed_config.py
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Test (x86_64)
- GitHub Check: Pre-commit
- GitHub Check: Test (arm64)
🧰 Additional context used
📓 Path-based instructions (8)
**
⚙️ CodeRabbit configuration file
**:Contributing to NeMo Fabric
Thank you for your interest in contributing to NeMo Fabric. This guide covers
the development workflow, coding standards, and pull request process.Development Setup
This section collects the setup steps needed before building, testing, or
contributing changes.Package Installation
NeMo Fabric is not currently available on PyPI. To consume the Python packages,
build wheels from a source checkout:just wheels uv pip install --find-links dist "nemo-fabric[runtime]"Adapters are distributed as optional extras. For example, install the Hermes
SDK adapter with:uv pip install --find-links dist "nemo-fabric[adapters-hermes-sdk]"Refer to the installation guide for the
complete list of adapters and installation options.Source Development
Install these tools before you start:
- Rust (stable toolchain) -- install with rustup
- Python >= 3.11
- uv -- follow the uv installation guide
- just >= 1.50.0 --
cargo install just --lockedClone the repository, create a virtual environment, and build the Rust and
Python packages:git clone https://github.com/NVIDIA/NeMo-Fabric.git cd NeMo-Fabric uv venv --seed .venv --python 3.13 source .venv/bin/activate uv sync --all-groups --all-extras just no_uv=true build-allVerify the checkout by running the test suites described in
Testing Requirements.Release Tagging
Versioned release tags must use raw Rust-compatible SemVer without a leading
v.
- Use
0.1.0for stable releases.- Use
0.1.0-rc.1for prereleases.- Do not create tags such as
v0.1.0orv0.1.0-rc.1.This keeps release tags aligned with Cargo package versions and lets...
Files:
docs/reference/api/rust-library-reference/fabric-core/fn-version.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-run-plan-from-config.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-validate-agent-directory.mdxdocs/reference/api/rust-library-reference/fabric-core/error/enum-fabricerror.mdxadapters/hermes-sdk/README.mdtests/fixtures/hermes-cli-agent/profiles/env-local.yamldocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-effective-config.mdxdocs/reference/api/rust-library-reference/fabric-core/doctor/enum-doctorstatus.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-capabilitytarget.mdxtests/fixtures/file-config-agent/profiles/env-local.yamldocs/reference/api/rust-library-reference/fabric-core/doctor/index.mdxdocs/reference/api/rust-library-reference/fabric-core/doctor/struct-doctorcheck.mdxdocs/reference/api/rust-library-reference/fabric-core/doctor/struct-doctorreport.mdxdocs/reference/api/rust-library-reference/fabric-core/schema/index.mdxtests/fixtures/file-config-agent/profiles/codex-cli.yamldocs/reference/api/rust-library-reference/fabric-core/runtime/index.mdxadapters/hermes-cli/README.mdadapters/deepagents/pyproject.tomlexamples/harbor/demo/task/environment/fabric/configs/codex.yamldocs/reference/api/rust-library-reference/fabric-core/index.mdxdocs/reference/api/rust-library-reference/fabric-core/error/index.mdxtests/fixtures/hermes-shim-agent/profiles/env-local.yamltests/fixtures/hermes-shim-agent/profiles/harbor-swebench-django-13741.yamltests/fixtures/file-config-agent/profiles/hermes-sdk.yamldocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-run-plan.mdxdocs/reference/api/rust-library-reference/fabric-core/error/type-result.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-load-adapter-descriptor.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-capabilitykind.mdxtests/_utils/utils.pydocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-run-plan-from-effective-config.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-effective-config-from-config.mdxtests/fixtures/file-config-agent/profiles/relay-openinference.yamldocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-effective-config-with-profiles.mdxcrates/fabric-core/src/lib.rstests/fixtures/file-config-agent/profiles/native-otel.yamldocs/reference/api/rust-library-reference/fabric-core/config/struct-relayatofendpointconfig.mdxREADME.mddocs/reference/api/rust-library-reference/fabric-core/doctor/fn-doctor-plan.mdxtests/fixtures/file-config-agent/profiles/env-opensandbox.yamltests/adapters/test_hermes_sdk_adapter.pytests/adapters/test_hermes_cli.pydocs/reference/api/rust-library-reference/fabric-core/config/fn-load-fabric-document.mdxtests/fixtures/file-config-agent/profiles/hermes-cli.yamltests/fixtures/hermes-cli-agent/agent.yamlexamples/harbor/demo/task/environment/fabric/configs/hermes.yamltests/fixtures/hermes-shim-agent/profiles/mcp-github.yamlpyproject.tomldocs/reference/api/rust-library-reference/fabric-core/config/struct-relaycomponentconfig.mdxadapters/claude/README.mdtests/fixtures/file-config-agent/profiles/relay.yamlexamples/harbor/demo/task/environment/fabric/configs/smoke.yamltests/fixtures/file-config-agent/agent.yamldocs/reference/api/rust-library-reference/fabric-core/config/struct-telemetryproviderconfig.mdxATTRIBUTIONS-Python.mddocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofmode.mdxtests/fixtures/file-config-agent/profiles/relay-otel.yamldocs/reference/api/rust-library-reference/fabric-core/config/struct-telemetryplan.mdxadapters/hermes-cli/src/nemo_fabric_adapters/hermes_cli/adapter.pydocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-run-plan-with-profiles.mdxtests/fixtures/hermes-shim-agent/agent.yamldocs/reference/api/rust-library-reference/fabric-core/config/struct-fabricconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-telemetryprovider.mdxschemas/profile.schema.jsontests/fixtures/hermes-shim-agent/profiles/swebench-shim.yamlexamples/harbor/demo/task/environment/fabric/configs/hermes-relay.yamldocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofendpointtransport.mdxtests/e2e/test_cli.pyadapters/deepagents/README.mdexamples/harbor/README.mddocs/reference/api/rust-library-reference/fabric-core/config/enum-relayotlptransport.mdxpython/src/nemo_fabric/__init__.pydocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofendpointfieldnamepolicy.mdxtests/python/test_native_sdk.pydocs/reference/api/rust-library-reference/fabric-core/config/struct-telemetryconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayotlpconfig.mdxadapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.pydocs/sdk/python.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayconfigpolicy.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayatifconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayobservabilityconfig.mdxdocs/reference/api/python-library-reference/index.mddocs/reference/api/rust-library-reference/fabric-core/config/index.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayatofconfig.mdxtests/python/test_code_review_example.pycrates/fabric-core/src/runtime.rsdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatifstorageconfig.mdxtests/adapters/test_claude_adapter.pytests/fixtures/file-config-agent/profiles/mcp-github.yamltests/python/test_typed_config.pyadapters/common/src/nemo_fabric_adapters/common/hermes.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.pytests/adapters/test_adapaters_common_hermes.pydocs/reference/api/rust-library-reference/fabric-core/config/enum-relayunsupportedbehavior.mdxexamples/code_review_agent/config.pytests/adapters/test_deepagents.pydocs/reference/api/python-library-reference/nemo_fabric.models.mdadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyschemas/adapter-invocation.schema.jsonschemas/effective-config.schema.jsonadapters/codex-cli/src/nemo_fabric_adapters/codex_cli/adapter.pytests/integrations/test_harbor_runner.pyschemas/agent.schema.jsonschemas/run-plan.schema.jsontests/python/test_sdk_contract.pytests/adapters/test_codex_cli.pyadapters/common/src/nemo_fabric_adapters/common/utils.pypython/src/nemo_fabric/models.pypython/src/nemo_fabric/types.pycrates/fabric-core/src/config.rs
{docs/**,README.md,AGENTS.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,AGENTS.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency with generated schemas.
Files:
docs/reference/api/rust-library-reference/fabric-core/fn-version.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-run-plan-from-config.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-validate-agent-directory.mdxdocs/reference/api/rust-library-reference/fabric-core/error/enum-fabricerror.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-effective-config.mdxdocs/reference/api/rust-library-reference/fabric-core/doctor/enum-doctorstatus.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-capabilitytarget.mdxdocs/reference/api/rust-library-reference/fabric-core/doctor/index.mdxdocs/reference/api/rust-library-reference/fabric-core/doctor/struct-doctorcheck.mdxdocs/reference/api/rust-library-reference/fabric-core/doctor/struct-doctorreport.mdxdocs/reference/api/rust-library-reference/fabric-core/schema/index.mdxdocs/reference/api/rust-library-reference/fabric-core/runtime/index.mdxdocs/reference/api/rust-library-reference/fabric-core/index.mdxdocs/reference/api/rust-library-reference/fabric-core/error/index.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-run-plan.mdxdocs/reference/api/rust-library-reference/fabric-core/error/type-result.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-load-adapter-descriptor.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-capabilitykind.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-run-plan-from-effective-config.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-effective-config-from-config.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-effective-config-with-profiles.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayatofendpointconfig.mdxREADME.mddocs/reference/api/rust-library-reference/fabric-core/doctor/fn-doctor-plan.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-load-fabric-document.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relaycomponentconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-telemetryproviderconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofmode.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-telemetryplan.mdxdocs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-run-plan-with-profiles.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-fabricconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-telemetryprovider.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofendpointtransport.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayotlptransport.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofendpointfieldnamepolicy.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-telemetryconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayotlpconfig.mdxdocs/sdk/python.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayconfigpolicy.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayatifconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayobservabilityconfig.mdxdocs/reference/api/python-library-reference/index.mddocs/reference/api/rust-library-reference/fabric-core/config/index.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-relayatofconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatifstorageconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayunsupportedbehavior.mdxdocs/reference/api/python-library-reference/nemo_fabric.models.md
{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 Fabric contracts.
Files:
adapters/hermes-sdk/README.mdadapters/hermes-cli/README.mdadapters/deepagents/pyproject.tomlexamples/harbor/demo/task/environment/fabric/configs/codex.yamlexamples/harbor/demo/task/environment/fabric/configs/hermes.yamladapters/claude/README.mdexamples/harbor/demo/task/environment/fabric/configs/smoke.yamladapters/hermes-cli/src/nemo_fabric_adapters/hermes_cli/adapter.pyexamples/harbor/demo/task/environment/fabric/configs/hermes-relay.yamladapters/deepagents/README.mdexamples/harbor/README.mdadapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.pyadapters/common/src/nemo_fabric_adapters/common/hermes.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyexamples/code_review_agent/config.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/codex-cli/src/nemo_fabric_adapters/codex_cli/adapter.pyadapters/common/src/nemo_fabric_adapters/common/utils.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/fixtures/hermes-cli-agent/profiles/env-local.yamltests/fixtures/file-config-agent/profiles/env-local.yamltests/fixtures/file-config-agent/profiles/codex-cli.yamltests/fixtures/hermes-shim-agent/profiles/env-local.yamltests/fixtures/hermes-shim-agent/profiles/harbor-swebench-django-13741.yamltests/fixtures/file-config-agent/profiles/hermes-sdk.yamltests/_utils/utils.pytests/fixtures/file-config-agent/profiles/relay-openinference.yamltests/fixtures/file-config-agent/profiles/native-otel.yamltests/fixtures/file-config-agent/profiles/env-opensandbox.yamltests/adapters/test_hermes_sdk_adapter.pytests/adapters/test_hermes_cli.pytests/fixtures/file-config-agent/profiles/hermes-cli.yamltests/fixtures/hermes-cli-agent/agent.yamltests/fixtures/hermes-shim-agent/profiles/mcp-github.yamltests/fixtures/file-config-agent/profiles/relay.yamltests/fixtures/file-config-agent/agent.yamltests/fixtures/file-config-agent/profiles/relay-otel.yamltests/fixtures/hermes-shim-agent/agent.yamltests/fixtures/hermes-shim-agent/profiles/swebench-shim.yamltests/e2e/test_cli.pytests/python/test_native_sdk.pytests/python/test_code_review_example.pytests/adapters/test_claude_adapter.pytests/fixtures/file-config-agent/profiles/mcp-github.yamltests/python/test_typed_config.pytests/adapters/test_adapaters_common_hermes.pytests/adapters/test_deepagents.pytests/integrations/test_harbor_runner.pytests/python/test_sdk_contract.pytests/adapters/test_codex_cli.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Usepytestto run Python tests.
Do not add@pytest.mark.asyncioto test functions; async tests are detected and run automatically.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, addingspecwhen needed.
Name mocked classes with amockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not duplicate fixtures across test files; if a fixture is needed in multiple test files, define it inconftest.py.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the function as<fixture_name>_fixture() -> <return_type>; only passscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating separate tests for different input types.
If a fixture is needed for a test but does not return a value, or its value is unused, use@pytest.mark.usefixtures.
When modifying environment variables in a test, useos.environ;tests/conftest.pyprovides an autouserestore_environ_fixturethat restores environment variables after each test, somonkeypatch.setenvis unnecessary.
Files:
tests/_utils/utils.pytests/adapters/test_hermes_sdk_adapter.pytests/adapters/test_hermes_cli.pytests/e2e/test_cli.pytests/python/test_native_sdk.pytests/python/test_code_review_example.pytests/adapters/test_claude_adapter.pytests/python/test_typed_config.pytests/adapters/test_adapaters_common_hermes.pytests/adapters/test_deepagents.pytests/integrations/test_harbor_runner.pytests/python/test_sdk_contract.pytests/adapters/test_codex_cli.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/lib.rscrates/fabric-core/src/runtime.rscrates/fabric-core/src/config.rs
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/profile.schema.jsonschemas/adapter-invocation.schema.jsonschemas/effective-config.schema.jsonschemas/agent.schema.jsonschemas/run-plan.schema.json
python/src/nemo_fabric/**/*
⚙️ CodeRabbit configuration file
python/src/nemo_fabric/**/*: Review Python SDK changes for typed API consistency, import-time dependency neutrality, async/session behavior, and parity with the native extension.
Stubs and runtime implementations should stay aligned.
Files:
python/src/nemo_fabric/__init__.pypython/src/nemo_fabric/models.pypython/src/nemo_fabric/types.py
🧠 Learnings (2)
📚 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-cli/src/nemo_fabric_adapters/hermes_cli/adapter.pyadapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/codex-cli/src/nemo_fabric_adapters/codex_cli/adapter.py
📚 Learning: 2026-06-28T04:03:32.877Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 26
File: python/tests/smoke_typed_config.py:163-177
Timestamp: 2026-06-28T04:03:32.877Z
Learning: In NVIDIA NeMo Fabric Python SDK serialization of `RuntimeCapabilities` (to satisfy the “parity contract” with Rust core and the CLI), do not emit metadata keys when the corresponding metadata is absent. Instead, omit those fields entirely so the produced JSON matches the Rust/CLI output (e.g., avoid `null`, empty objects, or placeholder metadata). During review, verify the serializer/builders follow this omission rule and that Python outputs/parity tests reflect the same shape.
Applied to files:
python/src/nemo_fabric/__init__.pypython/src/nemo_fabric/models.pypython/src/nemo_fabric/types.py
🧬 Code graph analysis (6)
adapters/claude/src/nemo_fabric_adapters/claude/adapter.py (1)
tests/adapters/test_claude_adapter.py (1)
build_options(131-131)
examples/code_review_agent/config.py (1)
tests/python/test_code_review_example.py (6)
with_relay_openinference(59-59)with_native_otel(56-56)with_relay_otel(60-60)codex_cli_config(34-34)with_relay(58-58)base_config(31-31)
tests/adapters/test_deepagents.py (1)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (1)
awrap_tool_call(258-261)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (1)
tests/adapters/test_deepagents.py (5)
request(442-443)handler(439-440)_allowed_tool_names(482-482)awrap_tool_call(445-445)run_deepagents(203-203)
adapters/codex-cli/src/nemo_fabric_adapters/codex_cli/adapter.py (2)
tests/integrations/test_harbor_runner.py (2)
write_config_files(292-292)build_command(293-293)tests/adapters/test_codex_cli.py (7)
wait_for_relay_gateway(486-486)write_config_files(159-159)redact_command(518-518)load_thread_id(598-598)build_command(161-161)toml_value(525-525)run_codex(413-413)
tests/integrations/test_harbor_runner.py (1)
python/src/nemo_fabric/integrations/harbor/runner.py (2)
compose_config(27-59)load_config(21-24)
🪛 ast-grep (0.44.1)
tests/python/test_sdk_contract.py
[info] 634-634: use jsonify instead of json.dumps for JSON output
Context: json.dumps(_plan()["effective_config"])
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 644-644: use jsonify instead of json.dumps for JSON output
Context: json.dumps(_plan())
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 648-648: use jsonify instead of json.dumps for JSON output
Context: json.dumps(_runtime())
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 LanguageTool
docs/reference/api/rust-library-reference/fabric-core/config/struct-relayatofendpointconfig.mdx
[style] ~27-~27: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...## headers: BTreeMap<String, String> Endpoint headers. ### timeout_millis: u64 Re...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
docs/reference/api/rust-library-reference/fabric-core/config/struct-relayotlpconfig.mdx
[style] ~31-~31: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...## headers: BTreeMap<String, String> OTLP headers. ### `resource_attributes: BTr...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~35-~35: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ..._attributes: BTreeMap<String, String> OTLP resource attributes. ###service_name...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~39-~39: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ttributes. ### service_name: String OTLP service name. ### `service_namespace: ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~43-~43: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...## service_namespace: Option<String> OTLP service namespace. ### `service_versio...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~47-~47: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... ### service_version: Option<String> OTLP service version. ### `instrumentation_...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~51-~51: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...instrumentation_scope: Option OTLP instrumentation scope. ###timeout_mi...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
docs/reference/api/rust-library-reference/fabric-core/config/struct-relayconfigpolicy.mdx
[style] ~27-~27: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...orted_value: RelayUnsupportedBehavior` Policy for unsupported values. ## Trait Imple...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.22.1)
docs/reference/api/python-library-reference/nemo_fabric.models.md
[warning] 611-611: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 671-671: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 731-731: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 791-791: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 851-851: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 911-911: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 971-971: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1031-1031: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1091-1091: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1151-1151: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1211-1211: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1271-1271: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🪛 Ruff (0.15.20)
adapters/common/src/nemo_fabric_adapters/common/hermes.py
[warning] 45-45: Avoid specifying long messages outside the exception class
(TRY003)
tests/adapters/test_deepagents.py
[warning] 465-465: Missing return type annotation for private function handler
Add return type annotation: str
(ANN202)
[warning] 468-468: Missing return type annotation for private function request
(ANN202)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
[warning] 270-270: Dynamically typed expressions (typing.Any) are disallowed in blocked_tools_middleware
(ANN401)
[warning] 276-276: Dynamically typed expressions (typing.Any) are disallowed in request
(ANN401)
[warning] 276-276: Dynamically typed expressions (typing.Any) are disallowed in _blocked
(ANN401)
[warning] 285-285: Dynamically typed expressions (typing.Any) are disallowed in request
(ANN401)
[warning] 285-285: Dynamically typed expressions (typing.Any) are disallowed in handler
(ANN401)
[warning] 285-285: Dynamically typed expressions (typing.Any) are disallowed in awrap_tool_call
(ANN401)
[warning] 290-290: Dynamically typed expressions (typing.Any) are disallowed in request
(ANN401)
[warning] 290-290: Dynamically typed expressions (typing.Any) are disallowed in handler
(ANN401)
[warning] 290-290: Dynamically typed expressions (typing.Any) are disallowed in wrap_tool_call
(ANN401)
[warning] 494-494: Dynamically typed expressions (typing.Any) are disallowed in subagent
(ANN401)
[warning] 494-494: Dynamically typed expressions (typing.Any) are disallowed in _block_subagent
(ANN401)
[warning] 502-502: Too many statements (53 > 50)
(PLR0915)
adapters/codex-cli/src/nemo_fabric_adapters/codex_cli/adapter.py
[warning] 131-131: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 208-208: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 232-232: Prefer TypeError exception for invalid type
(TRY004)
[warning] 232-232: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 294-294: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 357-357: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 384-384: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 481-481: Avoid specifying long messages outside the exception class
(TRY003)
tests/python/test_sdk_contract.py
[warning] 651-651: Unused method argument: plan_json
(ARG002)
tests/adapters/test_codex_cli.py
[warning] 325-325: Unused function argument: tmp_path
(ARG001)
[error] 560-560: Possible hardcoded password assigned to: "FABRIC_UNRELATED_SECRET"
(S105)
adapters/common/src/nemo_fabric_adapters/common/utils.py
[warning] 156-156: Dynamically typed expressions (typing.Any) are disallowed in *values
(ANN401)
python/src/nemo_fabric/types.py
[warning] 107-107: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 394-394: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 509-509: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 515-515: Remove quotes from type annotation
Remove quotes
(UP037)
[warning] 523-523: Remove quotes from type annotation
Remove quotes
(UP037)
[warning] 534-534: Remove quotes from type annotation
Remove quotes
(UP037)
[warning] 547-547: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 757-757: Remove quotes from type annotation
Remove quotes
(UP037)
Signed-off-by: Anuradha Karuppiah <26330987+AnuradhaKaruppiah@users.noreply.github.com>
31f9c95 to
30bf20c
Compare
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 `@adapters/claude/README.md`:
- Around line 40-42: Update the typed SDK example near the FabricConfig usage to
import and instantiate ToolsConfig with its blocked mapping instead of passing
tools as a list. Ensure the example uses the canonical typed FabricConfig API,
or remove the tools field if no blocked tools are required.
In `@docs/reference/api/python-library-reference/nemo_fabric.models.md`:
- Around line 1424-1425: Add a blank line after the ToolsConfig class heading
before its descriptive text, preserving the existing heading and documentation
content so the section conforms to MD022.
In `@tests/adapters/test_deepagents.py`:
- Around line 658-662: Strengthen the assertions in the test around the
configured main-agent and subagent middleware so they verify blocked-tool
behavior rather than merely non-empty lists. Inspect the blocked-tool
configuration or invoke each middleware gate to confirm write_file is rejected
while read_file remains allowed for both create_kwargs["middleware"] and
subagents[0]["middleware"].
- Around line 465-468: Annotate the local helper functions handler and request
with explicit return types to satisfy Ruff ANN202, using the appropriate type
for each helper’s returned value; leave the surrounding test function
unannotated and do not add -> None to it.
🪄 Autofix (Beta)
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: 68a3fdea-198e-4cfe-a066-f62fd380e1d1
📒 Files selected for processing (30)
README.mdadapters/claude/README.mdadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/common/src/nemo_fabric_adapters/common/hermes.pyadapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/deepagents/README.mdadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/hermes-cli/README.mdadapters/hermes-sdk/README.mdadapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.pycrates/fabric-core/src/config.rsdocs/reference/api/python-library-reference/index.mddocs/reference/api/python-library-reference/nemo_fabric.models.mddocs/reference/api/python-library-reference/nemo_fabric.types.mdexamples/code_review_agent/config.pypython/src/nemo_fabric/__init__.pypython/src/nemo_fabric/models.pypython/src/nemo_fabric/types.pyschemas/adapter-invocation.schema.jsonschemas/agent.schema.jsonschemas/effective-config.schema.jsonschemas/profile.schema.jsonschemas/run-plan.schema.jsontests/_utils/utils.pytests/adapters/test_adapaters_common_hermes.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_cli.pytests/adapters/test_deepagents.pytests/e2e/test_claude.pytests/python/test_sdk_contract.py
💤 Files with no reviewable changes (1)
- docs/reference/api/python-library-reference/nemo_fabric.types.md
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Test (x86_64)
- GitHub Check: Test (arm64)
🧰 Additional context used
📓 Path-based instructions (8)
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Usepytestto run Python tests.
Do not add@pytest.mark.asyncioto test functions; async tests are detected and run automatically.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, addingspecwhen needed.
Name mocked classes with amockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not duplicate fixtures across test files; if a fixture is needed in multiple test files, define it inconftest.py.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the function as<fixture_name>_fixture() -> <return_type>; only passscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating separate tests for different input types.
If a fixture is needed for a test but does not return a value, or its value is unused, use@pytest.mark.usefixtures.
When modifying environment variables in a test, useos.environ;tests/conftest.pyprovides an autouserestore_environ_fixturethat restores environment variables after each test, somonkeypatch.setenvis unnecessary.
Files:
tests/_utils/utils.pytests/e2e/test_claude.pytests/adapters/test_deepagents.pytests/adapters/test_claude_adapter.pytests/adapters/test_adapaters_common_hermes.pytests/adapters/test_codex_cli.pytests/python/test_sdk_contract.py
**
⚙️ CodeRabbit configuration file
**:Contributing to NeMo Fabric
Thank you for your interest in contributing to NeMo Fabric. This guide covers
the development workflow, coding standards, and pull request process.Development Setup
This section collects the setup steps needed before building, testing, or
contributing changes.Package Installation
NeMo Fabric is not currently available on PyPI. To consume the Python packages,
build wheels from a source checkout:just wheels uv pip install --find-links dist "nemo-fabric[runtime]"Adapters are distributed as optional extras. For example, install the Hermes
SDK adapter with:uv pip install --find-links dist "nemo-fabric[adapters-hermes-sdk]"Refer to the installation guide for the
complete list of adapters and installation options.Source Development
Install these tools before you start:
- Rust (stable toolchain) -- install with rustup
- Python >= 3.11
- uv -- follow the uv installation guide
- just >= 1.50.0 --
cargo install just --lockedClone the repository, create a virtual environment, and build the Rust and
Python packages:git clone https://github.com/NVIDIA/NeMo-Fabric.git cd NeMo-Fabric uv venv --seed .venv --python 3.13 source .venv/bin/activate uv sync --all-groups --all-extras just no_uv=true build-allVerify the checkout by running the test suites described in
Testing Requirements.Release Tagging
Versioned release tags must use raw Rust-compatible SemVer without a leading
v.
- Use
0.1.0for stable releases.- Use
0.1.0-rc.1for prereleases.- Do not create tags such as
v0.1.0orv0.1.0-rc.1.This keeps release tags aligned with Cargo package versions and lets...
Files:
tests/_utils/utils.pyadapters/hermes-cli/README.mdadapters/claude/README.mdREADME.mdadapters/hermes-sdk/README.mdpython/src/nemo_fabric/__init__.pyschemas/profile.schema.jsondocs/reference/api/python-library-reference/index.mdadapters/deepagents/README.mdadapters/claude/src/nemo_fabric_adapters/claude/adapter.pytests/e2e/test_claude.pyexamples/code_review_agent/config.pyschemas/agent.schema.jsonadapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.pyschemas/effective-config.schema.jsontests/adapters/test_deepagents.pyschemas/run-plan.schema.jsontests/adapters/test_claude_adapter.pyadapters/common/src/nemo_fabric_adapters/common/hermes.pyschemas/adapter-invocation.schema.jsonpython/src/nemo_fabric/models.pytests/adapters/test_adapaters_common_hermes.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_codex_cli.pypython/src/nemo_fabric/types.pytests/python/test_sdk_contract.pydocs/reference/api/python-library-reference/nemo_fabric.models.mdadapters/common/src/nemo_fabric_adapters/common/utils.pycrates/fabric-core/src/config.rs
{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/_utils/utils.pytests/e2e/test_claude.pytests/adapters/test_deepagents.pytests/adapters/test_claude_adapter.pytests/adapters/test_adapaters_common_hermes.pytests/adapters/test_codex_cli.pytests/python/test_sdk_contract.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 Fabric contracts.
Files:
adapters/hermes-cli/README.mdadapters/claude/README.mdadapters/hermes-sdk/README.mdadapters/deepagents/README.mdadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyexamples/code_review_agent/config.pyadapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.pyadapters/common/src/nemo_fabric_adapters/common/hermes.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/common/src/nemo_fabric_adapters/common/utils.py
{docs/**,README.md,AGENTS.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,AGENTS.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency with generated schemas.
Files:
README.mddocs/reference/api/python-library-reference/index.mddocs/reference/api/python-library-reference/nemo_fabric.models.md
python/src/nemo_fabric/**/*
⚙️ CodeRabbit configuration file
python/src/nemo_fabric/**/*: Review Python SDK changes for typed API consistency, import-time dependency neutrality, async/session behavior, and parity with the native extension.
Stubs and runtime implementations should stay aligned.
Files:
python/src/nemo_fabric/__init__.pypython/src/nemo_fabric/models.pypython/src/nemo_fabric/types.py
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/profile.schema.jsonschemas/agent.schema.jsonschemas/effective-config.schema.jsonschemas/run-plan.schema.jsonschemas/adapter-invocation.schema.json
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 (2)
📚 Learning: 2026-06-28T04:03:32.877Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 26
File: python/tests/smoke_typed_config.py:163-177
Timestamp: 2026-06-28T04:03:32.877Z
Learning: In NVIDIA NeMo Fabric Python SDK serialization of `RuntimeCapabilities` (to satisfy the “parity contract” with Rust core and the CLI), do not emit metadata keys when the corresponding metadata is absent. Instead, omit those fields entirely so the produced JSON matches the Rust/CLI output (e.g., avoid `null`, empty objects, or placeholder metadata). During review, verify the serializer/builders follow this omission rule and that Python outputs/parity tests reflect the same shape.
Applied to files:
python/src/nemo_fabric/__init__.pypython/src/nemo_fabric/models.pypython/src/nemo_fabric/types.py
📚 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/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
🧬 Code graph analysis (6)
tests/adapters/test_deepagents.py (1)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (1)
run_deepagents(491-577)
adapters/common/src/nemo_fabric_adapters/common/hermes.py (1)
adapters/common/src/nemo_fabric_adapters/common/utils.py (1)
settings_payload(103-105)
tests/adapters/test_adapaters_common_hermes.py (1)
adapters/common/src/nemo_fabric_adapters/common/utils.py (1)
relay_enabled(124-125)
python/src/nemo_fabric/types.py (1)
tests/python/test_sdk_contract.py (2)
_ResolvedFabricConfig(474-474)from_mapping(79-79)
tests/python/test_sdk_contract.py (2)
python/src/nemo_fabric/types.py (2)
from_mapping(648-670)to_mapping(868-875)python/src/nemo_fabric/models.py (1)
to_mapping(426-432)
adapters/common/src/nemo_fabric_adapters/common/utils.py (1)
adapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.py (1)
tools_config(46-46)
🪛 markdownlint-cli2 (0.22.1)
docs/reference/api/python-library-reference/nemo_fabric.models.md
[warning] 1424-1424: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🪛 Ruff (0.15.20)
tests/adapters/test_deepagents.py
[warning] 465-465: Missing return type annotation for private function handler
Add return type annotation: str
(ANN202)
[warning] 468-468: Missing return type annotation for private function request
(ANN202)
tests/adapters/test_adapaters_common_hermes.py
[warning] 111-111: Boolean-typed positional argument in function definition
(FBT001)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
[warning] 236-236: Dynamically typed expressions (typing.Any) are disallowed in is_blocked
(ANN401)
[warning] 236-236: Dynamically typed expressions (typing.Any) are disallowed in message
(ANN401)
[warning] 236-236: Dynamically typed expressions (typing.Any) are disallowed in _tool_gate_middleware
(ANN401)
[warning] 240-240: Dynamically typed expressions (typing.Any) are disallowed in request
(ANN401)
[warning] 240-240: Dynamically typed expressions (typing.Any) are disallowed in _blocked
(ANN401)
[warning] 249-249: Dynamically typed expressions (typing.Any) are disallowed in request
(ANN401)
[warning] 249-249: Dynamically typed expressions (typing.Any) are disallowed in handler
(ANN401)
[warning] 249-249: Dynamically typed expressions (typing.Any) are disallowed in awrap_tool_call
(ANN401)
[warning] 254-254: Dynamically typed expressions (typing.Any) are disallowed in request
(ANN401)
[warning] 254-254: Dynamically typed expressions (typing.Any) are disallowed in handler
(ANN401)
[warning] 254-254: Dynamically typed expressions (typing.Any) are disallowed in wrap_tool_call
(ANN401)
[warning] 262-262: Dynamically typed expressions (typing.Any) are disallowed in allowed_tools_middleware
(ANN401)
[warning] 277-277: Dynamically typed expressions (typing.Any) are disallowed in blocked_tools_middleware
(ANN401)
[warning] 482-482: Dynamically typed expressions (typing.Any) are disallowed in subagent
(ANN401)
[warning] 482-482: Dynamically typed expressions (typing.Any) are disallowed in _block_subagent
(ANN401)
python/src/nemo_fabric/types.py
[warning] 387-387: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 398-398: Avoid specifying long messages outside the exception class
(TRY003)
adapters/common/src/nemo_fabric_adapters/common/utils.py
[warning] 156-156: Dynamically typed expressions (typing.Any) are disallowed in *values
(ANN401)
🔇 Additional comments (29)
tests/_utils/utils.py (1)
47-47: LGTM!adapters/hermes-cli/README.md (1)
27-28: LGTM!tests/adapters/test_codex_cli.py (1)
580-580: LGTM!python/src/nemo_fabric/types.py (1)
372-415: LGTM!Also applies to: 610-610, 625-625, 640-640, 691-700, 747-751
tests/python/test_sdk_contract.py (1)
48-50: LGTM!Also applies to: 146-153, 191-213
docs/reference/api/python-library-reference/nemo_fabric.models.md (1)
1308-1347: LGTM!Also applies to: 1393-1418, 1549-1568
README.md (1)
139-151: LGTM!adapters/hermes-sdk/README.md (1)
25-26: LGTM!adapters/common/src/nemo_fabric_adapters/common/hermes.py (1)
48-53: LGTM!Also applies to: 66-78, 156-156
tests/adapters/test_adapaters_common_hermes.py (1)
105-113: LGTM!Also applies to: 146-146, 168-168, 378-378
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (1)
222-283: LGTM!Also applies to: 423-441, 482-487
adapters/common/src/nemo_fabric_adapters/common/utils.py (1)
136-162: LGTM!crates/fabric-core/src/config.rs (1)
55-55: LGTM!Also applies to: 93-102, 418-426, 1593-1679, 1883-1915, 2030-2073, 2526-2566, 2577-2640
adapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.py (1)
181-195: 🔒 Security & PrivacyNo issue here.
tools.blockedis already written into the Hermes config viaadapters/common/src/nemo_fabric_adapters/common/hermes.py, so droppingdisabled_toolsetsfromAIAgent(...)only skips an optional constructor override.> Likely an incorrect or invalid review comment.python/src/nemo_fabric/__init__.py (1)
38-38: LGTM!Also applies to: 79-88, 103-105
adapters/deepagents/README.md (1)
55-57: LGTM!Also applies to: 72-72, 134-141
tests/e2e/test_claude.py (1)
14-20: LGTM!Also applies to: 95-98
examples/code_review_agent/config.py (1)
9-9: LGTM!Also applies to: 18-18, 273-286
schemas/agent.schema.json (1)
915-928: LGTM!Also applies to: 976-986, 1018-1025
schemas/adapter-invocation.schema.json (1)
111-115: LGTM!Also applies to: 425-435, 467-474, 1420-1475, 1502-1528, 1543-1543
python/src/nemo_fabric/models.py (1)
397-417: LGTM!Also applies to: 481-491, 539-540
schemas/profile.schema.json (1)
2-17: LGTM!Also applies to: 67-74, 107-114
docs/reference/api/python-library-reference/index.md (1)
34-49: LGTM!adapters/claude/src/nemo_fabric_adapters/claude/adapter.py (2)
14-30: LGTM!Also applies to: 233-234, 243-249
346-347: 🔒 Security & PrivacyVerify deny-list precedence over
allowed_tools.
build_optionsnow passes user-configurableallowed_toolsalongside blocked names indisallowed_tools. If the Claude SDK permits overlap or applies the allow-list first,tools.blockedcould be bypassed. Reject overlaps, or confirm thatdisallowed_toolsalways wins, and add a regression test.schemas/effective-config.schema.json (1)
131-141: LGTM!Also applies to: 173-180, 349-919, 1001-1021, 1022-1034
schemas/run-plan.schema.json (1)
240-244: LGTM!Also applies to: 554-564, 596-603, 795-1365, 1507-1512, 1527-1535, 1560-1560, 1579-1615, 1653-1653
tests/adapters/test_deepagents.py (1)
19-20: LGTM!Also applies to: 135-136, 281-292, 312-318, 334-334, 494-497
tests/adapters/test_claude_adapter.py (1)
14-22: LGTM!Also applies to: 129-130, 160-168
Signed-off-by: Anuradha Karuppiah <26330987+AnuradhaKaruppiah@users.noreply.github.com> # Conflicts: # adapters/deepagents/README.md # crates/fabric-core/src/config.rs # crates/fabric-core/src/lib.rs # docs/reference/api/python-library-reference/index.md # docs/reference/api/python-library-reference/nemo_fabric.models.md # docs/reference/api/rust-library-reference/fabric-core/config/enum-capabilitykind.mdx # docs/reference/api/rust-library-reference/fabric-core/config/enum-capabilitytarget.mdx # docs/reference/api/rust-library-reference/fabric-core/config/enum-relayatifstorageconfig.mdx # docs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofendpointfieldnamepolicy.mdx # docs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofendpointtransport.mdx # docs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofmode.mdx # docs/reference/api/rust-library-reference/fabric-core/config/enum-relayotlptransport.mdx # docs/reference/api/rust-library-reference/fabric-core/config/enum-relayunsupportedbehavior.mdx # docs/reference/api/rust-library-reference/fabric-core/config/fn-load-adapter-descriptor.mdx # docs/reference/api/rust-library-reference/fabric-core/config/fn-load-fabric-document.mdx # docs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-effective-config-from-config.mdx # docs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-effective-config-with-profiles.mdx # docs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-effective-config.mdx # docs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-run-plan-from-config.mdx # docs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-run-plan-from-effective-config.mdx # docs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-run-plan-with-profiles.mdx # docs/reference/api/rust-library-reference/fabric-core/config/fn-resolve-run-plan.mdx # docs/reference/api/rust-library-reference/fabric-core/config/fn-validate-agent-directory.mdx # docs/reference/api/rust-library-reference/fabric-core/config/index.mdx # docs/reference/api/rust-library-reference/fabric-core/config/struct-relayatifconfig.mdx # docs/reference/api/rust-library-reference/fabric-core/config/struct-relayatofconfig.mdx # docs/reference/api/rust-library-reference/fabric-core/config/struct-relayatofendpointconfig.mdx # docs/reference/api/rust-library-reference/fabric-core/config/struct-relaycomponentconfig.mdx # docs/reference/api/rust-library-reference/fabric-core/config/struct-relayconfig.mdx # docs/reference/api/rust-library-reference/fabric-core/config/struct-relayconfigpolicy.mdx # docs/reference/api/rust-library-reference/fabric-core/config/struct-relayobservabilityconfig.mdx # docs/reference/api/rust-library-reference/fabric-core/config/struct-relayotlpconfig.mdx # docs/reference/api/rust-library-reference/fabric-core/config/struct-telemetryproviderconfig.mdx # docs/reference/api/rust-library-reference/fabric-core/doctor/enum-doctorstatus.mdx # docs/reference/api/rust-library-reference/fabric-core/doctor/fn-doctor-plan.mdx # docs/reference/api/rust-library-reference/fabric-core/doctor/index.mdx # docs/reference/api/rust-library-reference/fabric-core/doctor/struct-doctorcheck.mdx # docs/reference/api/rust-library-reference/fabric-core/doctor/struct-doctorreport.mdx # docs/reference/api/rust-library-reference/fabric-core/error/enum-fabricerror.mdx # docs/reference/api/rust-library-reference/fabric-core/error/index.mdx # docs/reference/api/rust-library-reference/fabric-core/error/type-result.mdx # docs/reference/api/rust-library-reference/fabric-core/fn-version.mdx # docs/reference/api/rust-library-reference/fabric-core/runtime/index.mdx # docs/reference/api/rust-library-reference/fabric-core/schema/index.mdx # examples/code_review_agent/config.py # python/src/nemo_fabric/__init__.py # python/src/nemo_fabric/models.py # python/src/nemo_fabric/types.py # schemas/adapter-invocation.schema.json # schemas/agent.schema.json # schemas/effective-config.schema.json # schemas/run-plan.schema.json # tests/_utils/utils.py # tests/adapters/test_adapaters_common_hermes.py # tests/python/test_sdk_contract.py
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
crates/fabric-core/src/config.rs (1)
1607-1619: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve policy-presence semantics for
tools_configured.
tools: { blocked: [] }provides a tool policy, but this change reportscapability_plan.tools_configured == false. Keep the top-level flag based onconfig.tools.is_some(); target routing may still remain disabled when the deny-list is empty. Otherwise consumers cannot distinguish an omitted policy from an explicitly empty one.As per path instructions,
capability_plan.tools_configuredindicates whether tool config was provided.Also applies to: 1690-1693, 2708-2772
🤖 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 1607 - 1619, Update the tools_configured assignment in the capability-plan construction to use config.tools.is_some(), preserving true for an explicitly provided empty blocked list while remaining false when tools is omitted. Keep the existing blocked_tools-based checks for target routing and native/unsupported handling unchanged, including the related logic in the capability-plan paths around tools_configured.Source: Path instructions
python/src/nemo_fabric/models.py (1)
417-417: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRemove the raw
dictfallback fromtools
FabricBaseModel(extra="allow")already preserves unknownToolsConfigfields, soToolsConfig | dict[str, Any] | Noneonly bypasses validation for known keys likeblockedand can leak malformed configs intoto_mapping(). Make bothtoolsfieldsToolsConfig | None.🤖 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 `@python/src/nemo_fabric/models.py` at line 417, Update both tools field declarations in the relevant model definitions to use ToolsConfig | None, removing the raw dict[str, Any] fallback. Preserve the existing optional behavior and rely on FabricBaseModel extra-field handling for unknown ToolsConfig fields so known fields remain validated before to_mapping().Source: Path instructions
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (1)
408-429: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPropagate blocked-tool middleware to delegated subagents (adapter.py:422-428)
blocked_tools_middleware(blocked)is attached only to the parent agent here, and_block_subagent()rewrites only explicit dict entries. The built-intasksubagent has its own middleware stack, so blocked tools can still be reached through delegation unless that path is gated too.🤖 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 `@adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py` around lines 408 - 429, Update the blocked-tools handling in the adapter configuration flow to propagate the blocked middleware to the built-in task/delegated subagent path, not only the parent middleware and explicit dictionary subagents. Extend or reuse _block_subagent so every delegated subagent receives the same blocked set while preserving existing behavior for parent middleware and non-subagent configuration values.tests/adapters/test_claude_adapter.py (1)
152-159: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCover the rest of the options contract here (
tests/adapters/test_claude_adapter.py:152) — assertoptions.allowed_tools == ["Read"], and add a duplicate inblockedif this test should also pin the order-preserving dedup behavior ofdisallowed_tools.🤖 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 `@tests/adapters/test_claude_adapter.py` around lines 152 - 159, Extend test_build_options_maps_blocked_tools_to_disallowed_tools to assert options.allowed_tools equals ["Read"] alongside the existing disallowed-tools assertions. Add a duplicate blocked tool only if this test is intended to verify order-preserving deduplication, and assert the resulting disallowed_tools contains each tool once in its original order.
🤖 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 `@schemas/run-plan.schema.json`:
- Around line 193-199: Update the Rust type/schema source defining telemetry
providers so the generated schema restricts telemetry.providers keys to the
TelemetryProvider enum values relay and native instead of allowing arbitrary
keys. Regenerate schemas/run-plan.schema.json from that intentional type change,
preserving AdapterTelemetryProviderSupport as the value schema and ensuring core
deserialization accepts every schema-valid provider.
In `@tests/adapters/test_adapaters_common_hermes.py`:
- Around line 105-116: Consolidate
test_validate_hermes_telemetry_provider_rejects_native and
test_validate_hermes_telemetry_provider_rejects_mixed_native_and_relay into one
pytest.mark.parametrize test, parameterizing the differing telemetry_plan
payloads while preserving the shared ValueError match assertion.
---
Outside diff comments:
In `@adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py`:
- Around line 408-429: Update the blocked-tools handling in the adapter
configuration flow to propagate the blocked middleware to the built-in
task/delegated subagent path, not only the parent middleware and explicit
dictionary subagents. Extend or reuse _block_subagent so every delegated
subagent receives the same blocked set while preserving existing behavior for
parent middleware and non-subagent configuration values.
In `@crates/fabric-core/src/config.rs`:
- Around line 1607-1619: Update the tools_configured assignment in the
capability-plan construction to use config.tools.is_some(), preserving true for
an explicitly provided empty blocked list while remaining false when tools is
omitted. Keep the existing blocked_tools-based checks for target routing and
native/unsupported handling unchanged, including the related logic in the
capability-plan paths around tools_configured.
In `@python/src/nemo_fabric/models.py`:
- Line 417: Update both tools field declarations in the relevant model
definitions to use ToolsConfig | None, removing the raw dict[str, Any] fallback.
Preserve the existing optional behavior and rely on FabricBaseModel extra-field
handling for unknown ToolsConfig fields so known fields remain validated before
to_mapping().
In `@tests/adapters/test_claude_adapter.py`:
- Around line 152-159: Extend
test_build_options_maps_blocked_tools_to_disallowed_tools to assert
options.allowed_tools equals ["Read"] alongside the existing disallowed-tools
assertions. Add a duplicate blocked tool only if this test is intended to verify
order-preserving deduplication, and assert the resulting disallowed_tools
contains each tool once in its original order.
🪄 Autofix (Beta)
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: 2f990cab-e9df-4985-b18b-8812de0ce66a
📒 Files selected for processing (28)
adapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/deepagents/README.mdadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/hermes-cli/README.mdadapters/hermes-sdk/README.mdcrates/fabric-core/src/config.rsdocs/reference/api/rust-library-reference/fabric-core/config/enum-capabilitykind.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-capabilitytarget.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatifstorageconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofendpointfieldnamepolicy.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofendpointtransport.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofmode.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayotlptransport.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayunsupportedbehavior.mdxdocs/reference/api/rust-library-reference/fabric-core/config/index.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-capabilityplan.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-fabricconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-toolsconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-toolsplan.mdxpython/src/nemo_fabric/models.pypython/src/nemo_fabric/types.pyschemas/run-plan.schema.jsontests/adapters/test_adapaters_common_hermes.pytests/adapters/test_claude_adapter.pytests/adapters/test_deepagents.pytests/e2e/test_claude.pytests/python/test_sdk_contract.py
📜 Review details
⚠️ CI failures not shown inline (10)
GitHub Actions: Python / Build wheels (x86_64): feat(tools): add blocked tools policy
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
GitHub Actions: Python / Test (arm64): feat(tools): add blocked tools policy
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mjust test-python�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
CARGO_INCREMENTAL: 0
CARGO_PROFILE_DEV_DEBUG: 0
CARGO_TERM_COLOR: always
RUST_BACKTRACE: short
RUSTFLAGS: -D warnings
CARGO_UNSTABLE_SPARSE_REGISTRY: true
CARGO_REGISTRIES_CRATES_IO_PROTOCOL: sparse
UV_PYTHON_INSTALL_DIR: /home/runner/work/_temp/uv-python-dir
UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
CACHE_ON_FAILURE: false
##[endgroup]
Resolved 193 packages in 2ms
Checked 162 packages in 1ms
============================= test session starts ==============================
platform linux -- Python 3.12.3, pytest-9.1.1, pluggy-1.6.0
rootdir: /home/runner/work/NeMo-Fabric/NeMo-Fabric
configfile: pyproject.toml
plugins: cov-7.1.0, asyncio-1.4.0, langsmith-0.10.1, anyio-4.14.0
asyncio: mode=Mode.AUTO, debug=False, asyncio_default_fixture_loop_scope=session, asyncio_default_test_loop_scope=function
collected 305 items
tests/adapters/test_adapaters_common_hermes.py ......................... [ 8%]
...... [ 10%]
tests/adapters/test_adapaters_common_utils.py ..................... [ 17%]
tests/adapters/test_claude_adapter.py ........................... [ 25%]
tests/adapters/test_codex_cli.py .................F................. [ 37%]
tests/adapters/test_deepagents.py ................................ [ 47%]
tests/adapters/test_hermes_cli.py ... [ 48%]
tests/adapters/test_hermes_cli_preflight.py .. [ 49%]
tests/adapters/test_hermes_sdk_adapter.py .. [ 50%]
tests/docs/test_python_api_docs.py .... [ 51%]
tests/e2e/test_claude.py .s [ 52%]
tests/e2e/test_cli.py . [...
GitHub Actions: Python / Test (x86_64): feat(tools): add blocked tools policy
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mjust test-python�[0m
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
CARGO_INCREMENTAL: 0
CARGO_PROFILE_DEV_DEBUG: 0
CARGO_TERM_COLOR: always
RUST_BACKTRACE: short
RUSTFLAGS: -D warnings
CARGO_UNSTABLE_SPARSE_REGISTRY: true
CARGO_REGISTRIES_CRATES_IO_PROTOCOL: sparse
UV_PYTHON_INSTALL_DIR: /home/runner/work/_temp/uv-python-dir
UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
CACHE_ON_FAILURE: false
##[endgroup]
Resolved 193 packages in 3ms
Checked 162 packages in 2ms
============================= test session starts ==============================
platform linux -- Python 3.12.3, pytest-9.1.1, pluggy-1.6.0
rootdir: /home/runner/work/NeMo-Fabric/NeMo-Fabric
configfile: pyproject.toml
plugins: asyncio-1.4.0, anyio-4.14.0, cov-7.1.0, langsmith-0.10.1
asyncio: mode=Mode.AUTO, debug=False, asyncio_default_fixture_loop_scope=session, asyncio_default_test_loop_scope=function
collected 305 items
tests/adapters/test_adapaters_common_hermes.py ......................... [ 8%]
...... [ 10%]
tests/adapters/test_adapaters_common_utils.py ..................... [ 17%]
tests/adapters/test_claude_adapter.py ........................... [ 25%]
tests/adapters/test_codex_cli.py .................F................. [ 37%]
tests/adapters/test_deepagents.py ................................ [ 47%]
tests/adapters/test_hermes_cli.py ... [ 48%]
tests/adapters/test_hermes_cli_preflight.py .. [ 49%]
tests/adapters/test_hermes_sdk_adapter.py .. [ 50%]
tests/docs/test_python_api_docs.py .... [ 51%]
tests/e2e/test_claude.py .s [ 52%]
tests/e2e/test_cli.py . [...
GitHub Actions: Python / Test (arm64): feat(tools): add blocked tools policy
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
GitHub Actions: Python / 2_Test (x86_64).txt: feat(tools): add blocked tools policy
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
GitHub Actions: Python / Build wheels (arm64): feat(tools): add blocked tools policy
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
GitHub Actions: Python / 1_Test (arm64).txt: feat(tools): add blocked tools policy
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
GitHub Actions: Python / Test (x86_64): feat(tools): add blocked tools policy
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
GitHub Actions: Python / 3_Build wheels (arm64).txt: feat(tools): add blocked tools policy
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
GitHub Actions: Python / 0_Build wheels (x86_64).txt: feat(tools): add blocked tools policy
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
🧰 Additional context used
📓 Path-based instructions (7)
{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 Fabric contracts.
Files:
adapters/hermes-sdk/README.mdadapters/hermes-cli/README.mdadapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/deepagents/README.mdadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
{docs/**,README.md,AGENTS.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,AGENTS.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency with generated schemas.
Files:
docs/reference/api/rust-library-reference/fabric-core/config/enum-capabilitykind.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-capabilitytarget.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofendpointfieldnamepolicy.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatifstorageconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayunsupportedbehavior.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-capabilityplan.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-toolsconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-toolsplan.mdxdocs/reference/api/rust-library-reference/fabric-core/config/index.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofendpointtransport.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofmode.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-fabricconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayotlptransport.mdx
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Usepytestto run Python tests.
Do not add@pytest.mark.asyncioto test functions; async tests are detected and run automatically.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, addingspecwhen needed.
Name mocked classes with amockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not duplicate fixtures across test files; if a fixture is needed in multiple test files, define it inconftest.py.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the function as<fixture_name>_fixture() -> <return_type>; only passscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating separate tests for different input types.
If a fixture is needed for a test but does not return a value, or its value is unused, use@pytest.mark.usefixtures.
When modifying environment variables in a test, useos.environ;tests/conftest.pyprovides an autouserestore_environ_fixturethat restores environment variables after each test, somonkeypatch.setenvis unnecessary.
Files:
tests/e2e/test_claude.pytests/adapters/test_claude_adapter.pytests/adapters/test_adapaters_common_hermes.pytests/adapters/test_deepagents.pytests/python/test_sdk_contract.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_claude.pytests/adapters/test_claude_adapter.pytests/adapters/test_adapaters_common_hermes.pytests/adapters/test_deepagents.pytests/python/test_sdk_contract.py
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
python/src/nemo_fabric/**/*
⚙️ CodeRabbit configuration file
python/src/nemo_fabric/**/*: Review Python SDK changes for typed API consistency, import-time dependency neutrality, async/session behavior, and parity with the native extension.
Stubs and runtime implementations should stay aligned.
Files:
python/src/nemo_fabric/models.pypython/src/nemo_fabric/types.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
🧠 Learnings (2)
📚 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/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
📚 Learning: 2026-06-28T04:03:32.877Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 26
File: python/tests/smoke_typed_config.py:163-177
Timestamp: 2026-06-28T04:03:32.877Z
Learning: In NVIDIA NeMo Fabric Python SDK serialization of `RuntimeCapabilities` (to satisfy the “parity contract” with Rust core and the CLI), do not emit metadata keys when the corresponding metadata is absent. Instead, omit those fields entirely so the produced JSON matches the Rust/CLI output (e.g., avoid `null`, empty objects, or placeholder metadata). During review, verify the serializer/builders follow this omission rule and that Python outputs/parity tests reflect the same shape.
Applied to files:
python/src/nemo_fabric/models.pypython/src/nemo_fabric/types.py
🧬 Code graph analysis (4)
adapters/claude/src/nemo_fabric_adapters/claude/adapter.py (1)
adapters/common/src/nemo_fabric_adapters/common/utils.py (2)
load_payload(69-75)capability_plan(131-132)
tests/adapters/test_adapaters_common_hermes.py (1)
adapters/common/src/nemo_fabric_adapters/common/hermes.py (1)
validate_hermes_telemetry_provider(43-46)
tests/python/test_sdk_contract.py (1)
python/src/nemo_fabric/types.py (1)
to_mapping(866-873)
python/src/nemo_fabric/types.py (1)
tests/python/test_sdk_contract.py (1)
_ToolsConfig(214-214)
🪛 Ruff (0.15.21)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
[warning] 72-72: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 318-318: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 395-395: Dynamically typed expressions (typing.Any) are disallowed in model
(ANN401)
adapters/claude/src/nemo_fabric_adapters/claude/adapter.py
[warning] 538-538: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (33)
python/src/nemo_fabric/models.py (1)
360-388: LGTM!Also applies to: 400-400, 481-525
python/src/nemo_fabric/types.py (1)
372-410: LGTM!Also applies to: 479-550, 594-642, 696-698, 745-775
schemas/run-plan.schema.json (1)
261-265: LGTM!Also applies to: 616-624, 1611-1636
crates/fabric-core/src/config.rs (1)
55-55: LGTM!Also applies to: 374-395, 432-432, 1323-1323, 1710-1764, 1921-1921, 1950-1950, 2066-2109, 2152-2154, 2184-2186, 2240-2242, 2290-2404, 2657-2687
docs/reference/api/rust-library-reference/fabric-core/config/struct-toolsconfig.mdx (1)
1-107: LGTM!docs/reference/api/rust-library-reference/fabric-core/config/struct-toolsplan.mdx (1)
1-103: LGTM!docs/reference/api/rust-library-reference/fabric-core/config/enum-capabilitykind.mdx (1)
5-5: LGTM!docs/reference/api/rust-library-reference/fabric-core/config/enum-capabilitytarget.mdx (1)
5-5: LGTM!docs/reference/api/rust-library-reference/fabric-core/config/enum-relayatifstorageconfig.mdx (1)
5-5: LGTM!docs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofendpointfieldnamepolicy.mdx (1)
5-5: LGTM!docs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofendpointtransport.mdx (1)
5-5: LGTM!docs/reference/api/rust-library-reference/fabric-core/config/struct-fabricconfig.mdx (1)
12-12: LGTM!Also applies to: 42-42, 58-61
docs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofmode.mdx (1)
5-5: LGTM!docs/reference/api/rust-library-reference/fabric-core/config/enum-relayotlptransport.mdx (1)
5-5: LGTM!Also applies to: 21-124
docs/reference/api/rust-library-reference/fabric-core/config/enum-relayunsupportedbehavior.mdx (1)
5-5: LGTM!Also applies to: 22-130
docs/reference/api/rust-library-reference/fabric-core/config/index.mdx (1)
53-54: LGTM!docs/reference/api/rust-library-reference/fabric-core/config/struct-capabilityplan.mdx (1)
5-5: LGTM!Also applies to: 12-20
adapters/deepagents/README.md (1)
55-57: 📐 Maintainability & Code QualityCross-check this "delegated subagents alike" guarantee against the general-purpose subagent case.
This documents
tools.blockedas enforced across "delegated subagents alike," but the adapter's_block_subagenthelper (per the change summary) only rewrites dict-form entries in the explicitsubagentslist. See the companion comment onadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py(lines 408-429) regarding whether the built-in, auto-injected general-purpose subagent inherits the parent's blocking middleware. If it doesn't, this documentation overstates the guarantee for that path.Also applies to: 68-73
adapters/claude/src/nemo_fabric_adapters/claude/adapter.py (2)
211-230: Blocked-dict short-circuit looks correct.Returning
Nonefrom_normalized_toolswhentoolsis a dict with"blocked"correctly defers restriction to_disallowed_tools/disallowed_tools, matching the adapter-invocation schema'sToolsConfigshape and the PR's Claude mapping (blocked →disallowed_tools).
102-107: LGTM!Also applies to: 187-208, 240-245, 263-263, 302-302, 375-378, 413-415, 485-493, 524-539
adapters/common/src/nemo_fabric_adapters/common/utils.py (1)
47-47: LGTM!adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (2)
210-224: Blocked-dict short-circuit for_allowed_tool_namesis consistent with the Claude adapter's equivalent logic.
69-73: LGTM!Also applies to: 127-132, 315-320, 352-357, 362-367, 394-397, 441-445, 519-526
adapters/hermes-cli/README.md (1)
27-29: LGTM!adapters/hermes-sdk/README.md (1)
25-27: LGTM!tests/adapters/test_deepagents.py (3)
450-470: 🎯 Functional CorrectnessConfirm blocked-tool middleware tests assert actual blocking, not just presence.
A prior review flagged that these tests (
test_blocked_tools_middleware_blocks_configured_tools,test_subagents_are_gated_by_blocked_tools) risk passing even with unrelated middleware if they only check for non-empty middleware lists. These bodies weren't in the reviewed snippet, so please confirm the assertions actually invoke the gate (e.g., blocked tool call returns an errorToolMessage, allowed tool executes) for both the main agent andsubagents[0]["middleware"].#!/bin/bash sed -n '440,475p;635,675p' tests/adapters/test_deepagents.pyAlso applies to: 641-671
19-20: LGTM!Also applies to: 135-136, 276-276, 304-304, 356-356, 372-372, 494-497, 761-761, 785-785
272-301: LGTM!Also applies to: 304-344
tests/adapters/test_adapaters_common_hermes.py (1)
146-168: LGTM!Also applies to: 378-378, 382-444, 447-448
tests/adapters/test_claude_adapter.py (1)
26-26: LGTM!Also applies to: 121-122, 171-171, 303-303
tests/e2e/test_claude.py (1)
14-20: LGTM!Also applies to: 51-51, 62-63, 86-93, 111-111
tests/python/test_sdk_contract.py (2)
890-900: LGTM!Also applies to: 926-935
277-299: 🗄️ Data Integrity & IntegrationNo change needed:
enable_relay()preserves omitted fields and only updates explicitly provided values.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
crates/fabric-core/src/config.rs (1)
1607-1619: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve policy-presence semantics for
tools_configured.
tools: { blocked: [] }provides a tool policy, but this change reportscapability_plan.tools_configured == false. Keep the top-level flag based onconfig.tools.is_some(); target routing may still remain disabled when the deny-list is empty. Otherwise consumers cannot distinguish an omitted policy from an explicitly empty one.As per path instructions,
capability_plan.tools_configuredindicates whether tool config was provided.Also applies to: 1690-1693, 2708-2772
🤖 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 1607 - 1619, Update the tools_configured assignment in the capability-plan construction to use config.tools.is_some(), preserving true for an explicitly provided empty blocked list while remaining false when tools is omitted. Keep the existing blocked_tools-based checks for target routing and native/unsupported handling unchanged, including the related logic in the capability-plan paths around tools_configured.Source: Path instructions
python/src/nemo_fabric/models.py (1)
417-417: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRemove the raw
dictfallback fromtools
FabricBaseModel(extra="allow")already preserves unknownToolsConfigfields, soToolsConfig | dict[str, Any] | Noneonly bypasses validation for known keys likeblockedand can leak malformed configs intoto_mapping(). Make bothtoolsfieldsToolsConfig | None.🤖 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 `@python/src/nemo_fabric/models.py` at line 417, Update both tools field declarations in the relevant model definitions to use ToolsConfig | None, removing the raw dict[str, Any] fallback. Preserve the existing optional behavior and rely on FabricBaseModel extra-field handling for unknown ToolsConfig fields so known fields remain validated before to_mapping().Source: Path instructions
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (1)
408-429: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPropagate blocked-tool middleware to delegated subagents (adapter.py:422-428)
blocked_tools_middleware(blocked)is attached only to the parent agent here, and_block_subagent()rewrites only explicit dict entries. The built-intasksubagent has its own middleware stack, so blocked tools can still be reached through delegation unless that path is gated too.🤖 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 `@adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py` around lines 408 - 429, Update the blocked-tools handling in the adapter configuration flow to propagate the blocked middleware to the built-in task/delegated subagent path, not only the parent middleware and explicit dictionary subagents. Extend or reuse _block_subagent so every delegated subagent receives the same blocked set while preserving existing behavior for parent middleware and non-subagent configuration values.tests/adapters/test_claude_adapter.py (1)
152-159: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCover the rest of the options contract here (
tests/adapters/test_claude_adapter.py:152) — assertoptions.allowed_tools == ["Read"], and add a duplicate inblockedif this test should also pin the order-preserving dedup behavior ofdisallowed_tools.🤖 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 `@tests/adapters/test_claude_adapter.py` around lines 152 - 159, Extend test_build_options_maps_blocked_tools_to_disallowed_tools to assert options.allowed_tools equals ["Read"] alongside the existing disallowed-tools assertions. Add a duplicate blocked tool only if this test is intended to verify order-preserving deduplication, and assert the resulting disallowed_tools contains each tool once in its original order.
🤖 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 `@schemas/run-plan.schema.json`:
- Around line 193-199: Update the Rust type/schema source defining telemetry
providers so the generated schema restricts telemetry.providers keys to the
TelemetryProvider enum values relay and native instead of allowing arbitrary
keys. Regenerate schemas/run-plan.schema.json from that intentional type change,
preserving AdapterTelemetryProviderSupport as the value schema and ensuring core
deserialization accepts every schema-valid provider.
In `@tests/adapters/test_adapaters_common_hermes.py`:
- Around line 105-116: Consolidate
test_validate_hermes_telemetry_provider_rejects_native and
test_validate_hermes_telemetry_provider_rejects_mixed_native_and_relay into one
pytest.mark.parametrize test, parameterizing the differing telemetry_plan
payloads while preserving the shared ValueError match assertion.
---
Outside diff comments:
In `@adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py`:
- Around line 408-429: Update the blocked-tools handling in the adapter
configuration flow to propagate the blocked middleware to the built-in
task/delegated subagent path, not only the parent middleware and explicit
dictionary subagents. Extend or reuse _block_subagent so every delegated
subagent receives the same blocked set while preserving existing behavior for
parent middleware and non-subagent configuration values.
In `@crates/fabric-core/src/config.rs`:
- Around line 1607-1619: Update the tools_configured assignment in the
capability-plan construction to use config.tools.is_some(), preserving true for
an explicitly provided empty blocked list while remaining false when tools is
omitted. Keep the existing blocked_tools-based checks for target routing and
native/unsupported handling unchanged, including the related logic in the
capability-plan paths around tools_configured.
In `@python/src/nemo_fabric/models.py`:
- Line 417: Update both tools field declarations in the relevant model
definitions to use ToolsConfig | None, removing the raw dict[str, Any] fallback.
Preserve the existing optional behavior and rely on FabricBaseModel extra-field
handling for unknown ToolsConfig fields so known fields remain validated before
to_mapping().
In `@tests/adapters/test_claude_adapter.py`:
- Around line 152-159: Extend
test_build_options_maps_blocked_tools_to_disallowed_tools to assert
options.allowed_tools equals ["Read"] alongside the existing disallowed-tools
assertions. Add a duplicate blocked tool only if this test is intended to verify
order-preserving deduplication, and assert the resulting disallowed_tools
contains each tool once in its original order.
🪄 Autofix (Beta)
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: 2f990cab-e9df-4985-b18b-8812de0ce66a
📒 Files selected for processing (28)
adapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/deepagents/README.mdadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/hermes-cli/README.mdadapters/hermes-sdk/README.mdcrates/fabric-core/src/config.rsdocs/reference/api/rust-library-reference/fabric-core/config/enum-capabilitykind.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-capabilitytarget.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatifstorageconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofendpointfieldnamepolicy.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofendpointtransport.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofmode.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayotlptransport.mdxdocs/reference/api/rust-library-reference/fabric-core/config/enum-relayunsupportedbehavior.mdxdocs/reference/api/rust-library-reference/fabric-core/config/index.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-capabilityplan.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-fabricconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-toolsconfig.mdxdocs/reference/api/rust-library-reference/fabric-core/config/struct-toolsplan.mdxpython/src/nemo_fabric/models.pypython/src/nemo_fabric/types.pyschemas/run-plan.schema.jsontests/adapters/test_adapaters_common_hermes.pytests/adapters/test_claude_adapter.pytests/adapters/test_deepagents.pytests/e2e/test_claude.pytests/python/test_sdk_contract.py
📜 Review details
🔇 Additional comments (33)
python/src/nemo_fabric/models.py (1)
360-388: LGTM!Also applies to: 400-400, 481-525
python/src/nemo_fabric/types.py (1)
372-410: LGTM!Also applies to: 479-550, 594-642, 696-698, 745-775
schemas/run-plan.schema.json (1)
261-265: LGTM!Also applies to: 616-624, 1611-1636
crates/fabric-core/src/config.rs (1)
55-55: LGTM!Also applies to: 374-395, 432-432, 1323-1323, 1710-1764, 1921-1921, 1950-1950, 2066-2109, 2152-2154, 2184-2186, 2240-2242, 2290-2404, 2657-2687
docs/reference/api/rust-library-reference/fabric-core/config/struct-toolsconfig.mdx (1)
1-107: LGTM!docs/reference/api/rust-library-reference/fabric-core/config/struct-toolsplan.mdx (1)
1-103: LGTM!docs/reference/api/rust-library-reference/fabric-core/config/enum-capabilitykind.mdx (1)
5-5: LGTM!docs/reference/api/rust-library-reference/fabric-core/config/enum-capabilitytarget.mdx (1)
5-5: LGTM!docs/reference/api/rust-library-reference/fabric-core/config/enum-relayatifstorageconfig.mdx (1)
5-5: LGTM!docs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofendpointfieldnamepolicy.mdx (1)
5-5: LGTM!docs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofendpointtransport.mdx (1)
5-5: LGTM!docs/reference/api/rust-library-reference/fabric-core/config/struct-fabricconfig.mdx (1)
12-12: LGTM!Also applies to: 42-42, 58-61
docs/reference/api/rust-library-reference/fabric-core/config/enum-relayatofmode.mdx (1)
5-5: LGTM!docs/reference/api/rust-library-reference/fabric-core/config/enum-relayotlptransport.mdx (1)
5-5: LGTM!Also applies to: 21-124
docs/reference/api/rust-library-reference/fabric-core/config/enum-relayunsupportedbehavior.mdx (1)
5-5: LGTM!Also applies to: 22-130
docs/reference/api/rust-library-reference/fabric-core/config/index.mdx (1)
53-54: LGTM!docs/reference/api/rust-library-reference/fabric-core/config/struct-capabilityplan.mdx (1)
5-5: LGTM!Also applies to: 12-20
adapters/deepagents/README.md (1)
55-57: 📐 Maintainability & Code QualityCross-check this "delegated subagents alike" guarantee against the general-purpose subagent case.
This documents
tools.blockedas enforced across "delegated subagents alike," but the adapter's_block_subagenthelper (per the change summary) only rewrites dict-form entries in the explicitsubagentslist. See the companion comment onadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py(lines 408-429) regarding whether the built-in, auto-injected general-purpose subagent inherits the parent's blocking middleware. If it doesn't, this documentation overstates the guarantee for that path.Also applies to: 68-73
adapters/claude/src/nemo_fabric_adapters/claude/adapter.py (2)
211-230: Blocked-dict short-circuit looks correct.Returning
Nonefrom_normalized_toolswhentoolsis a dict with"blocked"correctly defers restriction to_disallowed_tools/disallowed_tools, matching the adapter-invocation schema'sToolsConfigshape and the PR's Claude mapping (blocked →disallowed_tools).
102-107: LGTM!Also applies to: 187-208, 240-245, 263-263, 302-302, 375-378, 413-415, 485-493, 524-539
adapters/common/src/nemo_fabric_adapters/common/utils.py (1)
47-47: LGTM!adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (2)
210-224: Blocked-dict short-circuit for_allowed_tool_namesis consistent with the Claude adapter's equivalent logic.
69-73: LGTM!Also applies to: 127-132, 315-320, 352-357, 362-367, 394-397, 441-445, 519-526
adapters/hermes-cli/README.md (1)
27-29: LGTM!adapters/hermes-sdk/README.md (1)
25-27: LGTM!tests/adapters/test_deepagents.py (3)
450-470: 🎯 Functional CorrectnessConfirm blocked-tool middleware tests assert actual blocking, not just presence.
A prior review flagged that these tests (
test_blocked_tools_middleware_blocks_configured_tools,test_subagents_are_gated_by_blocked_tools) risk passing even with unrelated middleware if they only check for non-empty middleware lists. These bodies weren't in the reviewed snippet, so please confirm the assertions actually invoke the gate (e.g., blocked tool call returns an errorToolMessage, allowed tool executes) for both the main agent andsubagents[0]["middleware"].#!/bin/bash sed -n '440,475p;635,675p' tests/adapters/test_deepagents.pyAlso applies to: 641-671
19-20: LGTM!Also applies to: 135-136, 276-276, 304-304, 356-356, 372-372, 494-497, 761-761, 785-785
272-301: LGTM!Also applies to: 304-344
tests/adapters/test_adapaters_common_hermes.py (1)
146-168: LGTM!Also applies to: 378-378, 382-444, 447-448
tests/adapters/test_claude_adapter.py (1)
26-26: LGTM!Also applies to: 121-122, 171-171, 303-303
tests/e2e/test_claude.py (1)
14-20: LGTM!Also applies to: 51-51, 62-63, 86-93, 111-111
tests/python/test_sdk_contract.py (2)
890-900: LGTM!Also applies to: 926-935
277-299: 🗄️ Data Integrity & IntegrationNo change needed:
enable_relay()preserves omitted fields and only updates explicitly provided values.
🛑 Comments failed to post (2)
schemas/run-plan.schema.json (1)
193-199: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Constrain telemetry provider keys in the public schema.
This schema accepts arbitrary
telemetry.providerskeys, but core deserialization rejects providers outsiderelayandnative. A schema-valid adapter descriptor can therefore fail at runtime. Generate a schema that restricts provider names to the RustTelemetryProviderenum.As per path instructions, generated schemas are public contract snapshots and must correspond to intentional Rust type changes.
🤖 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 `@schemas/run-plan.schema.json` around lines 193 - 199, Update the Rust type/schema source defining telemetry providers so the generated schema restricts telemetry.providers keys to the TelemetryProvider enum values relay and native instead of allowing arbitrary keys. Regenerate schemas/run-plan.schema.json from that intentional type change, preserving AdapterTelemetryProviderSupport as the value schema and ensuring core deserialization accepts every schema-valid provider.Source: Path instructions
tests/adapters/test_adapaters_common_hermes.py (1)
105-116: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Re-merge these reject tests into one
parametrize.This duplicates a previously flagged (and reportedly addressed) issue:
test_validate_hermes_telemetry_provider_rejects_nativeandtest_validate_hermes_telemetry_provider_rejects_mixed_native_and_relaydiffer only by theproviders/relay_enabledpayload and assert the identical error.♻️ Proposed consolidation
-def test_validate_hermes_telemetry_provider_rejects_native(): - payload = {"telemetry_plan": {"providers": ["native"], "relay_enabled": False}} - - with pytest.raises(ValueError, match="only relay telemetry is supported for Hermes"): - hermes_common.validate_hermes_telemetry_provider(payload) - - -def test_validate_hermes_telemetry_provider_rejects_mixed_native_and_relay(): - payload = {"telemetry_plan": {"providers": ["relay", "native"], "relay_enabled": True}} - - with pytest.raises(ValueError, match="only relay telemetry is supported for Hermes"): - hermes_common.validate_hermes_telemetry_provider(payload) +@pytest.mark.parametrize( + ("providers", "relay_enabled"), + [(["native"], False), (["relay", "native"], True)], +) +def test_validate_hermes_telemetry_provider_rejects_native(providers, relay_enabled): + payload = {"telemetry_plan": {"providers": providers, "relay_enabled": relay_enabled}} + + with pytest.raises(ValueError, match="only relay telemetry is supported for Hermes"): + hermes_common.validate_hermes_telemetry_provider(payload)As per coding guidelines, "Prefer
pytest.mark.parametrizeover creating separate tests for different input types."🤖 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 `@tests/adapters/test_adapaters_common_hermes.py` around lines 105 - 116, Consolidate test_validate_hermes_telemetry_provider_rejects_native and test_validate_hermes_telemetry_provider_rejects_mixed_native_and_relay into one pytest.mark.parametrize test, parameterizing the differing telemetry_plan payloads while preserving the shared ValueError match assertion.Source: Coding guidelines
Signed-off-by: Anuradha Karuppiah <26330987+AnuradhaKaruppiah@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/adapters/test_deepagents.py (2)
410-427: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
handler/requesthelper pattern across two tests.The same closures are redefined in both
test_blocked_tools_middleware_blocks_configured_toolsandtest_subagents_are_gated_by_blocked_tools. Consider hoisting these into a sharedconftest.pyfixture (e.g., atool_call_request_factory) per the guideline to prefer fixtures over helper methods and avoid duplication.As per coding guidelines, "Prefer pytest fixtures over helper methods" and "Do not duplicate fixtures across test files; if a fixture is needed in multiple test files, define it in
conftest.py."Also applies to: 576-605
🤖 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 `@tests/adapters/test_deepagents.py` around lines 410 - 427, Extract the duplicated handler and request helper closures from test_blocked_tools_middleware_blocks_configured_tools and test_subagents_are_gated_by_blocked_tools into shared pytest fixtures in conftest.py, such as a tool_call_request_factory fixture. Update both tests to use the fixtures while preserving their existing blocked and allowed tool assertions.Source: Coding guidelines
685-685: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRestore coverage for invalid
toolsmappings.ToolsConfigstill accepts mapping input, and_ToolsConfig.from_mapping()rejects badblockedshapes; the existing SDK contract test only covers the scalar case, not{"blocked": "browser"}. Keep this path covered here or add a contract test next to the typed config.🤖 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 `@tests/adapters/test_deepagents.py` at line 685, Extend test_bad_mcp_transport_is_normalized_failure or add a nearby typed-config contract test to cover ToolsConfig mapping input with {"blocked": "browser"}. Assert that _ToolsConfig.from_mapping() rejects the invalid blocked shape, while preserving the existing scalar invalid-tools coverage.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 `@adapters/deepagents/README.md`:
- Around line 84-86: Update the preflight failure list in the README sentence to
include the article “a” before “passthrough option,” preserving the existing
parallel list structure and surrounding wording.
In `@tests/adapters/test_deepagents.py`:
- Around line 416-420: Update the local handler and request helpers, including
the duplicate pair, to remove typing.Any: annotate request’s return value with
types.SimpleNamespace and annotate handler’s request parameter with the
appropriate types.SimpleNamespace type matching the constructed and consumed
object.
- Line 577: Remove the unused fake_sdks parameter from
test_subagents_are_gated_by_blocked_tools, or replace it with
`@pytest.mark.usefixtures`("fake_sdks") if the fixture’s side effects are
required; preserve the test’s existing behavior.
---
Outside diff comments:
In `@tests/adapters/test_deepagents.py`:
- Around line 410-427: Extract the duplicated handler and request helper
closures from test_blocked_tools_middleware_blocks_configured_tools and
test_subagents_are_gated_by_blocked_tools into shared pytest fixtures in
conftest.py, such as a tool_call_request_factory fixture. Update both tests to
use the fixtures while preserving their existing blocked and allowed tool
assertions.
- Line 685: Extend test_bad_mcp_transport_is_normalized_failure or add a nearby
typed-config contract test to cover ToolsConfig mapping input with {"blocked":
"browser"}. Assert that _ToolsConfig.from_mapping() rejects the invalid blocked
shape, while preserving the existing scalar invalid-tools coverage.
🪄 Autofix (Beta)
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: 18e6064e-684c-4395-8a21-5bf35ca816b3
📒 Files selected for processing (12)
adapters/claude/README.mdadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/deepagents/README.mdadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pycrates/fabric-core/src/doctor.rscrates/fabric-core/src/error.rscrates/fabric-core/src/runtime.rsdocs/reference/api/python-library-reference/nemo_fabric.models.mddocs/reference/api/rust-library-reference/fabric-core/error/enum-fabricerror.mdxscripts/generate_api_docs.shtests/adapters/test_claude_adapter.pytests/adapters/test_deepagents.py
💤 Files with no reviewable changes (1)
- adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Pre-commit
- GitHub Check: Test (x86_64)
- GitHub Check: Test (arm64)
🧰 Additional context used
📓 Path-based instructions (5)
{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 Fabric contracts.
Files:
adapters/claude/README.mdadapters/deepagents/README.mdadapters/claude/src/nemo_fabric_adapters/claude/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/error.rscrates/fabric-core/src/doctor.rscrates/fabric-core/src/runtime.rs
{docs/**,README.md,AGENTS.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,AGENTS.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency with generated schemas.
Files:
docs/reference/api/rust-library-reference/fabric-core/error/enum-fabricerror.mdxdocs/reference/api/python-library-reference/nemo_fabric.models.md
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Usepytestto run Python tests.
Do not add@pytest.mark.asyncioto test functions; async tests are detected and run automatically.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, addingspecwhen needed.
Name mocked classes with amockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not duplicate fixtures across test files; if a fixture is needed in multiple test files, define it inconftest.py.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the function as<fixture_name>_fixture() -> <return_type>; only passscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating separate tests for different input types.
If a fixture is needed for a test but does not return a value, or its value is unused, use@pytest.mark.usefixtures.
When modifying environment variables in a test, useos.environ;tests/conftest.pyprovides an autouserestore_environ_fixturethat restores environment variables after each test, somonkeypatch.setenvis unnecessary.
Files:
tests/adapters/test_deepagents.pytests/adapters/test_claude_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_deepagents.pytests/adapters/test_claude_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/claude/src/nemo_fabric_adapters/claude/adapter.py
🧬 Code graph analysis (5)
crates/fabric-core/src/doctor.rs (1)
crates/fabric-core/src/config.rs (6)
RunPlan(1837-1869)CapabilityTarget(1996-2003)AdapterKind(493-502)CapabilityRoute(1970-1979)resolve_run_plan(1183-1186)CapabilityKind(1984-1991)
crates/fabric-core/src/runtime.rs (1)
crates/fabric-core/src/config.rs (5)
RunPlan(1837-1869)CapabilityPlan(1919-1944)CapabilityTarget(1996-2003)resolve_run_plan(1183-1186)CapabilityKind(1984-1991)
tests/adapters/test_deepagents.py (1)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (2)
build_agent_kwargs(364-386)awrap_tool_call(228-231)
adapters/claude/src/nemo_fabric_adapters/claude/adapter.py (1)
python/src/nemo_fabric/types.py (2)
tools(691-694)tools(697-698)
tests/adapters/test_claude_adapter.py (1)
python/src/nemo_fabric/types.py (2)
tools(691-694)tools(697-698)
🪛 Ruff (0.15.21)
tests/adapters/test_deepagents.py
[warning] 416-416: Dynamically typed expressions (typing.Any) are disallowed in _request
(ANN401)
[warning] 419-419: Dynamically typed expressions (typing.Any) are disallowed in request
(ANN401)
[warning] 577-577: Unused function argument: fake_sdks
(ARG001)
[warning] 594-594: Dynamically typed expressions (typing.Any) are disallowed in _request
(ANN401)
[warning] 597-597: Dynamically typed expressions (typing.Any) are disallowed in request
(ANN401)
🔇 Additional comments (13)
docs/reference/api/python-library-reference/nemo_fabric.models.md (2)
1424-1426: Blank line after theToolsConfigheading is now present, resolving the previously flagged MD022 violation.
1550-1558: LGTM!scripts/generate_api_docs.sh (1)
42-43: LGTM!adapters/deepagents/README.md (1)
55-57: LGTM!Also applies to: 72-73
adapters/claude/src/nemo_fabric_adapters/claude/adapter.py (2)
14-30: LGTM!Also applies to: 106-106, 188-188, 241-241, 280-280, 353-356, 391-393, 463-470, 510-517
303-305: 🎯 Functional CorrectnessConfirm no broader consumer still relies on
ClaudeAgentOptions.tools. The local adapter no longer references_normalized_tools, but any SDK code that expectstoolsto carry capability-plan-derived values still needs a repo-wide check.tests/adapters/test_claude_adapter.py (2)
90-90: Fixture and assertions correctly reflect thetools.blockeddeny-list shift (options.tools is None,disallowed_toolsmerges blocked + settings-level entries). Coverage for the new_disallowed_toolsmerge behavior looks adequate for the changed API surface.As per path instructions,
{tests/**,python/tests/**}should cover behavior promised by the changed API surface — this is satisfied here.Also applies to: 122-130, 152-160
14-26: LGTM!Also applies to: 171-171, 186-194, 285-285
adapters/claude/README.md (1)
40-42: LGTM!Also applies to: 84-84, 108-108
crates/fabric-core/src/error.rs (1)
112-119: LGTM!docs/reference/api/rust-library-reference/fabric-core/error/enum-fabricerror.mdx (1)
12-12: LGTM!Also applies to: 202-217
crates/fabric-core/src/doctor.rs (1)
189-196: LGTM!Also applies to: 583-602
crates/fabric-core/src/runtime.rs (1)
410-421: LGTM!Also applies to: 424-439, 441-452, 2053-2072
Signed-off-by: Anuradha Karuppiah <26330987+AnuradhaKaruppiah@users.noreply.github.com>
Signed-off-by: Anuradha Karuppiah <26330987+AnuradhaKaruppiah@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
schemas/run-plan.schema.json (1)
261-264: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRequire the normalized tools plan in
CapabilityPlan.
defaultdoes not make a JSON Schema property required. A plan that omitstoolscan still validate, contradicting the contract that normalized plans always exposetoolsand use{}when no policy is configured. AddtoolstoCapabilityPlan.requiredand keep Rust serialization aligned.As per path instructions, schemas must encode the first-class normalized
capability_plan.toolsfield.Also applies to: 1674-1674
🤖 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 `@schemas/run-plan.schema.json` around lines 261 - 264, Update the CapabilityPlan schema’s required properties to include tools, while retaining its default {} behavior for omitted policy configuration. Ensure the corresponding Rust serialization model emits the normalized capability_plan.tools field consistently with this required schema contract.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 `@adapters/claude/src/nemo_fabric_adapters/claude/adapter.py`:
- Around line 711-804: Reduce the PLR0912/PLR0915 complexity of run_claude by
extracting relay lifecycle logic into focused helpers, such as _start_relay for
gateway startup and _finalize_relay for gateway shutdown, plugin removal, and
cleanup-error construction. Keep query streaming, session persistence, relay
output wrapping, and cleanup-error merging behavior unchanged while moving the
corresponding branches out of run_claude.
In `@adapters/deepagents/README.md`:
- Around line 84-85: Update the preflight-failure description in the README to
identify the condition as “an invalid or unsupported passthrough option,”
preserving the existing references to missing credentials, the absent package,
and invalid MCP servers.
In `@adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py`:
- Around line 414-445: Narrow the _gated_subagents subagents parameter from Any
to list[Any] | None, matching its existing None-or-list validation and
preserving the AdapterConfigError behavior for other runtime values.
In `@README.md`:
- Line 84: Update the README installation instruction to format the package
extra “nemo-fabric[hermes]” as inline code, while leaving the surrounding
wording unchanged.
In `@tests/adapters/test_hermes_adapter.py`:
- Around line 355-470: Add a regression test covering overlapping tool
configuration, with a name present in both tools.blocked and
settings.enabled_toolsets (and, where applicable, disabled_toolsets). In the
adapter.run_hermes test flow, assert the blocked tool is removed from the
enabled_toolsets passed to AIAgent or otherwise cannot execute, preserving
fail-closed behavior.
---
Outside diff comments:
In `@schemas/run-plan.schema.json`:
- Around line 261-264: Update the CapabilityPlan schema’s required properties to
include tools, while retaining its default {} behavior for omitted policy
configuration. Ensure the corresponding Rust serialization model emits the
normalized capability_plan.tools field consistently with this required schema
contract.
🪄 Autofix (Beta)
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: 73272b4d-8569-4ca5-bf4a-2ceb3c08f1e4
📒 Files selected for processing (21)
README.mdadapters/claude/README.mdadapters/claude/fabric-adapter.jsonadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/deepagents/README.mdadapters/deepagents/fabric-adapter.jsonadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/hermes/README.mdadapters/hermes/fabric-adapter.jsonadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pycrates/fabric-core/src/config.rscrates/fabric-core/src/doctor.rscrates/fabric-core/src/runtime.rsdocs/reference/api/rust-library-reference/fabric-core/config/struct-adapterconfigsupport.mdxschemas/adapter-descriptor.schema.jsonschemas/run-plan.schema.jsontests/adapters/test_claude_adapter.pytests/adapters/test_deepagents.pytests/adapters/test_hermes_adapter.pytests/e2e/test_claude.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (33)
**/*.{md,mdx,html}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Changes affecting public behavior, adapters, examples, or workspace structure must update the corresponding documentation; public API changes require updated SDK or API reference documentation.
Files:
docs/reference/api/rust-library-reference/fabric-core/config/struct-adapterconfigsupport.mdxadapters/hermes/README.mdREADME.mdadapters/claude/README.mdadapters/deepagents/README.md
**/*.{md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
For docs site changes, run
just docsto regenerate Python and Rust API references and validate Fern configuration.
**/*.{md,mdx}: Prioritize factual accuracy in NeMo Fabric documentation and keep commands, package names, APIs, file paths, repository layout, entry points, support claims, examples, and procedures aligned with current repository behavior.
Update relevant entry-point documentation when public behavior changes, includingREADME.md,docs/index.yml, package or crate READMEs, and adapter or integration READMEs.
Use{/* ... */}delimiters for top-of-file SPDX comments in MDX files, not HTML comment delimiters.
CapitalizeNVIDIAcorrectly and use consistent current repository terminology, product names, APIs, and feature names.
Format commands, code, expressions, file names, paths, and filenames as inline code where appropriate.
Use title case for technical-documentation headings.
Introduce code blocks, tables, and lists with complete lead-in sentences.
Use descriptive link text instead of raw URLs or generic labels such ashere.
Write procedures as short, imperative, parallel, easy-to-scan steps; prefer active voice, present tense, plain English, and concise sentences.
Useafterinstead ofoncewhen expressing temporal sequence, and usecaninstead ofmaywhen describing possibility rather than permission.
Use unambiguous date formats and avoid ordinal dates in body text.
When reviewing documentation, report findings in severity order underMust fix,Should fix, andNice to have, with file paths, line references, explanations, and concrete rewrites or directions.
Files:
docs/reference/api/rust-library-reference/fabric-core/config/struct-adapterconfigsupport.mdxadapters/hermes/README.mdREADME.mdadapters/claude/README.mdadapters/deepagents/README.md
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: All source files must include the specified SPDX copyright and Apache-2.0 license header using the comment syntax appropriate to the file type.
Release tags must use raw Rust-compatible SemVer without a leadingv, such as0.1.0or0.1.0-rc.1.
**/*: 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.
**/*: Keep pull request branch scope coherent and reviewable.
Run relevant tests undervalidate-changebefore opening or updating a pull request.
Format changed files with the language-native formatter.
Update documentation and examples for public behavior changes.
Update dependent maintainer or consumer guidance when code changes affect APIs, bindings, commands, paths, packaging guidance, or best practices.
Use Conventional Commit style for pull request titles:<type>: <concise imperative summary>, choosing the type from the actual change surface. Usefixonly for user-facing or runtime product-code bug fixes.
A pull request body must include#### Overview,#### Details,#### Validation,#### Where should the reviewer start?, and `#### Related ...
Files:
docs/reference/api/rust-library-reference/fabric-core/config/struct-adapterconfigsupport.mdxadapters/hermes/README.mdadapters/hermes/fabric-adapter.jsonREADME.mdadapters/claude/README.mdadapters/claude/fabric-adapter.jsonadapters/deepagents/fabric-adapter.jsonschemas/adapter-descriptor.schema.jsonadapters/deepagents/README.mdtests/adapters/test_hermes_adapter.pyadapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pycrates/fabric-core/src/doctor.rscrates/fabric-core/src/runtime.rsschemas/run-plan.schema.jsontests/e2e/test_claude.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.pytests/adapters/test_claude_adapter.pycrates/fabric-core/src/config.rstests/adapters/test_deepagents.py
**/*.mdx
📄 CodeRabbit inference engine (CONTRIBUTING.md)
MDX files must use the specified SPDX header in a JSX comment.
In MDX files, use JSX comment delimiters (
{/*and*/}) for top-of-file comments, including SPDX headers; do not use HTML comments.
Files:
docs/reference/api/rust-library-reference/fabric-core/config/struct-adapterconfigsupport.mdx
{README.md,docs/**/*.{md,mdx,yml},examples/**/*.{md,mdx,yml}}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repository references, and build commands current in documentation and examples.
Files:
docs/reference/api/rust-library-reference/fabric-core/config/struct-adapterconfigsupport.mdxREADME.md
{docs/**/*.{md,mdx,yml},examples/**/*.{md,mdx,yml}}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Update relevant getting-started, reference, adapter, and example documentation when the corresponding examples or adapters change.
Files:
docs/reference/api/rust-library-reference/fabric-core/config/struct-adapterconfigsupport.mdx
docs/**/*.{md,mdx,yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Run
just docswhen the documentation site changes.
Files:
docs/reference/api/rust-library-reference/fabric-core/config/struct-adapterconfigsupport.mdx
{docs/**/*,.github/workflows/ci_python.yml,.github/workflows/ci_rust.yml,justfile}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Use the current install, import, build, test, clean, and documentation commands consistently in documentation, examples, CI workflows, and just recipes.
Files:
docs/reference/api/rust-library-reference/fabric-core/config/struct-adapterconfigsupport.mdx
{docs/**/*,.github/workflows/ci_python.yml,.github/workflows/ci_rust.yml}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Reflect public packaging changes in release-facing documentation and examples.
Files:
docs/reference/api/rust-library-reference/fabric-core/config/struct-adapterconfigsupport.mdx
**/*.{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:
docs/reference/api/rust-library-reference/fabric-core/config/struct-adapterconfigsupport.mdxadapters/hermes/README.mdREADME.mdadapters/claude/README.mdadapters/deepagents/README.md
docs/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For documentation or examples changes, run
just docswhen practical and verify documented commands against the current repository.
Files:
docs/reference/api/rust-library-reference/fabric-core/config/struct-adapterconfigsupport.mdx
{docs/**,README.md,AGENTS.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,AGENTS.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency with generated schemas.
Files:
docs/reference/api/rust-library-reference/fabric-core/config/struct-adapterconfigsupport.mdxREADME.md
**/README.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update an adapter or example
README.mdwhen that adapter or example surface changes.
Files:
adapters/hermes/README.mdREADME.mdadapters/claude/README.mdadapters/deepagents/README.md
**/*.{html,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
HTML and Markdown files must use the specified SPDX header in an HTML comment.
Files:
adapters/hermes/README.mdREADME.mdadapters/claude/README.mdadapters/deepagents/README.md
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Update documentation and examples in the same branch as the public API change.
Verify README and documentation entry points, package names, paths, examples, and public commands remain current after changes.
Files:
adapters/hermes/README.mdREADME.mdadapters/claude/README.mdadapters/deepagents/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:
adapters/hermes/README.mdREADME.mdadapters/claude/README.mdadapters/deepagents/README.md
{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 Fabric contracts.
Files:
adapters/hermes/README.mdadapters/hermes/fabric-adapter.jsonadapters/claude/README.mdadapters/claude/fabric-adapter.jsonadapters/deepagents/fabric-adapter.jsonadapters/deepagents/README.mdadapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
**/*.{json,jsonc}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Public contract changes must keep checked-in JSON Schema snapshots synchronized.
Files:
adapters/hermes/fabric-adapter.jsonadapters/claude/fabric-adapter.jsonadapters/deepagents/fabric-adapter.jsonschemas/adapter-descriptor.schema.jsonschemas/run-plan.schema.json
**/*.{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:
adapters/hermes/fabric-adapter.jsonadapters/claude/fabric-adapter.jsonadapters/deepagents/fabric-adapter.jsonschemas/adapter-descriptor.schema.jsontests/adapters/test_hermes_adapter.pyadapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pycrates/fabric-core/src/doctor.rscrates/fabric-core/src/runtime.rsschemas/run-plan.schema.jsontests/e2e/test_claude.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.pytests/adapters/test_claude_adapter.pycrates/fabric-core/src/config.rstests/adapters/test_deepagents.py
README.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
The root
README.mdmust reflect the current workspace, supported adapters, and top-level documentation.Update
README.mdwhen a small Fabric bug fix changes public behavior.
Files:
README.md
{README.md,docs/index.yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Update
README.mdordocs/index.ymlwhen documentation entry points or example reading paths change.
Files:
README.md
schemas/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
When public configuration types change, ensure schema snapshot tests pass through
just test-rustand review generated schema diffs.
Files:
schemas/adapter-descriptor.schema.jsonschemas/run-plan.schema.json
⚙️ 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/adapter-descriptor.schema.jsonschemas/run-plan.schema.json
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: Python public APIs must use type annotations, and native Python binding declarations must remain synchronized with their Rust implementations.
Python files must begin with the specified#SPDX copyright and Apache-2.0 license header.
Files:
tests/adapters/test_hermes_adapter.pyadapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pytests/e2e/test_claude.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.pytests/adapters/test_claude_adapter.pytests/adapters/test_deepagents.py
**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{rs,py}: Usesnake_casefor Rust and Python functions and variables; usePascalCasefor Rust types and Python classes.
Run tests for every language surface affected by a change. Changes touching the Rust core or public schemas require both Rust and Python test suites.
Public contract changes must keep native Python binding declarations synchronized with their Rust implementations.
Files:
tests/adapters/test_hermes_adapter.pyadapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pycrates/fabric-core/src/doctor.rscrates/fabric-core/src/runtime.rstests/e2e/test_claude.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.pytests/adapters/test_claude_adapter.pycrates/fabric-core/src/config.rstests/adapters/test_deepagents.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 annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorAsyncMock, supplyingspecwhen necessary; do not define a new mock class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
Define shared fixtures inconftest.pyrather than repeating them across test files.
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 return value is unused.
Useos.environto modify environment variables in tests; do not usemonkeypatch.setenv, because the autouserestore_environ_fixtureintests/conftest.pyrestores the environment after each test.
Avoid defensive programming in tests; access expected data directly so missing data raises a clear error instead of being silently tolerated.
Run focused tests withuv run pytest -k "<pattern>"and all tests withuv run pytest.
Files:
tests/adapters/test_hermes_adapter.pytests/e2e/test_claude.pytests/adapters/test_claude_adapter.pytests/adapters/test_deepagents.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
When Python code or a Python-facing adapter changes, run
just test-python.
Files:
tests/adapters/test_hermes_adapter.pyadapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pytests/e2e/test_claude.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.pytests/adapters/test_claude_adapter.pytests/adapters/test_deepagents.py
**/*.{rs,py,pyi,toml}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
When the PyO3 bridge or package metadata changes, run
just build-pythonandcargo check -p fabric-python --locked.
Files:
tests/adapters/test_hermes_adapter.pyadapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pycrates/fabric-core/src/doctor.rscrates/fabric-core/src/runtime.rstests/e2e/test_claude.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.pytests/adapters/test_claude_adapter.pycrates/fabric-core/src/config.rstests/adapters/test_deepagents.py
tests/adapters/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
When an adapter or integration changes, run its focused tests under
tests/adapters, followed byjust test-python.
Files:
tests/adapters/test_hermes_adapter.pytests/adapters/test_claude_adapter.pytests/adapters/test_deepagents.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.pytests/e2e/test_claude.pytests/adapters/test_claude_adapter.pytests/adapters/test_deepagents.py
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{rs,toml}: Rust code must be formatted withcargo fmt --all; formatting can be checked withcargo fmt --all -- --check, and Rust workspaces must compile withcargo check --workspace --locked.
Rust files must begin with the specified//SPDX copyright and Apache-2.0 license header.When Rust code or Rust project configuration changes, run
cargo fmt --all -- --checkandjust test-rust.
Files:
crates/fabric-core/src/doctor.rscrates/fabric-core/src/runtime.rscrates/fabric-core/src/config.rs
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Implement new runtime or binding behavior in the shared Rust core first.
For any Rust change, run
just test-rustandcargo fmt --all -- --check.Run
cargo check --workspace --lockedafter version changes.
Files:
crates/fabric-core/src/doctor.rscrates/fabric-core/src/runtime.rscrates/fabric-core/src/config.rs
crates/fabric-core/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For changes under
crates/fabric-core, run both the Rust and Python test suites.When
crates/fabric-corechanges in a way exposed through Python, run both the Rust and Python test suites.
Files:
crates/fabric-core/src/doctor.rscrates/fabric-core/src/runtime.rscrates/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/doctor.rscrates/fabric-core/src/runtime.rscrates/fabric-core/src/config.rs
🧠 Learnings (2)
📚 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:
adapters/hermes/fabric-adapter.jsonadapters/claude/fabric-adapter.jsonadapters/deepagents/fabric-adapter.json
📚 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.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
🧬 Code graph analysis (5)
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py (1)
adapters/common/src/nemo_fabric_adapters/common/utils.py (9)
settings_payload(106-108)environment_payload(102-103)normalize_list(178-185)telemetry_providers(148-152)merge_unique(188-194)dump_yaml(201-207)capability_plan(164-165)blocked_tools(173-175)relay_enabled(155-156)
crates/fabric-core/src/doctor.rs (1)
crates/fabric-core/src/config.rs (1)
resolve_run_plan(1183-1186)
adapters/claude/src/nemo_fabric_adapters/claude/adapter.py (3)
adapters/common/src/nemo_fabric_adapters/common/utils.py (6)
load_relay_plugin_config(210-234)collect_relay_artifacts(422-440)capability_plan(164-165)config_root(66-67)relay_enabled(155-156)runtime_context(83-84)adapters/common/src/nemo_fabric_adapters/common/relay_gateway.py (6)
RelayGatewayLaunch(31-38)RelayGatewayError(26-27)start_relay_gateway(132-178)relay_cli_observability_version(63-83)stop_relay_gateway(111-129)resolve_relay_command(41-52)adapters/common/src/nemo_fabric_adapters/common/relay_hooks.py (1)
render_relay_hooks(45-69)
tests/adapters/test_claude_adapter.py (2)
adapters/claude/src/nemo_fabric_adapters/claude/adapter.py (2)
AdapterConfigError(114-115)run(808-816)adapters/common/src/nemo_fabric_adapters/common/relay_gateway.py (2)
RelayGatewayLaunch(31-38)RelayGatewayError(26-27)
tests/adapters/test_deepagents.py (1)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (2)
AdapterConfigError(55-56)awrap_tool_call(228-231)
🪛 ast-grep (0.44.1)
adapters/claude/src/nemo_fabric_adapters/claude/adapter.py
[warning] 420-420: Do not make http calls without encryption
Context: f"http://{gateway_bind}"
Note: [CWE-319] Cleartext Transmission of Sensitive Information.
(requests-http)
[info] 342-350: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"name": "nemo-fabric-relay",
"description": "NeMo Relay hooks managed by NeMo Fabric",
"version": "1.0.0",
},
indent=2,
sort_keys=True,
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 355-359: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
relay_hooks.render_relay_hooks("claude", executable),
indent=2,
sort_keys=True,
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
tests/adapters/test_claude_adapter.py
[info] 166-175: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"relay": {
"config": {
"atof": {"enabled": True},
"atif": {"enabled": True},
}
}
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 597-597: use jsonify instead of json.dumps for JSON output
Context: json.dumps(output)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 654-654: use jsonify instead of json.dumps for JSON output
Context: json.dumps(output)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 742-742: use jsonify instead of json.dumps for JSON output
Context: json.dumps(output)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.15.21)
tests/adapters/test_hermes_adapter.py
[warning] 136-136: Yoda condition detected
Rewrite as hermes_default == adapter.DEFAULT_MAX_ITERATIONS
(SIM300)
[warning] 388-388: Prefer dict over useless lambda
Replace with lambda with dict
(PIE807)
[warning] 390-390: Unused lambda argument: force
(ARG005)
[warning] 391-391: Unused lambda argument: args
(ARG005)
[warning] 391-391: Unused lambda argument: kwargs
(ARG005)
adapters/common/src/nemo_fabric_adapters/common/utils.py
[warning] 451-453: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 535-535: Consider moving this statement to an else block
(TRY300)
[warning] 537-537: Avoid specifying long messages outside the exception class
(TRY003)
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
[warning] 35-35: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 123-123: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 212-212: Avoid specifying long messages outside the exception class
(TRY003)
tests/e2e/test_claude.py
[warning] 169-169: Async functions should not use pathlib.Path methods, use trio.Path or anyio.path
(ASYNC240)
[warning] 170-170: Async functions should not use pathlib.Path methods, use trio.Path or anyio.path
(ASYNC240)
[warning] 213-213: Async functions should not use pathlib.Path methods, use trio.Path or anyio.path
(ASYNC240)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
[warning] 420-420: Dynamically typed expressions (typing.Any) are disallowed in subagents
(ANN401)
[warning] 426-428: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 433-433: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 436-436: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 438-438: Avoid specifying long messages outside the exception class
(TRY003)
adapters/claude/src/nemo_fabric_adapters/claude/adapter.py
[warning] 32-32: Use from nemo_fabric_adapters.common import relay_gateway in lieu of alias
Replace with from nemo_fabric_adapters.common import relay_gateway
(PLR0402)
[warning] 33-33: Use from nemo_fabric_adapters.common import relay_hooks in lieu of alias
Replace with from nemo_fabric_adapters.common import relay_hooks
(PLR0402)
[warning] 125-125: Dynamically typed expressions (typing.Any) are disallowed in value
(ANN401)
[warning] 135-135: Dynamically typed expressions (typing.Any) are disallowed in value
(ANN401)
[warning] 148-148: Dynamically typed expressions (typing.Any) are disallowed in value
(ANN401)
[warning] 670-670: Unnecessary dict comprehension for iterable; use dict.fromkeys instead
Replace with dict.fromkeys(iterable))
(C420)
[warning] 711-711: Too many branches (22 > 12)
(PLR0912)
[warning] 711-711: Too many statements (56 > 50)
(PLR0915)
tests/adapters/test_claude_adapter.py
[error] 458-458: Possible hardcoded password assigned to: "FABRIC_UNRELATED_SECRET"
(S105)
[error] 460-460: Possible hardcoded password assigned to: "FABRIC_UNRELATED_SECRET"
(S105)
[warning] 506-506: Missing return type annotation for private function query_result
(ANN202)
[warning] 506-506: Unused function argument: prompt
(ARG001)
[warning] 571-571: Missing return type annotation for private function query_result
(ANN202)
[warning] 571-571: Unused function argument: prompt
(ARG001)
[warning] 571-571: Unused function argument: options
(ARG001)
[warning] 629-629: Missing return type annotation for private function query_result
(ANN202)
[warning] 629-629: Missing type annotation for **_
(ANN003)
[warning] 689-689: Missing return type annotation for private function query_failure
(ANN202)
[warning] 689-689: Unused function argument: prompt
(ARG001)
[warning] 689-689: Unused function argument: options
(ARG001)
🔇 Additional comments (34)
crates/fabric-core/src/config.rs (4)
2709-2796: LGTM!
55-103: LGTM!Also applies to: 432-432, 1690-1701
1607-1638: 🗄️ Data Integrity & IntegrationThis concern doesn’t apply: the shipped
hermes,claude, anddeepagentsdescriptors already declaretools.blocked, so the routing change stays aligned with existing adapter support.> Likely an incorrect or invalid review comment.
1920-1922: 🗄️ Data Integrity & IntegrationNothing to change:
ToolsPlan.blockedis already omitted when empty.capability_plan.toolsstill serializes as{}and matches the schema default.> Likely an incorrect or invalid review comment.crates/fabric-core/src/doctor.rs (2)
590-608: Good coverage for fail-closed doctor behavior on unsupported tool policy.Test correctly asserts that an
Unsupportedroute withkind: Toolsescalates the overall report toDoctorStatus::Failrather thanWarn, aligning with the runtime's fail-closed enforcement inruntime.rs.
14-15: LGTM!Also applies to: 477-480
crates/fabric-core/src/runtime.rs (2)
2053-2083: Test correctly validates fail-closed enforcement at runtime start.Confirms
start_runtimerejects blocked-tools policy when the adapter only declares generic"tools"(not"tools.blocked"), returningFabricError::UnsupportedToolsPolicy. Good defense-in-depth alongside the doctor check indoctor.rs.
21-22: LGTM!README.md (2)
50-53: LGTM!Also applies to: 69-75, 77-82, 92-97, 118-120, 146-158, 160-162, 194-196
84-90: 🎯 Functional CorrectnessKeep the Hermes install example as-is
nemo-fabric[hermes]already pulls in bothhermes-agentandnemo-fabric-adapters-hermes; replacing it withnemo-fabric-adapters-hermeswould drop Hermes Agent.> Likely an incorrect or invalid review comment.adapters/claude/README.md (1)
24-33: LGTM!Also applies to: 51-53, 65-65, 77-100, 121-121, 145-145, 191-199
adapters/deepagents/README.md (1)
55-57: LGTM!Also applies to: 72-83
docs/reference/api/rust-library-reference/fabric-core/config/struct-adapterconfigsupport.mdx (1)
20-20: LGTM!schemas/adapter-descriptor.schema.json (1)
8-8: LGTM!Also applies to: 220-220
schemas/run-plan.schema.json (1)
8-8: LGTM!Also applies to: 617-624, 1611-1636, 1751-1751
adapters/hermes/fabric-adapter.json (1)
18-18: 🗄️ Data Integrity & IntegrationSame
tools.blockedaccepts-path verification as the DeepAgents manifest.Same concern raised for
adapters/deepagents/fabric-adapter.json: confirm the descriptor validator/capability resolver recognizes"tools.blocked"as a distinct accepted path.adapters/common/src/nemo_fabric_adapters/common/utils.py (3)
8-8: LGTM!Also applies to: 25-30, 53-56, 61-62, 114-140, 177-184, 196-198, 236-265, 500-537, 541-545
167-198: Line-range metadata conflicts with shown snippet fortools_config/blocked_tools/merge_unique.The line-range change details describe
tools_config,blocked_tools(167-176), andmerge_unique(187-195) as newly added logic, but the annotated snippet marks 142-176 and 185-195 as "unchanged... not shown." Since these functions underpin the entire blocked-tools policy (consumed by Claude/Hermes/DeepAgents adapters per the PR description), their actual implementation should be reviewed directly — particularly whetherblocked_tools/merge_uniquevalidate input types strictly (a past review comment on the Claude adapter flagged exactly this concern formerge_uniquecoercing malformed values).
442-497: 🎯 Functional Correctness | 🏗️ Heavy liftVerify
atifsinks aren't silently dropped underobservability_version == 2.
relay_cli_plugin_configonly converts theatofsection into the sink-based v2 format (config["atof"] = {"enabled": ..., "sinks": ...}); theatifsection is left untouched even thoughconfig["version"]is bumped to2for the whole component. If the external Relay CLI's v2 contract expects sink-based config uniformly, ATIF trajectory output configured underatif.enabled=Truecould be misinterpreted or ignored by a v2 CLI, silently losing trajectory telemetry (the e2e/hermes tests exerciseatof+atiftogether but not through this v2 conversion path).Please confirm with the NeMo Relay CLI v2 contract docs whether
atifrequires an analogous sink transform, or whether it intentionally stays in the legacy shape for v2.adapters/claude/src/nemo_fabric_adapters/claude/adapter.py (1)
14-43: LGTM!Also applies to: 84-106, 121-167, 216-266, 276-334, 337-437, 440-511, 527-572, 575-588, 641-642, 665-708
adapters/deepagents/fabric-adapter.json (1)
12-12: 🗄️ Data Integrity & IntegrationConfirm
acceptsvalidation supports the dottedtools.blockedsub-path.This adds
"tools.blocked"alongside the existing"tools"entry. The same pattern appears in the Claude and Hermes descriptors and is asserted bytest_claude_descriptor_is_narrow_and_versioned, so it looks intentional, but the descriptor-accepts validation logic (schemas/adapter-descriptor.schema.json,crates/fabric-corecapability-plan resolution) isn't in this review's file set. Please confirm the resolver treats"tools.blocked"as a distinct, recognized capability path rather than an unused/no-op string.Based on a retrieved learning,
config.acceptsshould stay limited to top-level Fabric capability sections consumed byresolve_capability_plan; confirming this dotted form is consumed the same way avoids silently-ignored entries.Source: Learnings
tests/adapters/test_claude_adapter.py (1)
6-9: LGTM!Also applies to: 25-25, 30-61, 128-340, 359-415, 418-461, 463-746, 764-772
tests/e2e/test_claude.py (1)
14-27: LGTM!Also applies to: 32-121, 124-148, 151-214, 220-256
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (2)
226-235: Shared middleware factory refactor already applied.Confirms the previously requested extraction (
_tool_gate_middleware) that removed duplication between allow-list and blocked-list middleware is in place.
363-383: LGTM!adapters/hermes/README.md (1)
6-25: LGTM!adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py (2)
128-138: LGTM!
38-43: 🔒 Security & PrivacyNo change needed:
disabled_toolsetsalready wins overenabled_toolsets.
Blocked toolsets cannot be re-enabled through the adapter’senabled_toolsetspath.> Likely an incorrect or invalid review comment.tests/adapters/test_deepagents.py (3)
70-83: LGTM!
414-431: LGTM!
581-636: Good coverage of default-subagent gating and unenforceable-subagent rejection.
test_subagents_are_gated_by_blocked_tools,test_default_subagent_is_gated_by_blocked_tools, andtest_blocked_tools_reject_unenforceable_subagentsdirectly exercise the previously-flagged gap (implicit default subagent bypassingtools.blocked) and the newgraph_id/runnablerejection path. This addresses the prior reviewer's concern about default subagent enforcement.tests/adapters/test_hermes_adapter.py (2)
55-126: LGTM!
327-345: LGTM!adapters/claude/fabric-adapter.json (1)
11-11: 🗄️ Data Integrity & IntegrationNo change needed for
tools.blocked:resolve_capability_planmatchestools.blockedexplicitly, and other adapter descriptors already use the same dotted entry.> Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
schemas/run-plan.schema.json (1)
261-264: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRequire the normalized tools plan in
CapabilityPlan.
defaultdoes not make a JSON Schema property required. A plan that omitstoolscan still validate, contradicting the contract that normalized plans always exposetoolsand use{}when no policy is configured. AddtoolstoCapabilityPlan.requiredand keep Rust serialization aligned.As per path instructions, schemas must encode the first-class normalized
capability_plan.toolsfield.Also applies to: 1674-1674
🤖 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 `@schemas/run-plan.schema.json` around lines 261 - 264, Update the CapabilityPlan schema’s required properties to include tools, while retaining its default {} behavior for omitted policy configuration. Ensure the corresponding Rust serialization model emits the normalized capability_plan.tools field consistently with this required schema contract.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 `@adapters/claude/src/nemo_fabric_adapters/claude/adapter.py`:
- Around line 711-804: Reduce the PLR0912/PLR0915 complexity of run_claude by
extracting relay lifecycle logic into focused helpers, such as _start_relay for
gateway startup and _finalize_relay for gateway shutdown, plugin removal, and
cleanup-error construction. Keep query streaming, session persistence, relay
output wrapping, and cleanup-error merging behavior unchanged while moving the
corresponding branches out of run_claude.
In `@adapters/deepagents/README.md`:
- Around line 84-85: Update the preflight-failure description in the README to
identify the condition as “an invalid or unsupported passthrough option,”
preserving the existing references to missing credentials, the absent package,
and invalid MCP servers.
In `@adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py`:
- Around line 414-445: Narrow the _gated_subagents subagents parameter from Any
to list[Any] | None, matching its existing None-or-list validation and
preserving the AdapterConfigError behavior for other runtime values.
In `@README.md`:
- Line 84: Update the README installation instruction to format the package
extra “nemo-fabric[hermes]” as inline code, while leaving the surrounding
wording unchanged.
In `@tests/adapters/test_hermes_adapter.py`:
- Around line 355-470: Add a regression test covering overlapping tool
configuration, with a name present in both tools.blocked and
settings.enabled_toolsets (and, where applicable, disabled_toolsets). In the
adapter.run_hermes test flow, assert the blocked tool is removed from the
enabled_toolsets passed to AIAgent or otherwise cannot execute, preserving
fail-closed behavior.
---
Outside diff comments:
In `@schemas/run-plan.schema.json`:
- Around line 261-264: Update the CapabilityPlan schema’s required properties to
include tools, while retaining its default {} behavior for omitted policy
configuration. Ensure the corresponding Rust serialization model emits the
normalized capability_plan.tools field consistently with this required schema
contract.
🪄 Autofix (Beta)
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: 73272b4d-8569-4ca5-bf4a-2ceb3c08f1e4
📒 Files selected for processing (21)
README.mdadapters/claude/README.mdadapters/claude/fabric-adapter.jsonadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/deepagents/README.mdadapters/deepagents/fabric-adapter.jsonadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/hermes/README.mdadapters/hermes/fabric-adapter.jsonadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pycrates/fabric-core/src/config.rscrates/fabric-core/src/doctor.rscrates/fabric-core/src/runtime.rsdocs/reference/api/rust-library-reference/fabric-core/config/struct-adapterconfigsupport.mdxschemas/adapter-descriptor.schema.jsonschemas/run-plan.schema.jsontests/adapters/test_claude_adapter.pytests/adapters/test_deepagents.pytests/adapters/test_hermes_adapter.pytests/e2e/test_claude.py
📜 Review details
🔇 Additional comments (34)
crates/fabric-core/src/config.rs (4)
2709-2796: LGTM!
55-103: LGTM!Also applies to: 432-432, 1690-1701
1607-1638: 🗄️ Data Integrity & IntegrationThis concern doesn’t apply: the shipped
hermes,claude, anddeepagentsdescriptors already declaretools.blocked, so the routing change stays aligned with existing adapter support.> Likely an incorrect or invalid review comment.
1920-1922: 🗄️ Data Integrity & IntegrationNothing to change:
ToolsPlan.blockedis already omitted when empty.capability_plan.toolsstill serializes as{}and matches the schema default.> Likely an incorrect or invalid review comment.crates/fabric-core/src/doctor.rs (2)
590-608: Good coverage for fail-closed doctor behavior on unsupported tool policy.Test correctly asserts that an
Unsupportedroute withkind: Toolsescalates the overall report toDoctorStatus::Failrather thanWarn, aligning with the runtime's fail-closed enforcement inruntime.rs.
14-15: LGTM!Also applies to: 477-480
crates/fabric-core/src/runtime.rs (2)
2053-2083: Test correctly validates fail-closed enforcement at runtime start.Confirms
start_runtimerejects blocked-tools policy when the adapter only declares generic"tools"(not"tools.blocked"), returningFabricError::UnsupportedToolsPolicy. Good defense-in-depth alongside the doctor check indoctor.rs.
21-22: LGTM!README.md (2)
50-53: LGTM!Also applies to: 69-75, 77-82, 92-97, 118-120, 146-158, 160-162, 194-196
84-90: 🎯 Functional CorrectnessKeep the Hermes install example as-is
nemo-fabric[hermes]already pulls in bothhermes-agentandnemo-fabric-adapters-hermes; replacing it withnemo-fabric-adapters-hermeswould drop Hermes Agent.> Likely an incorrect or invalid review comment.adapters/claude/README.md (1)
24-33: LGTM!Also applies to: 51-53, 65-65, 77-100, 121-121, 145-145, 191-199
adapters/deepagents/README.md (1)
55-57: LGTM!Also applies to: 72-83
docs/reference/api/rust-library-reference/fabric-core/config/struct-adapterconfigsupport.mdx (1)
20-20: LGTM!schemas/adapter-descriptor.schema.json (1)
8-8: LGTM!Also applies to: 220-220
schemas/run-plan.schema.json (1)
8-8: LGTM!Also applies to: 617-624, 1611-1636, 1751-1751
adapters/hermes/fabric-adapter.json (1)
18-18: 🗄️ Data Integrity & IntegrationSame
tools.blockedaccepts-path verification as the DeepAgents manifest.Same concern raised for
adapters/deepagents/fabric-adapter.json: confirm the descriptor validator/capability resolver recognizes"tools.blocked"as a distinct accepted path.adapters/common/src/nemo_fabric_adapters/common/utils.py (3)
8-8: LGTM!Also applies to: 25-30, 53-56, 61-62, 114-140, 177-184, 196-198, 236-265, 500-537, 541-545
167-198: Line-range metadata conflicts with shown snippet fortools_config/blocked_tools/merge_unique.The line-range change details describe
tools_config,blocked_tools(167-176), andmerge_unique(187-195) as newly added logic, but the annotated snippet marks 142-176 and 185-195 as "unchanged... not shown." Since these functions underpin the entire blocked-tools policy (consumed by Claude/Hermes/DeepAgents adapters per the PR description), their actual implementation should be reviewed directly — particularly whetherblocked_tools/merge_uniquevalidate input types strictly (a past review comment on the Claude adapter flagged exactly this concern formerge_uniquecoercing malformed values).
442-497: 🎯 Functional Correctness | 🏗️ Heavy liftVerify
atifsinks aren't silently dropped underobservability_version == 2.
relay_cli_plugin_configonly converts theatofsection into the sink-based v2 format (config["atof"] = {"enabled": ..., "sinks": ...}); theatifsection is left untouched even thoughconfig["version"]is bumped to2for the whole component. If the external Relay CLI's v2 contract expects sink-based config uniformly, ATIF trajectory output configured underatif.enabled=Truecould be misinterpreted or ignored by a v2 CLI, silently losing trajectory telemetry (the e2e/hermes tests exerciseatof+atiftogether but not through this v2 conversion path).Please confirm with the NeMo Relay CLI v2 contract docs whether
atifrequires an analogous sink transform, or whether it intentionally stays in the legacy shape for v2.adapters/claude/src/nemo_fabric_adapters/claude/adapter.py (1)
14-43: LGTM!Also applies to: 84-106, 121-167, 216-266, 276-334, 337-437, 440-511, 527-572, 575-588, 641-642, 665-708
adapters/deepagents/fabric-adapter.json (1)
12-12: 🗄️ Data Integrity & IntegrationConfirm
acceptsvalidation supports the dottedtools.blockedsub-path.This adds
"tools.blocked"alongside the existing"tools"entry. The same pattern appears in the Claude and Hermes descriptors and is asserted bytest_claude_descriptor_is_narrow_and_versioned, so it looks intentional, but the descriptor-accepts validation logic (schemas/adapter-descriptor.schema.json,crates/fabric-corecapability-plan resolution) isn't in this review's file set. Please confirm the resolver treats"tools.blocked"as a distinct, recognized capability path rather than an unused/no-op string.Based on a retrieved learning,
config.acceptsshould stay limited to top-level Fabric capability sections consumed byresolve_capability_plan; confirming this dotted form is consumed the same way avoids silently-ignored entries.Source: Learnings
tests/adapters/test_claude_adapter.py (1)
6-9: LGTM!Also applies to: 25-25, 30-61, 128-340, 359-415, 418-461, 463-746, 764-772
tests/e2e/test_claude.py (1)
14-27: LGTM!Also applies to: 32-121, 124-148, 151-214, 220-256
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (2)
226-235: Shared middleware factory refactor already applied.Confirms the previously requested extraction (
_tool_gate_middleware) that removed duplication between allow-list and blocked-list middleware is in place.
363-383: LGTM!adapters/hermes/README.md (1)
6-25: LGTM!adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py (2)
128-138: LGTM!
38-43: 🔒 Security & PrivacyNo change needed:
disabled_toolsetsalready wins overenabled_toolsets.
Blocked toolsets cannot be re-enabled through the adapter’senabled_toolsetspath.> Likely an incorrect or invalid review comment.tests/adapters/test_deepagents.py (3)
70-83: LGTM!
414-431: LGTM!
581-636: Good coverage of default-subagent gating and unenforceable-subagent rejection.
test_subagents_are_gated_by_blocked_tools,test_default_subagent_is_gated_by_blocked_tools, andtest_blocked_tools_reject_unenforceable_subagentsdirectly exercise the previously-flagged gap (implicit default subagent bypassingtools.blocked) and the newgraph_id/runnablerejection path. This addresses the prior reviewer's concern about default subagent enforcement.tests/adapters/test_hermes_adapter.py (2)
55-126: LGTM!
327-345: LGTM!adapters/claude/fabric-adapter.json (1)
11-11: 🗄️ Data Integrity & IntegrationNo change needed for
tools.blocked:resolve_capability_planmatchestools.blockedexplicitly, and other adapter descriptors already use the same dotted entry.> Likely an incorrect or invalid review comment.
🛑 Comments failed to post (5)
adapters/claude/src/nemo_fabric_adapters/claude/adapter.py (1)
711-804: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift
run_claudecomplexity exceeds lint thresholds (22 branches, 56 statements).The function interleaves relay start/stop, query streaming, session persistence, and multi-stage cleanup-error merging in one body. This is a lifecycle-critical path (gateway process leaks, plugin cleanup, session mismatch) where added complexity increases the risk of subtle regressions on future edits. Consider extracting the relay-start/relay-stop-and-cleanup blocks into dedicated helpers (e.g.,
_start_relay(...),_finalize_relay(...)) to bring branch/statement counts down and isolate the error-merging logic from the query loop.
Based on learnings and static analysis, ruff flagsPLR0912/PLR0915on this function.🧰 Tools
🪛 Ruff (0.15.21)
[warning] 711-711: Too many branches (22 > 12)
(PLR0912)
[warning] 711-711: Too many statements (56 > 50)
(PLR0915)
🤖 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 `@adapters/claude/src/nemo_fabric_adapters/claude/adapter.py` around lines 711 - 804, Reduce the PLR0912/PLR0915 complexity of run_claude by extracting relay lifecycle logic into focused helpers, such as _start_relay for gateway startup and _finalize_relay for gateway shutdown, plugin removal, and cleanup-error construction. Keep query streaming, session persistence, relay output wrapping, and cleanup-error merging behavior unchanged while moving the corresponding branches out of run_claude.Source: Linters/SAST tools
adapters/deepagents/README.md (1)
84-85: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Qualify the passthrough failure condition.
The text implies that any passthrough option is a failure, but supported options are documented above. Change this to “an invalid or unsupported passthrough option.”
As per path instructions, documentation must align with current repository behavior and public API.
🤖 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 `@adapters/deepagents/README.md` around lines 84 - 85, Update the preflight-failure description in the README to identify the condition as “an invalid or unsupported passthrough option,” preserving the existing references to missing credentials, the absent package, and invalid MCP servers.Source: Path instructions
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (1)
414-445: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Default subagent gating now addressed.
_gated_subagentsnow injects a gatedgeneral-purposesubagent when none is explicitly configured, and rejectsgraph_id/runnablesubagents outright — this resolves the earlier gap where the implicit default subagent could bypasstools.blocked.One residual nit: the
subagentsparameter is typedAny(flagged by Ruff ANN401), while the function body's own logic already assumes/validates it'sNone | list. A narrower type (list[Any] | None) would match the runtime checks without weakening validation.🧰 Tools
🪛 Ruff (0.15.21)
[warning] 420-420: Dynamically typed expressions (typing.Any) are disallowed in
subagents(ANN401)
[warning] 426-428: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 433-433: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 436-436: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 438-438: Avoid specifying long messages outside the exception class
(TRY003)
🤖 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 `@adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py` around lines 414 - 445, Narrow the _gated_subagents subagents parameter from Any to list[Any] | None, matching its existing None-or-list validation and preserving the AdapterConfigError behavior for other runtime values.Source: Linters/SAST tools
README.md (1)
84-84: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Format the package extra as inline code.
As per coding guidelines, package names must use inline code formatting.
Suggested wording
-Install Hermes into its own environment (the nemo-fabric[hermes] extra will install Hermes Agent, and the Hermes Agent adapter but not Fabric itself): +Install Hermes into its own environment (the `nemo-fabric[hermes]` extra will install Hermes Agent and the Hermes Agent adapter but not Fabric itself):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.Install Hermes into its own environment (the `nemo-fabric[hermes]` extra will install Hermes Agent and the Hermes Agent adapter but not Fabric itself):🤖 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 `@README.md` at line 84, Update the README installation instruction to format the package extra “nemo-fabric[hermes]” as inline code, while leaving the surrounding wording unchanged.Source: Coding guidelines
tests/adapters/test_hermes_adapter.py (1)
355-470: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Consider adding coverage for the enabled/disabled toolset overlap case.
None of the tests exercise
tools.blockedtogether with an overlappingenabled_toolsets/disabled_toolsets(e.g., a tool name present in bothtools.blockedandsettings.enabled_toolsets). Given the concern raised inadapter.pyaboutAIAgentprecedence between these two kwargs, a regression test asserting the blocked name is excluded from what's ultimately passed asenabled_toolsets(or otherwise proven non-executable) would directly validate fail-closed behavior.🧰 Tools
🪛 Ruff (0.15.21)
[warning] 388-388: Prefer
dictover useless lambdaReplace with
lambdawithdict(PIE807)
[warning] 390-390: Unused lambda argument:
force(ARG005)
[warning] 391-391: Unused lambda argument:
args(ARG005)
[warning] 391-391: Unused lambda argument:
kwargs(ARG005)
🤖 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 `@tests/adapters/test_hermes_adapter.py` around lines 355 - 470, Add a regression test covering overlapping tool configuration, with a name present in both tools.blocked and settings.enabled_toolsets (and, where applicable, disabled_toolsets). In the adapter.run_hermes test flow, assert the blocked tool is removed from the enabled_toolsets passed to AIAgent or otherwise cannot execute, preserving fail-closed behavior.
Signed-off-by: Anuradha Karuppiah <26330987+AnuradhaKaruppiah@users.noreply.github.com>
Great review. Addressed all comments. |
|
/merge |
2 participants
Overview
Adds first-class blocked tool policy across Fabric config, SDK authoring, run-plan routing, and adapters that can enforce it.
Canonical config shape:
Configuration note
The adapter-level
harness.settings.disallowed_toolssetting has been dropped. Configure the deny-list only through the typed, normalizedtools.blockedfield; the Claude adapter maps it internally to the Claude SDK'sdisallowed_toolsoption.Adapter mappings:
disallowed_tools.Unsupported harness handling
An adapter that cannot enforce blocked tools must omit
toolsfrom its descriptor'sconfig.accepts. Core preserves the configured policy and routes the tools capability tocapability_plan.unsupportedso the mismatch is explicit.Blocked-tool policy must be handled fail-closed: planning and doctor diagnostics should identify the unsupported adapter capability, and invocation must not silently ignore the deny-list. The user must remove
tools.blockedor select an adapter that declares and implements tools support.This PR is stacked on #54.
Where should the reviewer start?
crates/fabric-core/src/config.rspython/src/nemo_fabric/models.pyadapters/common/src/nemo_fabric_adapters/common/utils.pyValidation
cargo test -p fabric-coreruff checkon touched Python filescargo fmt --checkgit diff --checkRelated Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Closes FABRIC-74
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
New Features
tools.blockedconfiguration for disabling named tools and toolsets.FabricConfig.block_tools()for programmatically blocking tools.Documentation