fix: honor programmatic component enablement - #644
Conversation
WalkthroughPlugin configuration resolution now preserves diagnostics, tracks discovered enablement sources, and warns when code re-enables file-disabled components. Activation and reports retain these diagnostics. Tests and documentation cover the behavior. ChangesPlugin diagnostics and configuration precedence
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PluginFiles
participant initialize_plugins
participant PluginHostActivation
participant PluginInitialization
participant ActivePluginReport
PluginFiles->>initialize_plugins: merged configuration and source diagnostics
initialize_plugins->>PluginHostActivation: resolved configuration and diagnostics
PluginHostActivation->>PluginInitialization: configuration diagnostics
PluginInitialization->>ActivePluginReport: supplied and validation diagnostics
ActivePluginReport-->>PluginHostActivation: active plugin report
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/core/src/plugin.rs`:
- Around line 1613-1621: Update the doc comment for initialize_plugins to remove
the claim that it delegates to initialize_plugins_exact. Describe that it
resolves the plugin configuration and uses the shared
initialize_plugins_with_diagnostics helper, while retaining the guidance to call
initialize_plugins_exact when fully resolved values must be applied exactly.
🪄 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: a80fff6c-9ee3-4f34-9828-ba8c52d98451
📒 Files selected for processing (5)
crates/core/src/plugin.rscrates/core/src/plugin/dynamic/host.rscrates/core/tests/unit/plugin_tests.rsdocs/about-nemo-relay/release-notes/index.mdxdocs/configure-plugins/plugin-configuration-files.mdx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (27)
**/*.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/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.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/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.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/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/configure-plugins/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.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/configure-plugins/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.mdxcrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin/dynamic/host.rscrates/core/src/plugin.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/configure-plugins/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.mdx
**/*.{md,mdx,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Examples and documentation must use each exporter's documented flush/deregister order before shutdown.
Files:
docs/configure-plugins/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.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/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.mdx
docs/about-nemo-relay/release-notes/{index,highlights,known-issues}.mdx
📄 CodeRabbit inference engine (.agents/skills/draft-release-notes/SKILL.md)
docs/about-nemo-relay/release-notes/{index,highlights,known-issues}.mdx: Update onlydocs/about-nemo-relay/release-notes/index.mdx,docs/about-nemo-relay/release-notes/highlights.mdx, anddocs/about-nemo-relay/release-notes/known-issues.mdxunless the release changes their route or entry points.
Preserve the existing MDX front matter and the JSX SPDX comment in the release-notes pages.
Files:
docs/about-nemo-relay/release-notes/index.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/core/tests/unit/plugin_tests.rscrates/core/src/plugin/dynamic/host.rscrates/core/src/plugin.rs
{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/dynamic/host.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/dynamic/host.rscrates/core/src/plugin.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/tests/unit/plugin_tests.rscrates/core/src/plugin/dynamic/host.rscrates/core/src/plugin.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/core/tests/unit/plugin_tests.rscrates/core/src/plugin/dynamic/host.rscrates/core/src/plugin.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/core/tests/unit/plugin_tests.rscrates/core/src/plugin/dynamic/host.rscrates/core/src/plugin.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/core/tests/unit/plugin_tests.rscrates/core/src/plugin/dynamic/host.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/dynamic/host.rscrates/core/src/plugin.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.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
crates/core/tests/unit/plugin_tests.rscrates/core/src/plugin/dynamic/host.rscrates/core/src/plugin.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/core/tests/unit/plugin_tests.rscrates/core/src/plugin/dynamic/host.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/dynamic/host.rscrates/core/src/plugin.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/core/tests/unit/plugin_tests.rs
{crates/core/src/plugin/dynamic/**,crates/plugin/**,crates/worker/**,crates/worker-proto/**,crates/types/**,python/plugin/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Keep the stable boundary explicit: native plugins cross a C ABI, and worker plugins cross
grpc-v1.
Files:
crates/core/src/plugin/dynamic/host.rs
{crates/core/src/plugin/dynamic/**/*.rs,examples/rust-native-plugin/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not pass Rust runtime types, trait objects, futures, or allocator-owned strings across the native dynamic-library boundary.
Files:
crates/core/src/plugin/dynamic/host.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/core/src/plugin/dynamic/host.rscrates/core/src/plugin.rs
{crates/core/src/plugin/dynamic/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Native and worker plugins are trusted extensions; document that native plugins are in-process and unsandboxed, and worker plugins provide process isolation but not a security sandbox.
Files:
crates/core/src/plugin/dynamic/host.rs
{crates/core/src/plugin/dynamic/**/*.rs,crates/plugin/**/*.rs,crates/worker/**/*.rs,crates/worker-proto/**/*.rs,python/plugin/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Manifest validation must cover kind, compatibility, load contract, integrity, capability mismatch, and disabled-plugin behavior.
Files:
crates/core/src/plugin/dynamic/host.rs
crates/core/src/plugin/dynamic/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
The native loader must keep libraries alive until registered callbacks are cleared and must deregister plugin kinds before unload.
Files:
crates/core/src/plugin/dynamic/host.rs
🧠 Learnings (1)
📚 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/dynamic/host.rscrates/core/src/plugin.rs
🔇 Additional comments (7)
crates/core/src/plugin.rs (2)
1357-1401: LGTM!Also applies to: 1484-1595, 1627-1645, 1647-1682, 1686-1736, 1830-1842
1738-1804: 🗄️ Data Integrity & IntegrationThe positional matching is consistent with
layer_config.
validate_unique_component_kindsprevents repeated kinds within each discovered file, and layered discovered configuration therefore contains at most one component per kind. The by-kind source lookup also identifies the correct source for the finalenabledvalue. No code change is required.> Likely an incorrect or invalid review comment.crates/core/src/plugin/dynamic/host.rs (2)
91-118: LGTM!Also applies to: 191-196
73-83: 🗄️ Data Integrity & IntegrationKeep
activate()as the exact-configuration entry point. It has no non-test callers in this repository. The Relay CLI uses its own exact activation path, and bindings useactivate_with_discovered_config. Resolution diagnostics are produced only by the discovery path.> Likely an incorrect or invalid review comment.crates/core/tests/unit/plugin_tests.rs (1)
2108-2142: LGTM!Also applies to: 2352-2404, 2406-2431
docs/about-nemo-relay/release-notes/index.mdx (1)
101-104: LGTM!docs/configure-plugins/plugin-configuration-files.mdx (1)
369-372: LGTM!
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/0.7 #644 +/- ##
===============================================
+ Coverage 94.35% 94.39% +0.04%
===============================================
Files 329 329
Lines 95974 96199 +225
Branches 113 113
===============================================
+ Hits 90549 90802 +253
+ Misses 5425 5397 -28
... and 7 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
2c46496 to
e00a450
Compare
|
/ok to test e00a450 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/core/src/plugin.rs`:
- Around line 1757-1805: Extract the positional component-matching logic from
merge_plugin_components and programmatic_enable_override_diagnostics into a
shared nth_component_by_kind helper that selects the nth occurrence with the
requested kind. Replace both local lookup implementations with this helper while
preserving their existing occurrence counters and behavior.
🪄 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: 79258fe0-8054-4a00-bc96-3f225c0168cf
📒 Files selected for processing (5)
crates/core/src/plugin.rscrates/core/src/plugin/dynamic/host.rscrates/core/tests/unit/plugin_tests.rsdocs/about-nemo-relay/release-notes/index.mdxdocs/configure-plugins/plugin-configuration-files.mdx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (27)
**/*.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/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.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/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.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/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/configure-plugins/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.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/configure-plugins/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.mdxcrates/core/src/plugin/dynamic/host.rscrates/core/src/plugin.rscrates/core/tests/unit/plugin_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/configure-plugins/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.mdx
**/*.{md,mdx,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Examples and documentation must use each exporter's documented flush/deregister order before shutdown.
Files:
docs/configure-plugins/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.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/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.mdx
docs/about-nemo-relay/release-notes/{index,highlights,known-issues}.mdx
📄 CodeRabbit inference engine (.agents/skills/draft-release-notes/SKILL.md)
docs/about-nemo-relay/release-notes/{index,highlights,known-issues}.mdx: Update onlydocs/about-nemo-relay/release-notes/index.mdx,docs/about-nemo-relay/release-notes/highlights.mdx, anddocs/about-nemo-relay/release-notes/known-issues.mdxunless the release changes their route or entry points.
Preserve the existing MDX front matter and the JSX SPDX comment in the release-notes pages.
Files:
docs/about-nemo-relay/release-notes/index.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/core/src/plugin/dynamic/host.rscrates/core/src/plugin.rscrates/core/tests/unit/plugin_tests.rs
{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/src/plugin/dynamic/host.rscrates/core/src/plugin.rscrates/core/tests/unit/plugin_tests.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/src/plugin/dynamic/host.rscrates/core/src/plugin.rscrates/core/tests/unit/plugin_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/src/plugin/dynamic/host.rscrates/core/src/plugin.rscrates/core/tests/unit/plugin_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/core/src/plugin/dynamic/host.rscrates/core/src/plugin.rscrates/core/tests/unit/plugin_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/core/src/plugin/dynamic/host.rscrates/core/src/plugin.rscrates/core/tests/unit/plugin_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/core/src/plugin/dynamic/host.rscrates/core/src/plugin.rscrates/core/tests/unit/plugin_tests.rs
{crates/core/src/plugin/dynamic/**,crates/plugin/**,crates/worker/**,crates/worker-proto/**,crates/types/**,python/plugin/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Keep the stable boundary explicit: native plugins cross a C ABI, and worker plugins cross
grpc-v1.
Files:
crates/core/src/plugin/dynamic/host.rs
{crates/core/src/plugin/dynamic/**/*.rs,examples/rust-native-plugin/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not pass Rust runtime types, trait objects, futures, or allocator-owned strings across the native dynamic-library boundary.
Files:
crates/core/src/plugin/dynamic/host.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/core/src/plugin/dynamic/host.rscrates/core/src/plugin.rs
{crates/core/src/plugin/dynamic/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Native and worker plugins are trusted extensions; document that native plugins are in-process and unsandboxed, and worker plugins provide process isolation but not a security sandbox.
Files:
crates/core/src/plugin/dynamic/host.rs
{crates/core/src/plugin/dynamic/**/*.rs,crates/plugin/**/*.rs,crates/worker/**/*.rs,crates/worker-proto/**/*.rs,python/plugin/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Manifest validation must cover kind, compatibility, load contract, integrity, capability mismatch, and disabled-plugin behavior.
Files:
crates/core/src/plugin/dynamic/host.rs
crates/core/src/plugin/dynamic/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
The native loader must keep libraries alive until registered callbacks are cleared and must deregister plugin kinds before unload.
Files:
crates/core/src/plugin/dynamic/host.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/src/plugin/dynamic/host.rscrates/core/src/plugin.rscrates/core/tests/unit/plugin_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.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
crates/core/src/plugin/dynamic/host.rscrates/core/src/plugin.rscrates/core/tests/unit/plugin_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/core/src/plugin/dynamic/host.rscrates/core/src/plugin.rscrates/core/tests/unit/plugin_tests.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/src/plugin/dynamic/host.rscrates/core/src/plugin.rscrates/core/tests/unit/plugin_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/core/tests/unit/plugin_tests.rs
🧠 Learnings (1)
📚 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/dynamic/host.rscrates/core/src/plugin.rs
🔇 Additional comments (8)
crates/core/src/plugin.rs (4)
1357-1401: LGTM!Also applies to: 1484-1595
1613-1683: Doc comment now accurately describes the call path.The updated doc comment correctly states that diagnostics feed into
initialize_plugins_with_diagnosticsrather than claiming delegation toinitialize_plugins_exact. This resolves the earlier stale-doc concern.
1687-1755: LGTM!
1831-1844: LGTM!crates/core/src/plugin/dynamic/host.rs (1)
65-118: LGTM!Also applies to: 191-196
crates/core/tests/unit/plugin_tests.rs (1)
710-735: LGTM!Also applies to: 2135-2169, 2322-2351, 2410-2490
docs/about-nemo-relay/release-notes/index.mdx (1)
101-104: LGTM!docs/configure-plugins/plugin-configuration-files.mdx (1)
369-372: 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>
e00a450 to
1587a0c
Compare
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/core/src/plugin/dynamic/host.rs (1)
91-118: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd dynamic-host diagnostic propagation coverage.
The existing host test checks only activation success and
report.has_errors(). Add a test that asserts diagnostics fromresolve_plugin_configappear in the returnedConfigReport.🤖 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/core/src/plugin/dynamic/host.rs` around lines 91 - 118, Add a dynamic-host activation test alongside the existing activation coverage that supplies configuration producing a diagnostic through resolve_plugin_config, then asserts the returned ConfigReport contains that diagnostic rather than only checking report.has_errors(). Reuse the existing activation entry point and verify the diagnostic’s identifying message or fields.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.
Outside diff comments:
In `@crates/core/src/plugin/dynamic/host.rs`:
- Around line 91-118: Add a dynamic-host activation test alongside the existing
activation coverage that supplies configuration producing a diagnostic through
resolve_plugin_config, then asserts the returned ConfigReport contains that
diagnostic rather than only checking report.has_errors(). Reuse the existing
activation entry point and verify the diagnostic’s identifying message or
fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: a34509f8-9ac1-4fae-bb5b-05adfc6b0b8c
📒 Files selected for processing (5)
crates/core/src/plugin.rscrates/core/src/plugin/dynamic/host.rscrates/core/tests/unit/plugin_tests.rsdocs/about-nemo-relay/release-notes/index.mdxdocs/configure-plugins/plugin-configuration-files.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 (27)
**/*.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/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.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/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.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/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/configure-plugins/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.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/configure-plugins/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.mdxcrates/core/tests/unit/plugin_tests.rscrates/core/src/plugin/dynamic/host.rscrates/core/src/plugin.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/configure-plugins/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.mdx
**/*.{md,mdx,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Examples and documentation must use each exporter's documented flush/deregister order before shutdown.
Files:
docs/configure-plugins/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.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/plugin-configuration-files.mdxdocs/about-nemo-relay/release-notes/index.mdx
docs/about-nemo-relay/release-notes/{index,highlights,known-issues}.mdx
📄 CodeRabbit inference engine (.agents/skills/draft-release-notes/SKILL.md)
docs/about-nemo-relay/release-notes/{index,highlights,known-issues}.mdx: Update onlydocs/about-nemo-relay/release-notes/index.mdx,docs/about-nemo-relay/release-notes/highlights.mdx, anddocs/about-nemo-relay/release-notes/known-issues.mdxunless the release changes their route or entry points.
Preserve the existing MDX front matter and the JSX SPDX comment in the release-notes pages.
Files:
docs/about-nemo-relay/release-notes/index.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/core/tests/unit/plugin_tests.rscrates/core/src/plugin/dynamic/host.rscrates/core/src/plugin.rs
{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/dynamic/host.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/dynamic/host.rscrates/core/src/plugin.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/tests/unit/plugin_tests.rscrates/core/src/plugin/dynamic/host.rscrates/core/src/plugin.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/core/tests/unit/plugin_tests.rscrates/core/src/plugin/dynamic/host.rscrates/core/src/plugin.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/core/tests/unit/plugin_tests.rscrates/core/src/plugin/dynamic/host.rscrates/core/src/plugin.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/core/tests/unit/plugin_tests.rscrates/core/src/plugin/dynamic/host.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/dynamic/host.rscrates/core/src/plugin.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.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
crates/core/tests/unit/plugin_tests.rscrates/core/src/plugin/dynamic/host.rscrates/core/src/plugin.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/core/tests/unit/plugin_tests.rscrates/core/src/plugin/dynamic/host.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/dynamic/host.rscrates/core/src/plugin.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/core/tests/unit/plugin_tests.rs
{crates/core/src/plugin/dynamic/**,crates/plugin/**,crates/worker/**,crates/worker-proto/**,crates/types/**,python/plugin/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Keep the stable boundary explicit: native plugins cross a C ABI, and worker plugins cross
grpc-v1.
Files:
crates/core/src/plugin/dynamic/host.rs
{crates/core/src/plugin/dynamic/**/*.rs,examples/rust-native-plugin/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not pass Rust runtime types, trait objects, futures, or allocator-owned strings across the native dynamic-library boundary.
Files:
crates/core/src/plugin/dynamic/host.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/core/src/plugin/dynamic/host.rscrates/core/src/plugin.rs
{crates/core/src/plugin/dynamic/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Native and worker plugins are trusted extensions; document that native plugins are in-process and unsandboxed, and worker plugins provide process isolation but not a security sandbox.
Files:
crates/core/src/plugin/dynamic/host.rs
{crates/core/src/plugin/dynamic/**/*.rs,crates/plugin/**/*.rs,crates/worker/**/*.rs,crates/worker-proto/**/*.rs,python/plugin/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Manifest validation must cover kind, compatibility, load contract, integrity, capability mismatch, and disabled-plugin behavior.
Files:
crates/core/src/plugin/dynamic/host.rs
crates/core/src/plugin/dynamic/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
The native loader must keep libraries alive until registered callbacks are cleared and must deregister plugin kinds before unload.
Files:
crates/core/src/plugin/dynamic/host.rs
🧠 Learnings (1)
📚 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/dynamic/host.rscrates/core/src/plugin.rs
🔇 Additional comments (14)
crates/core/src/plugin.rs (7)
1727-1770: 📐 Maintainability & Code Quality | 💤 Low valueSource attribution is keyed by kind, not position.
component_enabled_sourcesmaps a discoveredkindto the last file that setenabledfor it, without positional disambiguation. For the normal discovery path this is safe: eachplugins.tomlfile declares akindat most once, and cross-file merging combines same-kind entries into a single discovered slot before this map is consulted, so duplicate-kind discovered entries do not occur in practice. No change needed; noting this only becauseprogrammatic_enable_override_diagnosticsperforms positional matching againstdiscovered_components, which could look inconsistent at first glance next to this kind-only map.
1199-1223: LGTM!Also applies to: 1321-1329
1358-1402: LGTM!
1485-1596: Diagnostics threading and rollback-report reuse are correct.
initialize_plugins_exact_for_hostandinitialize_plugins_exact_innercorrectly seedreportwith the supplied resolution diagnostics before extending with validation diagnostics. On rollback restore,previous_state.reportis reused directly at Line 1560 instead of being recomputed, matching the stated "rollback report preservation unchanged" behavior.
1614-1656: Doc comment fix confirmed; resolution and activation wiring look correct.The doc comment for
initialize_pluginsno longer claims delegation toinitialize_plugins_exact; it now correctly states the resolved diagnostics feed the sharedinitialize_plugins_with_diagnosticshelper. This addresses the earlier review comment flagged on a prior commit.
resolve_plugin_configcorrectly computesprogrammatic_enable_override_diagnosticsfrom the discovered value before layering the programmatic overlay on top, so the diagnostic reflects the pre-overlay discovered state.
1657-1719: LGTM!
1772-1813: Positional matching correctly mirrors the merge algorithm.
programmatic_enable_override_diagnosticsreusesnth_component_by_kind, the same helpermerge_plugin_componentsuses for positional slot matching. This addresses the earlier review comment requesting a shared helper to keep the two lookups from diverging.crates/core/src/plugin/dynamic/host.rs (2)
73-83: LGTM!
119-257: LGTM!crates/core/tests/unit/plugin_tests.rs (3)
2135-2169: LGTM!
2410-2462: Overlay precedence test correctly covers both declared and undeclared components.The updated test verifies that a declared component (
observability) has itsenabledvalue forced by code, while an undeclared component (adaptive) retains the discoveredenabled = false. This is good coverage of the core precedence fix.
2464-2496: Positional diagnostic test correctly isolates the override case from the no-op case.The test constructs two discovered
observabilityentries (enabled: true,enabled: false) and two programmatic declarations, verifying only the second produces aplugin.component_reenableddiagnostic with the source path. This confirms the positional matching and source-naming logic together.docs/about-nemo-relay/release-notes/index.mdx (1)
101-104: LGTM!docs/configure-plugins/plugin-configuration-files.mdx (1)
369-372: LGTM!
|
/merge |
#### Overview Library callers can inherit plugin settings from discovered user, project, or system `plugins.toml` files without being told which files contributed to the resolved configuration. This is especially easy to miss when inherited Observability destinations cause telemetry to be exported somewhere the caller did not configure directly. This PR preserves the existing layered merge contract and makes that implicit inheritance visible. Initialization emits one source-only warning for every discovered configuration file, then continues with the resolved configuration. It does not overwrite inherited values, add an opt-out, or introduce a new initialization mode. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Keep programmatic and discovered configuration layers intact, including concatenation of `atof.sinks`, `opentelemetry.endpoints`, and `atif.storage` with higher-precedence entries first. - Emit one `plugin.configuration_inherited` warning per discovered `plugins.toml` file. - Write the warning to the operational log and include it in initialization results, dynamic-host activation reports, and the active plugin report. - Include only the source path in the warning; configuration values, destination URLs, headers, and credentials are not exposed. - Treat inheritance itself as non-fatal. Existing schema validation and plugin activation failures in the effective layered configuration remain errors. - Add binding-level assertions for the warning contract in FFI, Python, Node.js, and Go. - Update plugin-layering, Observability, and 0.7 release documentation. - Breaking changes: none. Public configuration types, schemas, binding signatures, and initialization modes are unchanged. Validation performed: - `cargo test -p nemo-relay plugin::tests --lib` — 44 passed - `cargo test --workspace --exclude nemo-relay-ffi` — passed - `cargo test -p nemo-relay-ffi -- --test-threads=1` — 168 passed - `cargo fmt --all -- --check` - `cargo clippy --workspace --all-targets -- -D warnings` - Python suite — 614 passed - Node.js suite — 344 passed - Go suite — passed - Fern validation and strict broken-link checks — passed; the unauthenticated redirect check emitted its expected warning - Repository pre-commit hooks on all changed files — passed `just` is not installed in the local validation environment, so the non-CI `test-rust` recipe commands were run directly. #### Where should the reviewer start? Start with `resolve_programmatic_plugin_config()` and `inherited_plugin_config_diagnostics()` in `crates/core/src/plugin.rs`, then review `test_programmatic_observability_destinations_concatenate_with_discovered_files_and_warn` in `crates/core/tests/unit/plugin_tests.rs`. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to #644 ## Summary by CodeRabbit - **New Features** - Observability destinations from programmatic configuration now take precedence over discovered file-based destinations when provided. - Layered destination configuration remains active across supported observability integrations. - **Bug Fixes** - Added clear warnings when plugin configuration is inherited from discovered files. - Warnings identify source paths without exposing destination values or credentials and do not block initialization. - **Documentation** - Updated configuration and release notes to explain destination layering, inheritance warnings, and validation behavior. Authors: - Bryan Bednarski (https://github.com/bbednarski9) Approvers: - Will Killian (https://github.com/willkill07) URL: #646
Overview
Discovered
plugins.tomlfiles could silently keep a plugin component disabled even when a caller explicitly included that component in its programmatic configuration withenabled = true. This happened because the typed overlay removed the default-valuedenabledfield before layering, allowing a lower-precedence file'senabled = falseto survive.This change restores the documented precedence contract: declaring a component programmatically applies its
enabledvalue over discovered file configuration. When that changes a discoveredfalsetotrue, initialization also returns and stores a warning that identifies the component and contributing file.Details
enabledin the typed programmatic overlay for every plugin component kind, rather than special-casing observability.enabled = falsevalues remain effective.enabled = falseauthoritative.plugin.component_reenabledwhen a programmatically declared component overrides a discoveredenabled = false; the diagnostic includes the component kind, theenabledfield, and the source path when available.ConfigReport, and the active report exposed byreport().Enablement matrix:
trueenabled = truetruetrueenabled = falsefalsefalseenabled = truetrueplugin.component_reenabledfalseenabled = falsefalseAcross discovered files, the highest-precedence layer that explicitly specifies
enabledwins; omission inherits the lower-precedence value. Declaring acomponent without specifying
enableduses the typed default,true. To inheritfile enablement, the caller omits that component.
Validation performed:
cargo fmt --all -- --checkpassed.cargo clippy --workspace --all-targets -- -D warningspassed.validate()remains discovery-free,initialize()reports the warning,report()retains it, and the caller-enabled collector receives events.python-worker-proto-checkhook could not findjuston the hook PATH; running the equivalentjust check-python-worker-protocommand directly passed.just test-rustpassed the Rust unit and integration suites, then failed in an unchanged pre-existing doctest incrates/core/src/api/runtime/scope_stack.rsbecause the example references the unavailablenemo_relay::Resultalias.Where should the reviewer start?
Start with
plugin_config_overlay_value()andprogrammatic_enable_override_diagnostics()incrates/core/src/plugin.rs, then reviewtest_plugin_config_overlay_enables_programmatically_declared_componentsandtest_programmatic_enable_override_diagnostic_names_discovered_sourceincrates/core/tests/unit/plugin_tests.rs.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
Bug Fixes
Documentation