fix: layer Relay configuration by priority - #584
Conversation
|
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:
WalkthroughConfiguration discovery now layers explicit-or-user, project, and system files. Plugin paths are deduplicated by physical identity, gateway merges handle upstream credentials and logging sinks, lifecycle operations use explicit plugin targets, editor targeting reflects those paths, and diagnostics expose explicit-layer status. ChangesConfiguration Layering
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ConfigResolver
participant PluginLoader
participant Lifecycle
participant GatewayMerger
participant Doctor
CLI->>ConfigResolver: resolve explicit-or-user, project, and system paths
ConfigResolver->>PluginLoader: provide ordered plugin configuration paths
PluginLoader->>Lifecycle: provide resolved explicit plugin configuration
Lifecycle->>GatewayMerger: hydrate dynamic plugin scopes from merged configuration
GatewayMerger->>CLI: return merged runtime configuration
CLI->>Doctor: collect configuration layer statuses
Doctor->>CLI: render explicit, workspace, global, and system diagnostics
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
size:L
github-actions
Bot
added
size:XL
bbednarski9
changed the title
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
b376883 to
839f555
Compare
![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/tests/coverage/shared/config_tests.rs (1)
1084-1093: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winWrap this test in the shared cwd/env guard.
plugin_config_paths(Some(&explicit), None)still reads process-globalcurrent_dir()andNEMO_RELAY_CONFIG_SCOPE, but this test is unguarded while the neighboring discovery tests usePluginConfigDiscoveryScope::enter(...)/CwdTestScope::locked(). In parallel runs, a sibling test can change cwd/env and make this assertion flaky.🤖 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/shared/config_tests.rs` around lines 1084 - 1093, Wrap plugins_toml_path_resolution_tracks_config_scope in the established shared cwd/environment guards, using PluginConfigDiscoveryScope::enter(...) and CwdTestScope::locked() as neighboring discovery tests do, before calling plugin_config_paths. Keep the existing explicit path and assertions unchanged.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 `@crates/cli/tests/cli_tests.rs`:
- Around line 3338-3352: Strengthen the assertions in the plugin_configs
verification loop so each matched reported layer is also confirmed to have a
non-failing status after the files are rewritten. Continue validating both
expected paths, and inspect the matched configuration entry’s status or
equivalent diagnostic field rather than only checking its path.
In `@crates/core/src/plugin.rs`:
- Around line 1654-1671: Expose the existing `deduplicate_plugin_config_paths`
in `crates/core/src/plugin.rs` as `pub` with `#[doc(hidden)]`, matching
`default_plugin_config_paths` for cross-crate reuse. Remove the duplicate helper
from `crates/cli/src/configuration/mod.rs` and call
`nemo_relay::plugin::deduplicate_plugin_config_paths` there.
In `@docs/reference/operational-logging.mdx`:
- Around line 116-120: Update the layer-order wording in the operational logging
documentation to match the documented precedence contract: explicit-or-user,
then project, then system. If retaining the current aggregation order,
explicitly label it as lowest-to-highest or reverse precedence so overlay
direction cannot be misinterpreted.
---
Outside diff comments:
In `@crates/cli/tests/coverage/shared/config_tests.rs`:
- Around line 1084-1093: Wrap plugins_toml_path_resolution_tracks_config_scope
in the established shared cwd/environment guards, using
PluginConfigDiscoveryScope::enter(...) and CwdTestScope::locked() as neighboring
discovery tests do, before calling plugin_config_paths. Keep the existing
explicit path and assertions unchanged.
🪄 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: 58564dd6-372d-4295-8b09-14c2db72d096
📒 Files selected for processing (23)
crates/cli/README.mdcrates/cli/src/commands/configure/model.rscrates/cli/src/commands/plugins/subcommands.rscrates/cli/src/commands/serve.rscrates/cli/src/configuration/mod.rscrates/cli/src/diagnostics/mod.rscrates/cli/src/diagnostics/model.rscrates/cli/src/diagnostics/render.rscrates/cli/src/plugins/types.rscrates/cli/tests/cli_tests.rscrates/cli/tests/coverage/agents/launcher_tests.rscrates/cli/tests/coverage/commands/main_tests.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/doctor_tests.rscrates/cli/tests/coverage/shared/setup_tests.rscrates/core/src/plugin.rscrates/core/tests/unit/plugin_tests.rsdocs/configure-plugins/model-pricing.mdxdocs/configure-plugins/observability/configuration.mdxdocs/configure-plugins/plugin-configuration-files.mdxdocs/nemo-relay-cli/basic-usage.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/reference/operational-logging.mdx
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: Rust / Test (windows-arm64)
- GitHub Check: Rust / Test (windows-amd64)
- GitHub Check: Node.js / Test (windows-arm64)
- GitHub Check: Python / Test (windows-arm64)
🧰 Additional context used
📓 Path-based instructions (25)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.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 workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.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.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/cli/src/commands/serve.rscrates/cli/src/plugins/types.rscrates/cli/tests/coverage/agents/launcher_tests.rscrates/cli/tests/coverage/shared/doctor_tests.rscrates/cli/src/diagnostics/model.rscrates/cli/tests/coverage/shared/setup_tests.rscrates/cli/src/commands/plugins/subcommands.rscrates/cli/src/commands/configure/model.rscrates/cli/src/diagnostics/render.rscrates/core/tests/unit/plugin_tests.rscrates/cli/src/diagnostics/mod.rscrates/cli/tests/coverage/commands/main_tests.rscrates/core/src/plugin.rscrates/cli/tests/cli_tests.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/cli/src/commands/serve.rscrates/cli/src/plugins/types.rscrates/cli/tests/coverage/agents/launcher_tests.rscrates/cli/tests/coverage/shared/doctor_tests.rscrates/cli/src/diagnostics/model.rscrates/cli/tests/coverage/shared/setup_tests.rscrates/cli/src/commands/plugins/subcommands.rscrates/cli/src/commands/configure/model.rscrates/cli/src/diagnostics/render.rscrates/core/tests/unit/plugin_tests.rscrates/cli/src/diagnostics/mod.rscrates/cli/tests/coverage/commands/main_tests.rscrates/core/src/plugin.rscrates/cli/tests/cli_tests.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/cli/src/commands/serve.rscrates/cli/src/plugins/types.rscrates/cli/tests/coverage/agents/launcher_tests.rscrates/cli/tests/coverage/shared/doctor_tests.rscrates/cli/src/diagnostics/model.rscrates/cli/tests/coverage/shared/setup_tests.rscrates/cli/src/commands/plugins/subcommands.rscrates/cli/src/commands/configure/model.rscrates/cli/src/diagnostics/render.rscrates/core/tests/unit/plugin_tests.rscrates/cli/src/diagnostics/mod.rscrates/cli/tests/coverage/commands/main_tests.rscrates/core/src/plugin.rscrates/cli/tests/cli_tests.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/cli/src/commands/serve.rscrates/cli/src/plugins/types.rscrates/cli/tests/coverage/agents/launcher_tests.rscrates/cli/tests/coverage/shared/doctor_tests.rscrates/cli/src/diagnostics/model.rscrates/cli/tests/coverage/shared/setup_tests.rscrates/cli/src/commands/plugins/subcommands.rscrates/cli/src/commands/configure/model.rscrates/cli/src/diagnostics/render.rscrates/core/tests/unit/plugin_tests.rscrates/cli/src/diagnostics/mod.rscrates/cli/tests/coverage/commands/main_tests.rscrates/core/src/plugin.rscrates/cli/tests/cli_tests.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/cli/src/commands/serve.rscrates/cli/src/plugins/types.rscrates/cli/tests/coverage/agents/launcher_tests.rscrates/cli/tests/coverage/shared/doctor_tests.rscrates/cli/src/diagnostics/model.rscrates/cli/tests/coverage/shared/setup_tests.rscrates/cli/src/commands/plugins/subcommands.rscrates/cli/src/commands/configure/model.rscrates/cli/src/diagnostics/render.rscrates/core/tests/unit/plugin_tests.rscrates/cli/src/diagnostics/mod.rscrates/cli/tests/coverage/commands/main_tests.rscrates/core/src/plugin.rscrates/cli/tests/cli_tests.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/cli/src/commands/serve.rscrates/cli/src/plugins/types.rscrates/cli/src/diagnostics/model.rscrates/cli/src/commands/plugins/subcommands.rscrates/cli/src/commands/configure/model.rscrates/cli/src/diagnostics/render.rscrates/cli/src/diagnostics/mod.rscrates/core/src/plugin.rscrates/cli/src/configuration/mod.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/cli/src/commands/serve.rsdocs/configure-plugins/observability/configuration.mdxcrates/cli/src/plugins/types.rscrates/cli/tests/coverage/agents/launcher_tests.rsdocs/reference/operational-logging.mdxdocs/configure-plugins/model-pricing.mdxdocs/nemo-relay-cli/claude-code.mdxcrates/cli/tests/coverage/shared/doctor_tests.rscrates/cli/src/diagnostics/model.rscrates/cli/tests/coverage/shared/setup_tests.rscrates/cli/src/commands/plugins/subcommands.rscrates/cli/src/commands/configure/model.rscrates/cli/README.mdcrates/cli/src/diagnostics/render.rscrates/core/tests/unit/plugin_tests.rsdocs/nemo-relay-cli/basic-usage.mdxcrates/cli/src/diagnostics/mod.rscrates/cli/tests/coverage/commands/main_tests.rscrates/core/src/plugin.rscrates/cli/tests/cli_tests.rsdocs/configure-plugins/plugin-configuration-files.mdxcrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/cli/src/commands/serve.rscrates/cli/src/plugins/types.rscrates/cli/tests/coverage/agents/launcher_tests.rscrates/cli/tests/coverage/shared/doctor_tests.rscrates/cli/src/diagnostics/model.rscrates/cli/tests/coverage/shared/setup_tests.rscrates/cli/src/commands/plugins/subcommands.rscrates/cli/src/commands/configure/model.rscrates/cli/src/diagnostics/render.rscrates/core/tests/unit/plugin_tests.rscrates/cli/src/diagnostics/mod.rscrates/cli/tests/coverage/commands/main_tests.rscrates/core/src/plugin.rscrates/cli/tests/cli_tests.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/cli/src/commands/serve.rscrates/cli/src/plugins/types.rscrates/cli/tests/coverage/agents/launcher_tests.rscrates/cli/tests/coverage/shared/doctor_tests.rscrates/cli/src/diagnostics/model.rscrates/cli/tests/coverage/shared/setup_tests.rscrates/cli/src/commands/plugins/subcommands.rscrates/cli/src/commands/configure/model.rscrates/cli/src/diagnostics/render.rscrates/core/tests/unit/plugin_tests.rscrates/cli/src/diagnostics/mod.rscrates/cli/tests/coverage/commands/main_tests.rscrates/core/src/plugin.rscrates/cli/tests/cli_tests.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
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
Files:
docs/configure-plugins/observability/configuration.mdxdocs/reference/operational-logging.mdxdocs/configure-plugins/model-pricing.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/basic-usage.mdxdocs/configure-plugins/plugin-configuration-files.mdx
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Update
README.md,fern/, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.
**/*.{md,mdx}: Prefer the documented public API, not internal shortcuts
Keep package names, repo references, and build commands current
Keep release-process and release-notes guidance in repo-maintainer docs such asRELEASING.md, not as user-facing docs pages orCHANGELOG.md
Keep stable user-facing wrappers atscripts/root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pagesIf links in documentation change, run
just docs-linkcheck.
Files:
docs/configure-plugins/observability/configuration.mdxdocs/reference/operational-logging.mdxdocs/configure-plugins/model-pricing.mdxdocs/nemo-relay-cli/claude-code.mdxcrates/cli/README.mddocs/nemo-relay-cli/basic-usage.mdxdocs/configure-plugins/plugin-configuration-files.mdx
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
docs/configure-plugins/observability/configuration.mdxdocs/reference/operational-logging.mdxdocs/configure-plugins/model-pricing.mdxdocs/nemo-relay-cli/claude-code.mdxcrates/cli/README.mddocs/nemo-relay-cli/basic-usage.mdxdocs/configure-plugins/plugin-configuration-files.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/configure-plugins/observability/configuration.mdxdocs/reference/operational-logging.mdxdocs/configure-plugins/model-pricing.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/basic-usage.mdxdocs/configure-plugins/plugin-configuration-files.mdx
docs/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If documentation examples or commands under
docs/change, run the targeted docs checks appropriate to the change.
Files:
docs/configure-plugins/observability/configuration.mdxdocs/reference/operational-logging.mdxdocs/configure-plugins/model-pricing.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/basic-usage.mdxdocs/configure-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/configure-plugins/observability/configuration.mdxdocs/reference/operational-logging.mdxdocs/configure-plugins/model-pricing.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/basic-usage.mdxdocs/configure-plugins/plugin-configuration-files.mdx
{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/agents/launcher_tests.rscrates/cli/tests/coverage/shared/doctor_tests.rscrates/cli/tests/coverage/shared/setup_tests.rscrates/core/tests/unit/plugin_tests.rscrates/cli/tests/coverage/commands/main_tests.rscrates/cli/tests/cli_tests.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{md,rst,html,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
**/*.{md,rst,html,txt}: Always spellNVIDIAin all caps. Do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun because the name starts with an 'en' sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company.
Use trademark symbols with product names only when the document type or legal guidance requires them.
Verify official capitalization, spacing, and hyphenation for product names.
Precede NVIDIA product names withNVIDIAon first mention when it is natural and accurate.
Do not rewrite product names for grammar or title-case rules.
Preserve third-party product names according to the owner's spelling.
Include the company name and full model qualifier on first use when it helps identify the model.
Preserve the official capitalization and punctuation of model names.
Use shorter family names only after the full name is established.
Spell out a term on first use and put the acronym in parentheses unless the acronym is widely understood by the intended audience.
Use the acronym on later mentions after it has been defined.
For long documents, reintroduce the full term if readers might lose context.
Form plurals of acronyms withs, not an apostrophe, such asGPUs.
In headings, common acronyms can remain abbreviated. Spell out the term in the first or second sentence of the body.
Common terms such asCPU,GPU,PC,API, andUIusually do not need to be spelled out for developer audiences.
Files:
crates/cli/README.md
**/*.{md,rst,html}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
Link the first mention of a product name when the destination helps the reader.
Files:
crates/cli/README.md
**/*.{md,rst,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
Spell
NVIDIAin all caps. Do not useNvidia,nvidia, orNV.
Files:
crates/cli/README.md
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,rst}: 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 as "here" or "read more."
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative steps. Keep steps parallel and split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce.
Preferrefer tooverseewhen the wording points readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical docs.
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 include commas in thousands.
Do not add trademark symbols to learning-oriented docs unless the source, platform, or legal guidance explicitly requires them.
Files:
crates/cli/README.md
**/*.md
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.md: Use title case consistently in technical documentation headings
Avoid quotation marks, ampersands, and exclamation marks in headings
Keep product, event, research, and whitepaper names in their official title case
Use title case for table headers
Do not force social-media sentence case into technical docs
Format code elements, commands, parameters, package names, and expressions in monospace
Format directories, file names, and paths in monospace using backticks
Use angle brackets inside monospace for variables inside paths, such as/home/<username>/.login
Format error messages and strings in quotation marks, keeping literal code strings in code formatting when clearer
Format UI buttons, menus, fields, and labels in bold
Use angle brackets between UI labels for menu paths, such as File > Save As
Use italics for new terms on first use, sparingly and only when introducing the term
Use italics for publication titles
Format keyboard shortcuts in plain text, such as Press Ctrl+Alt+Delete
Use owner/repo link text for GitHub repositories, preferring[NVIDIA/NeMo](link)over prose references like 'the GitHub repo'
Introduce every code block with a complete sentence
Do not make a code block complete the grammar of the previous sentence
Do not continue a sentence after a code block
Use syntax highlighting when the format supports it for code blocks
Avoid the word 'snippet' unless the surrounding docs already use it as a term of art
Keep inline method, function, and class references consistent with nearby docs, omitting empty parentheses for prose readability when no call is shown
Use descriptive anchor text that matches the destination title when possible for links
Avoid raw URLs in running text
Avoid generic anchor text such as 'here,' 'this page,' and 'read more'
Include acronyms in link text when a linked term includes an acronym
Do not link long sentences or multiple sentences
Avoid links that pull readers away from a procedure unless the link is a p...
Files:
crates/cli/README.md
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/tests/unit/plugin_tests.rscrates/core/src/plugin.rs
🧠 Learnings (2)
📚 Learning: 2026-07-14T02:53:59.997Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 415
File: docs/configure-plugins/observability/opentelemetry.mdx:98-113
Timestamp: 2026-07-14T02:53:59.997Z
Learning: In NeMo-Relay’s OpenTelemetry/OpenInference observability projection docs under docs/configure-plugins/observability/, document the projected-attribute contract as follows: (1) emit scalar top-level `data`/`metadata` fields as typed dotted OTLP attributes (for example, `nemo_relay.start.metadata.tenant`); (2) keep nested objects/arrays as JSON strings at their top-level OTLP attribute (rather than expanding them into nested OTLP attributes); and (3) do not reference the legacy `*_json` payload attributes (e.g., `data_json`, `metadata_json`, `input_json`) because they were intentionally removed as a breaking change.
Applied to files:
docs/configure-plugins/observability/configuration.mdx
📚 Learning: 2026-07-28T20:07:29.880Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 571
File: crates/core/src/api/runtime/state.rs:996-1020
Timestamp: 2026-07-28T20:07:29.880Z
Learning: In NeMo Relay (RELAY-509), sanitizer callback failures must be treated as intentional fail-open behavior. When an event/tool (request/response) or LLM (request/response) sanitizer callback fails, the sanitizer chain should retain and publish the last valid event/payload snapshot (rather than dropping/invalidating the data) and log the failure including callback context (e.g., which sanitizer/callback failed and relevant identifiers). Apply this consistently across all sanitizer chains mentioned in the RELAY-509 documentation/migration guide.
Applied to files:
crates/core/src/plugin.rs
🔇 Additional comments (25)
crates/cli/src/diagnostics/mod.rs (1)
156-166: LGTM!Also applies to: 309-314
crates/cli/src/diagnostics/model.rs (1)
53-54: LGTM!crates/cli/src/diagnostics/render.rs (1)
19-23: LGTM!Also applies to: 46-50, 99-105
crates/cli/tests/cli_tests.rs (1)
3207-3267: LGTM!Also applies to: 3269-3328
crates/cli/tests/coverage/shared/doctor_tests.rs (1)
59-59: LGTM!crates/cli/src/configuration/mod.rs (4)
1301-1329: 🎯 Functional CorrectnessVerified dedup correctness; flagging duplication with
crates/core/src/plugin.rs.
deduplicate_plugin_config_pathscorrectly preserves the highest-precedence entry for physically-aliased paths (reverse-scan +seenset + re-reverse), matching the symlink dedup test. However, this function is byte-for-byte duplicated incrates/core/src/plugin.rs(lines 1654-1671). See consolidated comment.Also applies to: 1394-1411
111-142: Requiredness now correctly scoped to the explicit path.
requirednow only trips for the file that equals the caller-supplied explicit path, rather than any discovered layer whenever an explicit path was set. Project/system layers stay optional-if-absent, and theexplicit_config_must_existtest confirms non-existent explicit files still fail loudly.Also applies to: 980-1023
1056-1110: Layer ordering verified against precedence tests.Traced
config_paths_scoped/plugin_config_paths_scopedagainstconfig_paths_layer_explicit_or_user_then_project_then_systemandplugin_config_paths_layer_explicit_or_user_then_project_then_systeminconfig_tests.rs: explicit-or-user, project (suppressed underuser_only), system-last (highest precedence) all line up correctly, including the explicit+override-plugin-path branch.
1631-1731: Gateway merge helpers verified correct across three scenarios.Manually traced
merge_gateway_config_toml→clear_credentials_for_replaced_upstreams→merge_logging_sinks_by_path→merge_logging_sink_lists→coalesce_logging_sinksagainst the newuphill_config_layers_replace_scalars_and_aggregate_distinct_agent_tables,upstream_base_url_identity_controls_credential_inheritance, andlogging_sinks_aggregate_by_path_with_higher_layers_firsttests. The three-layer sink-merge trace (system/shared/project/user) reproduces the expected["system.log", "shared.log", "project.log", "user.log"]output and field overlay exactly.crates/core/src/plugin.rs (1)
1634-1652: Ordering and dedup wiring look correct.
load_plugin_config_filesnow routes through dedup before reading, anddefault_plugin_config_pathsmatches the new user → project → system ordering; both are exercised by the added unit tests (test_default_plugin_config_paths_order_user_project_system,test_load_plugin_config_files_deduplicates_aliases_at_highest_precedence).Also applies to: 1716-1732
crates/cli/tests/coverage/shared/config_tests.rs (1)
336-364: Solid, well-targeted coverage for the new layering semantics.The new/updated tests exercise exactly the promised behaviors: explicit-vs-user sibling replacement plus project inclusion, endpoint-identity-scoped credential clearing, symlink-alias dedup at highest precedence, and the three new
merge_gateway_config_tomlscenarios (scalar/agent aggregation, credential inheritance, logging-sink aggregation). Traced these against the implementation and they match.Also applies to: 713-737, 880-918, 3354-3563
crates/core/tests/unit/plugin_tests.rs (1)
2077-2213: Coverage matches the new precedence and dedup behavior.
test_load_plugin_config_files_merges_files_by_precedence,test_default_plugin_config_paths_order_user_project_system, andtest_load_plugin_config_files_deduplicates_aliases_at_highest_precedencecorrectly exercise scalar/recursive-config/list-aggregation semantics and symlink-alias deduplication, matching thecrates/core/src/plugin.rsimplementation.crates/cli/README.md (1)
151-212: LGTM!docs/configure-plugins/model-pricing.mdx (1)
59-63: LGTM!docs/configure-plugins/observability/configuration.mdx (1)
86-95: LGTM!docs/configure-plugins/plugin-configuration-files.mdx (1)
70-77: Documentation is internally consistent with the implementation.Cross-checked the revised precedence chain (explicit-or-user → project → system, system-highest), the dedup/missing-file/user-only-bootstrap wording, the editor-target rules, and the ATOF sinks example (system-first ordering) against
crates/cli/src/configuration/mod.rsandcrates/core/src/plugin.rs. Also correctly distinguishes this plugin-config simple-concatenation model from the path-keyedconfig.tomllogging-sink merge described inbasic-usage.mdx.Also applies to: 185-208, 235-239, 277-333, 352-353, 381-382
docs/nemo-relay-cli/basic-usage.mdx (1)
146-149: Consistent with the merge/precedence implementation.The shared-config precedence description, the upstream-identity credential-clearing rule, and the path-keyed logging-sink aggregation description all match
clear_credentials_for_replaced_upstreams/merge_logging_sinks_by_pathincrates/cli/src/configuration/mod.rs.Also applies to: 169-175, 208-211, 271-276, 295-298
docs/nemo-relay-cli/claude-code.mdx (1)
140-141: LGTM!crates/cli/src/commands/serve.rs (1)
11-11: LGTM!crates/cli/tests/coverage/agents/launcher_tests.rs (1)
1877-1877: LGTM!crates/cli/tests/coverage/commands/main_tests.rs (1)
20-73: LGTM!Also applies to: 229-229
crates/cli/src/commands/configure/model.rs (1)
45-48: LGTM!crates/cli/src/commands/plugins/subcommands.rs (1)
75-75: LGTM!Also applies to: 172-177
crates/cli/src/plugins/types.rs (1)
21-21: LGTM!crates/cli/tests/coverage/shared/setup_tests.rs (1)
562-570: LGTM!
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/cli/tests/coverage/shared/config_tests.rs (1)
322-342: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winIsolate the system plugin layer in these source-list tests.
System
plugins.tomlis now part of implicit discovery, but these tests only isolate XDG and CWD. An existing system plugin file can invalidate the empty assertion and expected source vectors. Override/mock the system path or provide an isolated system-root fixture.Also applies to: 355-377
🤖 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/shared/config_tests.rs` around lines 322 - 342, Isolate the system plugin layer in the source-list tests around effective_plugin_toml_sources, including the related test at the noted range. Override or mock the system plugins.toml location, or create an isolated system-root fixture, so implicit system discovery cannot add existing host files and invalidate the empty and expected source assertions.
🤖 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.
Outside diff comments:
In `@crates/cli/tests/coverage/shared/config_tests.rs`:
- Around line 322-342: Isolate the system plugin layer in the source-list tests
around effective_plugin_toml_sources, including the related test at the noted
range. Override or mock the system plugins.toml location, or create an isolated
system-root fixture, so implicit system discovery cannot add existing host files
and invalidate the empty and expected source assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: e0ec4d5d-accc-417a-a790-3f7effc0fe03
📒 Files selected for processing (2)
crates/cli/tests/cli_tests.rscrates/cli/tests/coverage/shared/config_tests.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (9)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.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 workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.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.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/cli/tests/cli_tests.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/cli/tests/cli_tests.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/cli/tests/cli_tests.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/cli/tests/cli_tests.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/cli/tests/cli_tests.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/cli/tests/cli_tests.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/cli/tests/cli_tests.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/cli/tests/cli_tests.rscrates/cli/tests/coverage/shared/config_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/cli_tests.rscrates/cli/tests/coverage/shared/config_tests.rs
🔇 Additional comments (4)
crates/cli/tests/cli_tests.rs (1)
3352-3363: Assert repaired plugin layers are non-failing.The loop only confirms both paths are listed; it can still pass if either layer retains a
failstatus. Find each entry and assert its status is no longer failing.crates/cli/tests/coverage/shared/config_tests.rs (3)
68-76: 🎯 Functional CorrectnessVerify the system-path expectations are platform-correct.
These tests hard-code
/etc/nemo-relay/.... If production discovery uses a platform-specific system root, the expectations will fail on Windows or other supported targets. Derive both paths from the same helper used by discovery, or add target-specific expectations.Also applies to: 113-121
51-147: 📐 Maintainability & Code QualityConfirm the required Rust validation was run.
For this Rust change, confirm
just test-rust,cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings, and finaluv run pre-commit run --all-filescompleted successfully.Source: Coding guidelines
612-612: LGTM!Also applies to: 792-792, 816-816, 862-862, 984-984, 1028-1028, 1924-1924, 1949-1949, 1979-1979, 2010-2010, 2846-2846, 2862-2862, 2897-2897, 2964-2964, 3033-3033, 3106-3106, 3193-3193, 3218-3218, 3285-3285, 3628-3628, 3647-3647, 3707-3707, 3758-3758, 3854-3854, 3896-3896, 3937-3937, 3959-3959
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
|
/ok to test 47a52e4 |
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/nemo-relay-cli/basic-usage.mdx (1)
169-174: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign the interactive global plugin target.
The preceding setup instructions still say global setup uses the user
plugins.toml, but this section and Lines 208-212 define global as the system layer. Update the earlier sentence to name the system plugin configuration.As per path instructions, “Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.”
🤖 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/nemo-relay-cli/basic-usage.mdx` around lines 169 - 174, Update the earlier global setup sentence in basic-usage documentation to refer to the system plugin configuration instead of the user plugins.toml file, keeping it consistent with the system-layer definition used in the plugin precedence and global target guidance.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 `@docs/nemo-relay-cli/basic-usage.mdx`:
- Around line 265-277: The documented combination of a CLI base-URL override
with NEMO_RELAY_*_AUTH_HEADER is inaccurate because URL layering clears the
environment header. Update the guidance to state that CLI URL overrides cannot
use those environment-supplied headers, and revise the surrounding layering
explanation to match the current shared-config behavior.
---
Outside diff comments:
In `@docs/nemo-relay-cli/basic-usage.mdx`:
- Around line 169-174: Update the earlier global setup sentence in basic-usage
documentation to refer to the system plugin configuration instead of the user
plugins.toml file, keeping it consistent with the system-layer definition used
in the plugin precedence and global target guidance.
🪄 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: 5a03f188-e121-4a2f-a371-400d6b5aed95
📒 Files selected for processing (3)
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rsdocs/nemo-relay-cli/basic-usage.mdx
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (16)
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
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
Files:
docs/nemo-relay-cli/basic-usage.mdx
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Update
README.md,fern/, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.
**/*.{md,mdx}: Prefer the documented public API, not internal shortcuts
Keep package names, repo references, and build commands current
Keep release-process and release-notes guidance in repo-maintainer docs such asRELEASING.md, not as user-facing docs pages orCHANGELOG.md
Keep stable user-facing wrappers atscripts/root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pagesIf links in documentation change, run
just docs-linkcheck.
Files:
docs/nemo-relay-cli/basic-usage.mdx
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
docs/nemo-relay-cli/basic-usage.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/nemo-relay-cli/basic-usage.mdx
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
docs/nemo-relay-cli/basic-usage.mdxcrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rs
docs/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If documentation examples or commands under
docs/change, run the targeted docs checks appropriate to the change.
Files:
docs/nemo-relay-cli/basic-usage.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/nemo-relay-cli/basic-usage.mdx
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.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 workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.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.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/cli/src/configuration/mod.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_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/shared/config_tests.rs
🪛 LanguageTool
docs/nemo-relay-cli/basic-usage.mdx
[style] ~269-~269: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...ader across a URL change. Repeating the exact same URL preserves the existing header. The...
(EN_WORDINESS_PREMIUM_EXACT_SAME)
🔇 Additional comments (3)
crates/cli/src/configuration/mod.rs (1)
121-122: LGTM!Also applies to: 925-936, 962-973, 1005-1006, 1106-1121, 1330-1409, 1546-1600, 1647-1650
crates/cli/tests/coverage/shared/config_tests.rs (1)
307-317: LGTM!Also applies to: 321-355, 804-838, 2030-2134, 3489-3499
docs/nemo-relay-cli/basic-usage.mdx (1)
146-149: LGTM!Also applies to: 208-211, 297-300
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
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 `@crates/cli/src/plugins/lifecycle/mod.rs`:
- Around line 90-93: Consolidate explicit plugin-config resolution into one
crate-visible helper: update lifecycle_plugin_config_path in
crates/cli/src/plugins/lifecycle/mod.rs#L90-L93 to be reusable, or colocate the
shared helper with explicit_plugin_config_path; replace the duplicated call in
crates/cli/src/commands/mod.rs#L195-L200 with that helper, and update
crates/cli/src/process/launcher.rs#L70-L75 to use it while preserving the
command/inherited override fallback chain used by resolve_run_config.
In `@crates/cli/src/plugins/lifecycle/state.rs`:
- Around line 258-267: Align the physical-file deduplication in the lifecycle
scope state builder with the config-discovery deduplication used by the
plugin-loading layer. Compare the canonicalization and survivor-selection logic
in crates/core/src/plugin.rs and crates/cli/src/configuration/mod.rs, then
update the dedup flow around the layouts loop so it retains the same raw
plugins.toml path and priority winner; this must keep
load_and_hydrate_scopes_with_updates able to match
resolved.dynamic_plugins[i].source to scope.plugins_toml_path for duplicate
physical files.
- Around line 234-240: Update the layout-building logic around
user_plugin_config_path() to emit a warning when it returns None instead of
silently omitting the User/Explicit layer. Match the warning behavior and
logging mechanism used by scoped_paths_for_add, while preserving the existing
layout creation when the path is available.
In `@crates/cli/tests/coverage/shared/plugins_lifecycle_tests.rs`:
- Around line 3005-3081: Add a lifecycle test near
explicit_config_keeps_project_dynamic_plugin_lifecycle_scope that creates two
scope configuration paths resolving to the same physical plugins file, such as
via a symlink or hard link, then loads the scoped registries and asserts
canonicalize-based deduplication leaves exactly one ScopedRegistry with the
highest-priority RegistryScope and matching physical plugins_toml_path. Reuse
existing helpers such as load_scoped_registries, find_record_by_id, and
write_dynamic_manifest, and ensure the test validates the resulting
source-to-scope association.
🪄 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: ee438c40-7685-4fa3-abf1-fec926dd8039
📒 Files selected for processing (6)
crates/cli/src/commands/mod.rscrates/cli/src/configuration/mod.rscrates/cli/src/plugins/lifecycle/mod.rscrates/cli/src/plugins/lifecycle/state.rscrates/cli/src/process/launcher.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (10)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.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 workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.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.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/cli/src/commands/mod.rscrates/cli/src/process/launcher.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/src/plugins/lifecycle/state.rscrates/cli/src/plugins/lifecycle/mod.rscrates/cli/src/configuration/mod.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/cli/src/commands/mod.rscrates/cli/src/process/launcher.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/src/plugins/lifecycle/state.rscrates/cli/src/plugins/lifecycle/mod.rscrates/cli/src/configuration/mod.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/cli/src/commands/mod.rscrates/cli/src/process/launcher.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/src/plugins/lifecycle/state.rscrates/cli/src/plugins/lifecycle/mod.rscrates/cli/src/configuration/mod.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/cli/src/commands/mod.rscrates/cli/src/process/launcher.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/src/plugins/lifecycle/state.rscrates/cli/src/plugins/lifecycle/mod.rscrates/cli/src/configuration/mod.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/cli/src/commands/mod.rscrates/cli/src/process/launcher.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/src/plugins/lifecycle/state.rscrates/cli/src/plugins/lifecycle/mod.rscrates/cli/src/configuration/mod.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/cli/src/commands/mod.rscrates/cli/src/process/launcher.rscrates/cli/src/plugins/lifecycle/state.rscrates/cli/src/plugins/lifecycle/mod.rscrates/cli/src/configuration/mod.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/cli/src/commands/mod.rscrates/cli/src/process/launcher.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/src/plugins/lifecycle/state.rscrates/cli/src/plugins/lifecycle/mod.rscrates/cli/src/configuration/mod.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/cli/src/commands/mod.rscrates/cli/src/process/launcher.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/src/plugins/lifecycle/state.rscrates/cli/src/plugins/lifecycle/mod.rscrates/cli/src/configuration/mod.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/cli/src/commands/mod.rscrates/cli/src/process/launcher.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/src/plugins/lifecycle/state.rscrates/cli/src/plugins/lifecycle/mod.rscrates/cli/src/configuration/mod.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/shared/plugins_lifecycle_tests.rs
🔇 Additional comments (8)
crates/cli/src/configuration/mod.rs (5)
22-24: LGTM!Also applies to: 98-100, 864-875, 911-912, 1085-1109, 1122-1139, 1309-1318, 1323-1324, 1338-1345, 1395-1395, 1655-1664
123-124: 🎯 Functional CorrectnessPreserve required-file semantics across path normalization.
These changes rely on exact
PathBufequality. Verify that relative paths and symlinked explicit paths retain the same representation during discovery; otherwise a missing explicitly requested file could be treated as an optional ambient layer. Add relative-path and symlink regression tests.As per coding guidelines, changed configuration behavior must be tested for its promised behavior and error paths.
Also applies to: 1016-1017
Source: Coding guidelines
936-947: 🎯 Functional CorrectnessVerify override ordering does not erase explicit credentials.
replace_upstream_base_urlclears the paired auth header whenever the URL identity changes. Confirm every caller applies a same-layer auth-header override after this helper; otherwise supplying both a new URL and credential can discard the credential. Add paired server, run, and environment tests.As per coding guidelines, changed configuration behavior must be tested across its affected API surfaces.
Also applies to: 973-984, 1565-1569, 1579-1583, 1602-1612
Source: Coding guidelines
1358-1358: 🗄️ Data Integrity & IntegrationRetain the highest-precedence alias during physical-path deduplication.
Because scoped paths are layered by precedence, verify that deduplication of symlinked or otherwise aliased files keeps the highest-precedence occurrence rather than whichever spelling appears first. This affects both merge results and required-file behavior.
As per path instructions, tests should cover the behavior promised by the changed configuration surface and relevant error paths.
Source: Path instructions
1681-1754: 🗄️ Data Integrity & IntegrationKeep pathless sink documents independent.
When
pathis absent, ensure the merge does not use one shared sentinel that coalesces unrelated sink documents. Add coverage for pathless sinks and confirm the intended behavior for an empty higher-layer sink list.As per path instructions, tests should cover the behavior promised by the changed API surface.
Source: Path instructions
crates/cli/src/plugins/lifecycle/mod.rs (1)
20-24: LGTM!Also applies to: 105-135, 246-250, 270-384, 386-425, 427-462, 478-521, 1777-1799, 1863-1977
crates/cli/src/plugins/lifecycle/state.rs (1)
4-4: LGTM!Also applies to: 14-14, 112-114, 127-158
crates/cli/tests/coverage/shared/plugins_lifecycle_tests.rs (1)
2994-2996: LGTM!
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
willkill07
left a comment
willkill07
left a comment
There was a problem hiding this comment.
Approving. I do have some concern about project being layered on top of user with certain deployments but that's resolvable by updating the deployment and establishing better intent/contracts.
|
Yup i totally agree. Hardening the layering will come with some added responsibility/stress on good practice for local development, where multiple relay configs will likely exist. Possible docs follow-up. Wont create an issue yet, but will keep an eye on things |
|
/merge |
3 participants
Overview
Layer Relay's base and plugin configuration files using one consistent precedence model:
System configuration is applied last and has the highest file-level priority. An explicit file replaces only its ambient XDG user counterpart; it does not suppress project or system policy. CLI flags, environment variables, and inline/programmatic plugin configuration remain above the file layers.
Details
config.tomlandplugins.toml.pathas sink identity:relay.logand./relay.logremain distinct.doctorto report an explicit base file separately from workspace, XDG user, and system layers.The follow-up behavior is separated into three signed-off commits after the original plugin-layering commit: base configuration precedence, upstream credential identity, and logging sink identity.
This branch was rebased onto current
mainafter PR #583's squash merge. It contains only the four commits unique to this contribution, and a range-diff confirmed that their patches are unchanged.Validation on the current head:
cargo test -p nemo-relay-cli— 1,136 unit, 12 architecture, and 89 CLI integration tests passedcargo fmt --all -- --checkcargo clippy --workspace --all-targets -- -D warningsjust test-rustjust test-python— 564 tests passedjust test-node— 294 tests passedjust test-gojust test-python-plugin— 125 tests passed at 97% coverage, plus the Python worker integrationjust docs— passed with the expected unauthenticated Fern redirect warninguv run pre-commit run --all-files, with onlyattributions-rustskipped because it produces an unrelated baseline attribution rewriteWhere should the reviewer start?
Start with
crates/cli/src/configuration/mod.rs, which contains base-file source discovery, recursive TOML merging, upstream credential identity, and logging sink identity. Then reviewcrates/core/src/plugin.rsfor shared plugin ordering and physical-file deduplication. The primary regressions are incrates/cli/tests/coverage/shared/config_tests.rs,crates/core/tests/unit/plugin_tests.rs, andcrates/cli/tests/coverage/shared/setup_tests.rs.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
plugins.tomlloading inputs (including symlink aliases) and adjusted default plugin search order (user → nearest project → system).--configas replaced/active (including warn/fail rendering).