feat: add explicit plugin config file override#230
Conversation
…IG_FILE. - Added it to both daemon startup args and nemo-relay run. - It loads TOML from the exact file path. - It skips normal discovered plugins.toml loading when present. - It conflicts with --plugin-config. - [plugins].config in config.toml still conflicts with plugin TOML sources, including the explicit file. - Missing or invalid plugin TOML produces a startup config error. Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
github-actions
Bot
added
size:M
WalkthroughThis PR adds a ChangesPlugin config file CLI pathway
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
bbednarski9
changed the title
willkill07
left a comment
willkill07
left a comment
There was a problem hiding this comment.
Please coordinate with @zhongxuanwang-nv on this as it has overlap (conflicts) with #211
| #[arg(long, env = "NEMO_RELAY_PLUGIN_CONFIG_FILE")] | ||
| pub(crate) plugin_config_file: Option<PathBuf>, |
There was a problem hiding this comment.
| #[arg(long, env = "NEMO_RELAY_PLUGIN_CONFIG_FILE")] | |
| pub(crate) plugin_config_file: Option<PathBuf>, | |
| #[arg(long, env = "NEMO_RELAY_PLUGIN_OVERRIDE_CONFIG_FILE")] | |
| pub(crate) plugin_override_config_file: Option<PathBuf>, |
I would have the name be more descriptive indicating that it overrides rather than layers.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
crates/cli/src/config.rs (1)
464-491: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueParentheses would clarify the operator precedence.
The condition on lines 475-477 relies on
&&binding tighter than||. While correct, explicit parentheses would make the intent clearer at a glance.Optional: explicit grouping
- if command.plugin_config.is_some() && args.plugin_config.is_some() - || command.plugin_config_file.is_some() && args.plugin_config_file.is_some() + if (command.plugin_config.is_some() && args.plugin_config.is_some()) + || (command.plugin_config_file.is_some() && args.plugin_config_file.is_some())🤖 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/cli/src/config.rs` around lines 464 - 491, The boolean condition combining || and && in the if that checks command.plugin_config/is_some and command.plugin_config_file/is_some vs args.* is correct but unclear; update the expression in the if inside apply_server_overrides handling to add explicit parentheses around the two &&-combined clauses—i.e. group (command.plugin_config.is_some() && args.plugin_config.is_some()) and (command.plugin_config_file.is_some() && args.plugin_config_file.is_some()) and then OR those groups—so the intent is obvious when reading the code (look for the if that references command.plugin_config, args.plugin_config, command.plugin_config_file, and args.plugin_config_file).
🤖 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 `@crates/cli/src/config.rs`:
- Around line 769-799: The error text "invalid plugin TOML shape" in
load_plugin_toml_config_from_path is misleading because the failure comes from
serde_json::to_value (JSON conversion) rather than TOML parsing; update the
CliError message produced around the serde_json::to_value call to clearly state
it's a JSON serialization/conversion error (e.g., "failed converting plugin TOML
to JSON" or "plugin TOML could not be converted to JSON"), keeping the same
error interpolation of {error} and leaving the rest of the function flow
unchanged.
In `@crates/cli/tests/coverage/config_tests.rs`:
- Around line 671-704: Update the test
plugin_config_file_overrides_discovered_plugins_toml to explicitly document that
it only verifies precedence with minimal component shapes (string elements), and
add a companion test that uses full component objects ({ kind, enabled, config
}) to ensure real-world component shape is preserved when plugin_config_file
overrides discovered plugins; locate references to
load_plugin_toml_config_from_path and validate_plugin_toml_component_kinds to
ensure the new test exercises the same code-paths and assert the resulting
resolved.gateway.plugin_config contains the full object structure, mirroring the
existing assertion style.
In `@docs/build-plugins/plugin-configuration-files.mdx`:
- Around line 87-90: Add a clear sentence after the paragraph about
`--plugin-config-file`/`NEMO_RELAY_PLUGIN_CONFIG_FILE` stating that unlike
discovered `plugins.toml` files (which are skipped when missing), an explicitly
specified plugin file must exist and contain valid TOML or the process will fail
with a startup configuration error; reference the exact flags
`--plugin-config-file` and `NEMO_RELAY_PLUGIN_CONFIG_FILE` and contrast this
behavior with the "Missing files are skipped" note for discovered
`plugins.toml`.
---
Outside diff comments:
In `@crates/cli/src/config.rs`:
- Around line 464-491: The boolean condition combining || and && in the if that
checks command.plugin_config/is_some and command.plugin_config_file/is_some vs
args.* is correct but unclear; update the expression in the if inside
apply_server_overrides handling to add explicit parentheses around the two
&&-combined clauses—i.e. group (command.plugin_config.is_some() &&
args.plugin_config.is_some()) and (command.plugin_config_file.is_some() &&
args.plugin_config_file.is_some()) and then OR those groups—so the intent is
obvious when reading the code (look for the if that references
command.plugin_config, args.plugin_config, command.plugin_config_file, and
args.plugin_config_file).
🪄 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: 36a1c1aa-d9ab-4d45-868d-8e02431d461c
📒 Files selected for processing (5)
crates/cli/src/config.rscrates/cli/src/launcher.rscrates/cli/tests/coverage/config_tests.rscrates/cli/tests/coverage/launcher_tests.rsdocs/build-plugins/plugin-configuration-files.mdx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (22)
{docs/**,README.md,CONTRIBUTING.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
{docs/**,README.md,CONTRIBUTING.md}: For docs-only changes, run targeted checks only if commands, package names, or examples changed. Usejust docsfor docs-site builds andjust docs-linkcheckwhen links changed
Run docs site build withjust docs
Files:
docs/build-plugins/plugin-configuration-files.mdx
{docs/**,README.md,CONTRIBUTING.md,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Run docs link validation with
just docs-linkcheckwhen links change
Files:
docs/build-plugins/plugin-configuration-files.mdx
{docs/**,README.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Verify README and docs entry points still match current package names and paths for large or public-facing changes
Files:
docs/build-plugins/plugin-configuration-files.mdx
{docs/**,examples/**,README.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Verify examples still run with documented commands for large or public-facing changes
Files:
docs/build-plugins/plugin-configuration-files.mdx
{docs/**,README.md,**/Cargo.toml,**/package.json,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Ensure renamed public surfaces are reflected consistently in manifests and docs for large or public-facing changes
Files:
docs/build-plugins/plugin-configuration-files.mdx
**/*.{md,mdx,py,sh,yaml,yml,toml,json}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repo references, and build commands current
Files:
docs/build-plugins/plugin-configuration-files.mdx
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
In MDX files, top-of-file comments must use JSX comment delimiters: {/* to open and */} to close. Do not use HTML comments for MDX SPDX headers.
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
Files:
docs/build-plugins/plugin-configuration-files.mdx
**/*.{html,md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license header in HTML and Markdown files using HTML comment syntax
Files:
docs/build-plugins/plugin-configuration-files.mdx
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update embedded documentation snippets, patch docs, and binding-support notes if examples or supported bindings changed
Files:
docs/build-plugins/plugin-configuration-files.mdx
docs/**
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run
just docsor./scripts/build-docs.sh htmlto regenerate ignored Fern API reference pages before validation for documentation site changes
Files:
docs/build-plugins/plugin-configuration-files.mdx
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.
Flag stale examples, missing SPDX headers where required, and instructions that no longer match CI or pre-commit behavior.
Files:
docs/build-plugins/plugin-configuration-files.mdx
**
⚙️ CodeRabbit configuration file
**:AGENTS.md
This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.
Project Overview
NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go, WebAssembly, and the raw C FFI are experimental and source-first.
The shared runtime model is:
- Scope stacks decide where work belongs and which scope-local behavior is visible.
- Middleware registries decide what guardrails and intercepts run around managed calls.
- Plugins install reusable runtime behavior from configuration.
- Events record runtime behavior in ATOF form.
- Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.
Repository Structure
The repository layout separates the Rust runtime, language bindings, documentation,
integration patches, and agent-facing skills.crates/ core/ # Rust core runtime crate, published as nemo-relay adaptive/ # Adaptive runtime primitives and plugin components python/ # PyO3 native extension for the Python package ffi/ # Raw C ABI layer used by downstream bindings such as Go node/ # NAPI Node.js binding and JavaScript/TypeScript entry points wasm/ # wasm-bindgen WebAssembly binding and JS wrappers python/ nemo_relay/ # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers tests/ # Python tests go/ nemo_relay/ # Experimental Go CGo binding and tests fern/ # Fern documentation site scripts/ # Stable wrappers and helper scripts; build/test/docs entry points live in justfile third_party/ # P...
Files:
docs/build-plugins/plugin-configuration-files.mdxcrates/cli/src/launcher.rscrates/cli/tests/coverage/config_tests.rscrates/cli/tests/coverage/launcher_tests.rscrates/cli/src/config.rs
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Use
snake_casenaming convention for Rust identifiers (e.g.,nemo_relay_tool_call)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node work
**/*.rs: Runcargo fmt --allto format all Rust code
Runcargo clippy --workspace --all-targets -- -D warningsto enforce all clippy lints as errors
**/*.rs: Runcargo fmt --allwhen Rust files changed as part of WebAssembly work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files changed as part of WebAssembly work
**/*.rs: If any Rust code changed, always runjust test-rust
If any Rust code changed, also runcargo fmt --all
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings
Run Rust formatting withcargo fmt --all
Run Rust linting withcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Usecargo fmtfor Rust code formatting
Runcargo clippy -- -D warningsto lint Rust code and treat all warnings as errors
Use Rust snake_case naming convention for Rust identifiers
Include SPDX license header in all Rust source files using double-slash comment syntax
Validate Rust code withuv run pre-commit run --all-filesto enforce cargo fmt formatting check, cargo clippy lints, and cargo deny aud...
Files:
crates/cli/src/launcher.rscrates/cli/tests/coverage/config_tests.rscrates/cli/tests/coverage/launcher_tests.rscrates/cli/src/config.rs
**/{Cargo.toml,**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Maintain consistency between Rust package names in
Cargo.tomland their actual usage across the codebase
Files:
crates/cli/src/launcher.rscrates/cli/tests/coverage/config_tests.rscrates/cli/tests/coverage/launcher_tests.rscrates/cli/src/config.rs
**/*.{h,hpp,c,cpp,rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Ensure FFI header and library naming follows consistent conventions across platform-specific builds
Files:
crates/cli/src/launcher.rscrates/cli/tests/coverage/config_tests.rscrates/cli/tests/coverage/launcher_tests.rscrates/cli/src/config.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Rust crate names and module prefixes during coordinated rename operations
Files:
crates/cli/src/launcher.rscrates/cli/tests/coverage/config_tests.rscrates/cli/tests/coverage/launcher_tests.rscrates/cli/src/config.rs
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.
Files:
crates/cli/src/launcher.rscrates/cli/tests/coverage/config_tests.rscrates/cli/tests/coverage/launcher_tests.rscrates/cli/src/config.rs
**/*.{rs,py,go,js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions: Rust and Python use
snake_case, C FFI exports prefixednemo_relay_, Go usesPascalCasefor public APIs, Node.js usescamelCase.
Files:
crates/cli/src/launcher.rscrates/cli/tests/coverage/config_tests.rscrates/cli/tests/coverage/launcher_tests.rscrates/cli/src/config.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: Keep async behavior on the existing tokio-based model. Bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
UseJson = serde_json::Valuein Rust-facing runtime APIs for JSON payload handling.
Files:
crates/cli/src/launcher.rscrates/cli/tests/coverage/config_tests.rscrates/cli/tests/coverage/launcher_tests.rscrates/cli/src/config.rs
**/*config*.{rs,ts,py,go,js,json,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Ensure dynamic config shape still matches the documented canonical model
Files:
crates/cli/tests/coverage/config_tests.rscrates/cli/src/config.rs
{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Maintain documented and tested validation and report behavior for adaptive surfaces
Files:
crates/cli/tests/coverage/config_tests.rscrates/cli/tests/coverage/launcher_tests.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/cli/tests/coverage/config_tests.rscrates/cli/tests/coverage/launcher_tests.rs
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/maintain-optimizer/SKILL.md:0-0
Timestamp: 2026-05-21T22:49:09.811Z
Learning: Applies to {crates/adaptive/**,python/nemo_relay/plugin.py,go/nemo_relay/**,**/node/**,**/wasm/**} : Maintain consistent plugin lifecycle across all language bindings (Python, Go, Node/WebAssembly, and Rust)
📚 Learning: 2026-05-26T21:03:12.012Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/review-doc-style/SKILL.md:0-0
Timestamp: 2026-05-26T21:03:12.012Z
Learning: Keep documentation aligned with current NeMo Relay behavior, repo layout, and entry points
Applied to files:
docs/build-plugins/plugin-configuration-files.mdx
📚 Learning: 2026-05-26T21:03:12.012Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/review-doc-style/SKILL.md:0-0
Timestamp: 2026-05-26T21:03:12.012Z
Learning: Applies to **/README.md|docs/index.md|python/nemo_relay/README.md|crates/*/README.md : Public behavior changes must be reflected in corresponding entry-point documentation (Must-Fix)
Applied to files:
docs/build-plugins/plugin-configuration-files.mdx
📚 Learning: 2026-05-21T22:49:09.811Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/maintain-optimizer/SKILL.md:0-0
Timestamp: 2026-05-21T22:49:09.811Z
Learning: Applies to {crates/adaptive/**,python/nemo_relay/plugin.py,go/nemo_relay/**,**/node/**,**/wasm/**} : Maintain consistent plugin lifecycle across all language bindings (Python, Go, Node/WebAssembly, and Rust)
Applied to files:
docs/build-plugins/plugin-configuration-files.mdxcrates/cli/src/config.rs
📚 Learning: 2026-05-26T21:03:12.012Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/review-doc-style/SKILL.md:0-0
Timestamp: 2026-05-26T21:03:12.012Z
Learning: Applies to **/README.md|docs/index.md|python/nemo_relay/README.md|crates/*/README.md : Update entry-point documentation (README.md, docs/index.md, package/crate READMEs, and binding-level source READMEs) whenever public behavior changes
Applied to files:
docs/build-plugins/plugin-configuration-files.mdx
📚 Learning: 2026-05-21T22:47:33.109Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/add-binding-feature/SKILL.md:0-0
Timestamp: 2026-05-21T22:47:33.109Z
Learning: Applies to go/nemo_relay/**/*.go : Update Go wrapper in `go/nemo_relay/nemo_relay.go` with doc comment and shorthand package if the capability belongs there
Applied to files:
docs/build-plugins/plugin-configuration-files.mdx
📚 Learning: 2026-05-21T22:50:51.194Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/test-ffi-surface/SKILL.md:0-0
Timestamp: 2026-05-21T22:50:51.194Z
Learning: Applies to **/*.rs : Run `just test-rust` to validate FFI changes
Applied to files:
crates/cli/tests/coverage/config_tests.rscrates/cli/tests/coverage/launcher_tests.rs
📚 Learning: 2026-05-21T22:49:35.949Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/prepare-pr/SKILL.md:0-0
Timestamp: 2026-05-21T22:49:35.949Z
Learning: Applies to **/*.rs : Any Rust change must run `just test-rust`
Applied to files:
crates/cli/tests/coverage/config_tests.rs
📚 Learning: 2026-05-21T22:51:04.073Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/test-go-binding/SKILL.md:0-0
Timestamp: 2026-05-21T22:51:04.073Z
Learning: Applies to crates/ffi/**/*.rs : If the change touched `crates/ffi`, also use `test-ffi-surface` for validation
Applied to files:
crates/cli/tests/coverage/config_tests.rscrates/cli/tests/coverage/launcher_tests.rs
📚 Learning: 2026-05-21T22:51:21.017Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/test-python-binding/SKILL.md:0-0
Timestamp: 2026-05-21T22:51:21.017Z
Learning: Applies to crates/python/**/*.rs : If the native Rust bridge changed, add the Rust crate tests for `nemo-relay-python`
Applied to files:
crates/cli/tests/coverage/launcher_tests.rs
📚 Learning: 2026-05-21T22:52:14.330Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/validate-change/SKILL.md:0-0
Timestamp: 2026-05-21T22:52:14.330Z
Learning: For core runtime or shared semantics changes, use `test-rust-core` which includes Rust testing, formatting, linting, and full cross-language matrix
Applied to files:
crates/cli/tests/coverage/launcher_tests.rs
📚 Learning: 2026-05-21T22:51:21.017Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/test-python-binding/SKILL.md:0-0
Timestamp: 2026-05-21T22:51:21.017Z
Learning: If any Rust files changed as part of the Python work, also run `cargo fmt --all`, `just test-rust`, and `cargo clippy --workspace --all-targets -- -D warnings`
Applied to files:
crates/cli/tests/coverage/launcher_tests.rs
📚 Learning: 2026-05-21T22:48:57.484Z
Learnt from: CR
Repo: NVIDIA/NeMo-Relay PR: 0
File: .agents/skills/maintain-observability/SKILL.md:0-0
Timestamp: 2026-05-21T22:48:57.484Z
Learning: When event fields change, run Rust crate tests and execute `just test-rust` to validate the affected observability modules
Applied to files:
crates/cli/tests/coverage/launcher_tests.rs
🔇 Additional comments (17)
docs/build-plugins/plugin-configuration-files.mdx (2)
83-85: LGTM!
261-265: LGTM!crates/cli/src/config.rs (7)
201-204: LGTM!
212-219: LGTM!
275-277: LGTM!
440-449: LGTM!
551-604: LGTM!
861-897: LGTM!
1-3: Run Rust validation commands before merge.Per coding guidelines, ensure the following pass:
cargo fmt --all cargo clippy --workspace --all-targets -- -D warnings just test-rustcrates/cli/tests/coverage/config_tests.rs (6)
128-128: LGTM!Also applies to: 184-184, 238-238, 540-540, 571-571, 607-607, 633-633, 656-656, 848-848
706-720: LGTM!
722-747: LGTM!
749-773: LGTM!
775-806: LGTM!
808-836: LGTM!crates/cli/tests/coverage/launcher_tests.rs (1)
22-22: LGTM!Also applies to: 56-56, 84-84, 105-105, 131-131, 155-155, 177-177, 721-721, 763-763
crates/cli/src/launcher.rs (1)
79-79: LGTM!
| fn load_plugin_config_file_override( | ||
| path: Option<&PathBuf>, | ||
| ) -> Result<Option<PluginTomlConfig>, CliError> { | ||
| path.map(|path| load_plugin_toml_config_from_path(path)) | ||
| .transpose() | ||
| } | ||
|
|
||
| fn load_plugin_toml_config_from_path(path: &Path) -> Result<PluginTomlConfig, CliError> { | ||
| let raw = std::fs::read_to_string(path).map_err(|error| { | ||
| CliError::Config(format!( | ||
| "could not read plugin config file {}: {error}", | ||
| path.display() | ||
| )) | ||
| })?; | ||
| let parsed = raw | ||
| .parse::<toml::Table>() | ||
| .map(toml::Value::Table) | ||
| .map_err(|error| { | ||
| CliError::Config(format!( | ||
| "invalid plugin TOML in {}: {error}", | ||
| path.display() | ||
| )) | ||
| })?; | ||
| validate_plugin_toml_component_kinds(path, &parsed)?; | ||
| let value = serde_json::to_value(parsed) | ||
| .map_err(|error| CliError::Config(format!("invalid plugin TOML shape: {error}")))?; | ||
| Ok(PluginTomlConfig { | ||
| value, | ||
| sources: vec![path.to_path_buf()], | ||
| }) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Error message at line 794 could be more precise.
TOML parsing succeeds before this point; the error is from serde_json::to_value. "invalid plugin TOML shape" is slightly misleading since the issue is JSON conversion, not TOML structure. This failure path is extremely rare in practice, so low priority.
Optional: clarify error source
let value = serde_json::to_value(parsed)
- .map_err(|error| CliError::Config(format!("invalid plugin TOML shape: {error}")))?;
+ .map_err(|error| CliError::Config(format!("could not convert plugin TOML to JSON: {error}")))?;📝 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.
| fn load_plugin_config_file_override( | |
| path: Option<&PathBuf>, | |
| ) -> Result<Option<PluginTomlConfig>, CliError> { | |
| path.map(|path| load_plugin_toml_config_from_path(path)) | |
| .transpose() | |
| } | |
| fn load_plugin_toml_config_from_path(path: &Path) -> Result<PluginTomlConfig, CliError> { | |
| let raw = std::fs::read_to_string(path).map_err(|error| { | |
| CliError::Config(format!( | |
| "could not read plugin config file {}: {error}", | |
| path.display() | |
| )) | |
| })?; | |
| let parsed = raw | |
| .parse::<toml::Table>() | |
| .map(toml::Value::Table) | |
| .map_err(|error| { | |
| CliError::Config(format!( | |
| "invalid plugin TOML in {}: {error}", | |
| path.display() | |
| )) | |
| })?; | |
| validate_plugin_toml_component_kinds(path, &parsed)?; | |
| let value = serde_json::to_value(parsed) | |
| .map_err(|error| CliError::Config(format!("invalid plugin TOML shape: {error}")))?; | |
| Ok(PluginTomlConfig { | |
| value, | |
| sources: vec![path.to_path_buf()], | |
| }) | |
| } | |
| fn load_plugin_config_file_override( | |
| path: Option<&PathBuf>, | |
| ) -> Result<Option<PluginTomlConfig>, CliError> { | |
| path.map(|path| load_plugin_toml_config_from_path(path)) | |
| .transpose() | |
| } | |
| fn load_plugin_toml_config_from_path(path: &Path) -> Result<PluginTomlConfig, CliError> { | |
| let raw = std::fs::read_to_string(path).map_err(|error| { | |
| CliError::Config(format!( | |
| "could not read plugin config file {}: {error}", | |
| path.display() | |
| )) | |
| })?; | |
| let parsed = raw | |
| .parse::<toml::Table>() | |
| .map(toml::Value::Table) | |
| .map_err(|error| { | |
| CliError::Config(format!( | |
| "invalid plugin TOML in {}: {error}", | |
| path.display() | |
| )) | |
| })?; | |
| validate_plugin_toml_component_kinds(path, &parsed)?; | |
| let value = serde_json::to_value(parsed) | |
| .map_err(|error| CliError::Config(format!("could not convert plugin TOML to JSON: {error}")))?; | |
| Ok(PluginTomlConfig { | |
| value, | |
| sources: vec![path.to_path_buf()], | |
| }) | |
| } |
🤖 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/cli/src/config.rs` around lines 769 - 799, The error text "invalid
plugin TOML shape" in load_plugin_toml_config_from_path is misleading because
the failure comes from serde_json::to_value (JSON conversion) rather than TOML
parsing; update the CliError message produced around the serde_json::to_value
call to clearly state it's a JSON serialization/conversion error (e.g., "failed
converting plugin TOML to JSON" or "plugin TOML could not be converted to
JSON"), keeping the same error interpolation of {error} and leaving the rest of
the function flow unchanged.
| #[test] | ||
| fn plugin_config_file_overrides_discovered_plugins_toml() { | ||
| let temp = tempfile::tempdir().unwrap(); | ||
| let config_path = temp.path().join("config.toml"); | ||
| let explicit_plugin_path = temp.path().join("explicit-plugins.toml"); | ||
| std::fs::write(&config_path, "").unwrap(); | ||
| std::fs::write( | ||
| temp.path().join("plugins.toml"), | ||
| r#"version = 1 | ||
| components = ["discovered"] | ||
| "#, | ||
| ) | ||
| .unwrap(); | ||
| std::fs::write( | ||
| &explicit_plugin_path, | ||
| r#"version = 1 | ||
| components = ["explicit"] | ||
| "#, | ||
| ) | ||
| .unwrap(); | ||
| let args = ServerArgs { | ||
| config: Some(config_path), | ||
| plugin_config_file: Some(explicit_plugin_path), | ||
| ..ServerArgs::default() | ||
| }; | ||
|
|
||
| let resolved = resolve_server_config(&args).unwrap(); | ||
|
|
||
| assert_eq!( | ||
| resolved.gateway.plugin_config, | ||
| Some(json!({ "version": 1, "components": ["explicit"] })) | ||
| ); | ||
| assert!(args.requested_daemon_mode()); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for plugin config schema validation to understand when component structure is validated
rg -n -C3 'kind.*observability|validate.*component.*kind|component.*schema' --type=rust crates/cli/src/Repository: NVIDIA/NeMo-Relay
Length of output: 1469
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect plugin TOML validation to see what schema is required for components.
sed -n '760,880p' crates/cli/src/config.rs
echo "----"
sed -n '980,1065p' crates/cli/src/config.rsRepository: NVIDIA/NeMo-Relay
Length of output: 6443
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect plugin TOML validation to see what schema is required for components.
sed -n '760,880p' crates/cli/src/config.rs
echo "----"
sed -n '980,1065p' crates/cli/src/config.rsRepository: NVIDIA/NeMo-Relay
Length of output: 6443
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the canonical component structure referenced by the review
sed -n '220,290p' crates/cli/tests/coverage/config_tests.rs
echo "----"
# Find additional schema validation / deserialization for plugin TOML components
rg -n "PluginTomlConfig|components.*(kind|enabled|config)|validate_.*component|deserialize_.*component" crates/cli/src/Repository: NVIDIA/NeMo-Relay
Length of output: 4125
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the canonical component structure referenced by the review
sed -n '220,290p' crates/cli/tests/coverage/config_tests.rs
echo "----"
# Find additional schema validation / deserialization for plugin TOML components
rg -n "PluginTomlConfig|components.*(kind|enabled|config)|validate_.*component|deserialize_.*component" crates/cli/src/Repository: NVIDIA/NeMo-Relay
Length of output: 4125
Clarify plugin TOML component shape expectations in this precedence test
This test’s components = ["explicit"] matches current CLI behavior: load_plugin_toml_config_from_path(s) only runs validate_plugin_toml_component_kinds, which checks duplicate kinds only when each components[] entry is a TOML table containing kind—non-table entries (like strings) are accepted and passed through into gateway.plugin_config. So the test is correctly focused on override/merge precedence, not component schema validation.
For maintainability/coverage, document this intent (precedence-only, minimal shape) or add a companion test that uses the full { kind, enabled, config } component object structure so regressions in real component handling aren’t missed.
🤖 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/cli/tests/coverage/config_tests.rs` around lines 671 - 704, Update the
test plugin_config_file_overrides_discovered_plugins_toml to explicitly document
that it only verifies precedence with minimal component shapes (string
elements), and add a companion test that uses full component objects ({ kind,
enabled, config }) to ensure real-world component shape is preserved when
plugin_config_file overrides discovered plugins; locate references to
load_plugin_toml_config_from_path and validate_plugin_toml_component_kinds to
ensure the new test exercises the same code-paths and assert the resulting
resolved.gateway.plugin_config contains the full object structure, mirroring the
existing assertion style.
| `--plugin-config-file` and `NEMO_RELAY_PLUGIN_CONFIG_FILE` load exactly the | ||
| specified TOML file and skip normal `plugins.toml` discovery. This lets an | ||
| operator override a sibling or project plugin file for one run without moving | ||
| files around. |
There was a problem hiding this comment.
Clarify error handling for missing or invalid explicit plugin files.
The documentation states that --plugin-config-file loads "exactly the specified TOML file" but doesn't explicitly document what happens if the file is missing or contains invalid TOML. Later at line 105, the docs state "Missing files are skipped" in the context of discovered plugins.toml files, which could mislead users into thinking the same applies to explicit --plugin-config-file.
Per the PR objectives, missing or invalid plugin TOML specified via --plugin-config-file produces a startup configuration error (not a silent skip). Add a sentence clarifying that explicitly-specified files must exist and contain valid TOML, distinct from the discovery behavior.
Suggested addition
After line 90, add:
operator override a sibling or project plugin file for one run without moving
files around.
+
+The gateway will fail to start with a configuration error if the specified file
+does not exist or contains invalid TOML.Alternatively, add to the Validation section (around line 240):
Common validation failures include:
+- Missing or invalid TOML in files specified via `--plugin-config-file`.
- Unknown component kinds when policy treats them as errors.🤖 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 `@docs/build-plugins/plugin-configuration-files.mdx` around lines 87 - 90, Add
a clear sentence after the paragraph about
`--plugin-config-file`/`NEMO_RELAY_PLUGIN_CONFIG_FILE` stating that unlike
discovered `plugins.toml` files (which are skipped when missing), an explicitly
specified plugin file must exist and contain valid TOML or the process will fail
with a startup configuration error; reference the exact flags
`--plugin-config-file` and `NEMO_RELAY_PLUGIN_CONFIG_FILE` and contrast this
behavior with the "Missing files are skipped" note for discovered
`plugins.toml`.
2 participants
Overview
Add
--plugin-config-file <path>as an explicit TOML plugin configuration source for gateway startup andnemo-relay run.This gives operators, demos, and harnesses a file-based override path for large plugin configs without shell-escaping JSON through
--plugin-configor relying on sibling/project/userplugins.tomldiscovery.Details
--plugin-config-file <path>andNEMO_RELAY_PLUGIN_CONFIG_FILE.nemo-relay run.plugins.tomlloading when present.--plugin-config.These two explicit plugin config sources are mutually exclusive:
nemo-relay \ --plugin-config '{"version":1,"components":[]}' \ --plugin-config-file /path/to/plugins.toml[plugins].configinconfig.tomlstill conflicts with plugin TOML sources, including the explicit file.Validation run:
Manual E2E startup checks:
--plugin-config-filewhile a siblingplugins.tomlwas intentionally invalid./healthzreturned{"status":"ok"}.NEMO_RELAY_PLUGIN_CONFIG_FILEworks.--plugin-configplus--plugin-config-file.[plugins].configplus--plugin-config-file.Where should the reviewer start?
Start with
crates/cli/src/config.rs, especially the config-source validation and explicit plugin TOML loading path.Then review
crates/cli/tests/coverage/config_tests.rsfor the intended precedence and conflict behavior.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
New Features
--plugin-config-fileCLI option to load plugin configuration from a TOML file.NEMO_RELAY_PLUGIN_CONFIG_FILEenvironment variable support.Documentation