fix(plugin): layer discovered config for dynamic hosts

[bbednarski9]

Overview

Adds a shared harness-native activation path that layers explicit plugin configuration over the default discovered plugins.toml files before loading dynamic plugins. The existing exact activation path remains available for hosts such as the Relay CLI that already resolved their configuration.

Details

• Reuse the same one-time config resolution used by static initialization.
• Add PluginHostActivation::activate_with_discovered_config for language and FFI bindings.
• Keep PluginHostActivation::activate exact and free of implicit discovery.
• Validate that a file-configured static component and a dynamic component activate in one owned transaction.

Validation

• cargo test -p nemo-relay --test native_plugin_integration (28 passed)
• cargo clippy -p nemo-relay --all-targets -- -D warnings
• cargo fmt --all -- --check
• git diff --check

Related Issues

Relates to #365, #366, and #368.

Summary by CodeRabbit

• New Features
  • Added a plugin activation entrypoint that layers discovered plugins.toml configuration with dynamically provided plugin components.
• Refactor
  • Improved configuration resolution by centralizing how default/discovered configuration is merged with provided settings before activation.
• Bug Fixes
  • Ensured activation cleanup correctly deregisters the discovered static-base plugin kind after clearing, without impacting dynamic registrations.
• Tests
  • Added an end-to-end integration test covering combined discovered + dynamic activation and verifying post-clear deregistration.

[coderabbitai]

Walkthrough

Plugin activation now layers discovered plugins.toml configuration with supplied configuration through a shared resolver. Dynamic activation reuses validated activation logic, and integration tests verify combined registration and cleanup. A Go scope test delegates its assertions to a helper.

Changes

Plugin discovery activation

Layer / File(s)	Summary
Resolve and activate discovered plugin configuration crates/core/src/plugin.rs, crates/core/src/plugin/dynamic/host.rs	Configuration layering is extracted into resolve_plugin_config; activation validates dynamic specs and uses the resolved configuration through a shared helper.
Validate discovered static and dynamic plugin activation crates/core/tests/integration/native_plugin_tests.rs	A subprocess-isolated asynchronous test supplies discovered static configuration with dynamic components, verifies registration, and confirms cleanup removes both plugin kinds.

Go scope test refactor

Layer / File(s)	Summary
Extract scope cleanup test helper go/nemo_relay/scope/error_coverage_test.go	The exported test wrapper now delegates its existing assertions to an unexported helper.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant PluginHostActivation
  participant resolve_plugin_config
  participant initialize_plugins_exact
  Caller->>PluginHostActivation: activate_with_discovered_config(config, dynamic_plugins)
  PluginHostActivation->>resolve_plugin_config: layer discovered plugins.toml with config
  resolve_plugin_config-->>PluginHostActivation: resolved PluginConfig
  PluginHostActivation->>PluginHostActivation: validate dynamic plugin specs
  PluginHostActivation->>initialize_plugins_exact: activate validated plugins

Loading

Possibly related PRs

• NVIDIA/NeMo-Relay#364: Modifies the same PluginHostActivation activation flow.
• NVIDIA/NeMo-Relay#365: Calls the Rust dynamic activation and discovered-configuration APIs.
• NVIDIA/NeMo-Relay#368: Connects FFI activation to discovered configuration layering.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title follows Conventional Commits and accurately summarizes the plugin config layering change.
Description check	✅ Passed	The description covers Overview, Details, Validation, and Related Issues, with only template checklist and reviewer-start sections omitted.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Fern docs preview: https://nvidia-preview-pull-request-418.docs.buildwithfern.com/nemo/relay (​https://nvidia-preview-pull-request-418.docs.buildwithfern.com/nemo/relay​)

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/core/src/plugin.rs (1)

1-1: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Synchronous file I/O runs directly on the async task in both call sites of resolve_plugin_config. resolve_plugin_config reads and merges discovered plugins.toml files synchronously (via resolve_default_file_plugin_config → load_plugin_config_files) with no spawn_blocking, and it is now invoked from two async entrypoints, one of which (activate_with_discovered_config) is explicitly documented as the entrypoint for language/FFI bindings — a context where blocking the calling async task on disk I/O is more likely to matter for runtime responsiveness. As per coding guidelines, "bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly."

• crates/core/src/plugin.rs#L1462-1466: wrap the discovery/merge work (or at minimum the resolve_default_file_plugin_config call) in tokio::task::spawn_blocking so resolve_plugin_config doesn't block its calling executor thread.
• crates/core/src/plugin.rs#L1453-1456: no code change needed here beyond benefiting from the fix above once resolve_plugin_config is non-blocking.
• crates/core/src/plugin/dynamic/host.rs#L91-102: same — activate_with_discovered_config will stop blocking its caller once resolve_plugin_config is fixed at the root.

🤖 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.rs` at line 1, Update resolve_plugin_config to execute
the synchronous discovery and merge work, including
resolve_default_file_plugin_config and its load_plugin_config_files path, inside
tokio::task::spawn_blocking. Preserve the existing resolved configuration and
error behavior while ensuring both async callers, including
activate_with_discovered_config, do not perform blocking file I/O on their
executor threads.

Source: Coding guidelines

🤖 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/tests/integration/native_plugin_tests.rs`:
- Around line 1247-1262: Extend the post-clear assertions in the
discovered-config activation test after activation.clear() to verify
lookup_plugin("fixture_native") returns None, while retaining the existing
static deregistration checks and confirming the static base plugin remains
registered as established by the sibling test.
- Around line 46-77: After activation.clear() in the affected native plugin
discovery test, add an assertion that lookup_plugin("fixture_native") returns
None, matching the sibling test. Keep the existing static-base deregistration
assertion and ensure both plugin kinds are verified as deregistered.

---

Outside diff comments:
In `@crates/core/src/plugin.rs`:
- Line 1: Update resolve_plugin_config to execute the synchronous discovery and
merge work, including resolve_default_file_plugin_config and its
load_plugin_config_files path, inside tokio::task::spawn_blocking. Preserve the
existing resolved configuration and error behavior while ensuring both async
callers, including activate_with_discovered_config, do not perform blocking file
I/O on their executor threads.

🪄 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: d13414ad-5b5d-4ad9-93bd-fba8fe74335c

📥 Commits

Reviewing files that changed from the base of the PR and between 8889361 and bdcd93e.

📒 Files selected for processing (3)

• crates/core/src/plugin.rs
• crates/core/src/plugin/dynamic/host.rs
• crates/core/tests/integration/native_plugin_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 (18)

**/*.rs

📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)

**/*.rs: Any Rust change must run just test-rust
Any Rust change must run cargo fmt --all
Any Rust change must run cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all for all FFI work since it is Rust work
Run just test-rust to validate FFI changes
Run cargo clippy --workspace --all-targets -- -D warnings to enforce strict linting on FFI work

When Rust files changed as part of Go work, also run cargo fmt --all, just test-rust, and cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all when Rust files are changed as part of Node work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files are changed as part of Node work
Run just test-rust when Rust files are changed as part of Node work

When changing the core Rust runtime or Rust-facing API surface, format Rust code with cargo fmt (rustfmt defaults), keep cargo clippy -- -D warnings clean, and satisfy cargo deny check per deny.toml.

**/*.rs: If any Rust code changed, always run just test-rust.
If any Rust code changed, also run cargo fmt --all.
If any Rust code changed, also run cargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, run cargo fmt --all and cargo clippy --workspace --all-targets -- -D warnings even if relying on pre-commit.

Files:

• crates/core/src/plugin/dynamic/host.rs
• crates/core/tests/integration/native_plugin_tests.rs
• crates/core/src/plugin.rs

{crates/core,crates/adaptive}/**/*

📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)

Changes to crates/core or crates/adaptive must run the full language matrix

Files:

• crates/core/src/plugin/dynamic/host.rs
• crates/core/tests/integration/native_plugin_tests.rs
• crates/core/src/plugin.rs

crates/core/**/*.rs

📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)

If the change touched crates/core or shared runtime semantics, also use validate-change for broader validation

Files:

• crates/core/src/plugin/dynamic/host.rs
• crates/core/tests/integration/native_plugin_tests.rs
• crates/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/src/plugin/dynamic/host.rs
• crates/core/tests/integration/native_plugin_tests.rs
• crates/core/src/plugin.rs

**/*.{rs,py,js,mjs,cjs,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{rs,py,js,mjs,cjs,ts,tsx}: Use Json = serde_json::Value in Rust-facing runtime APIs where the existing code expects JSON payloads.
Use Result<T> with FlowError in 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.rs
• crates/core/tests/integration/native_plugin_tests.rs
• crates/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 prefixed nemo_relay_, Go PascalCase, Node.js camelCase, and Python snake_case.

Files:

• crates/core/src/plugin/dynamic/host.rs
• crates/core/tests/integration/native_plugin_tests.rs
• crates/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/src/plugin/dynamic/host.rs
• crates/core/tests/integration/native_plugin_tests.rs
• crates/core/src/plugin.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 crate tests/ trees, and Python SDK tests belong under python/tests.

Files:

• crates/core/src/plugin/dynamic/host.rs
• crates/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

**/*

📄 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, use maintain-dynamic-plugins and 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, prefer uv run pre-commit run --files <changed files...>.
Before review or handoff, run uv run pre-commit run --all-files.

Files:

• crates/core/src/plugin/dynamic/host.rs
• crates/core/tests/integration/native_plugin_tests.rs
• crates/core/src/plugin.rs

crates/{core,adaptive}/**/*

📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)

If crates/core or crates/adaptive changed, run the full validation matrix across Rust, Python, Go, and Node.js.

Files:

• crates/core/src/plugin/dynamic/host.rs
• crates/core/tests/integration/native_plugin_tests.rs
• crates/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.

Files:

• crates/core/src/plugin/dynamic/host.rs
• crates/core/tests/integration/native_plugin_tests.rs
• crates/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/src/plugin/dynamic/host.rs
• crates/core/tests/integration/native_plugin_tests.rs
• crates/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/integration/native_plugin_tests.rs

🔇 Additional comments (2)

crates/core/src/plugin.rs (1)

1453-1466: 🎯 Functional Correctness

Clean extraction; blocking file I/O concern noted separately.

The resolve_plugin_config extraction correctly preserves prior layering behavior and lets host.rs reuse the same one-time resolution. The synchronous file-discovery I/O invoked here (and from activate_with_discovered_config) is addressed in the consolidated comment below.

crates/core/src/plugin/dynamic/host.rs (1)

65-102: LGTM! The activate/activate_with_discovered_config/activate_validated split cleanly preserves activate's "exact" semantics while adding the discovery path, matching the PR's stated goal of keeping activate implicit-discovery-free for hosts like the Relay CLI.

[coderabbitai]

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details

putComment timed out

[coderabbitai]

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 `@go/nemo_relay/scope/error_coverage_test.go`:
- Around line 15-18: Update the call to runWithTestScopeStack so it receives a
no-argument closure that invokes testWithScopeCleanupNoopsWhenPushFails with the
existing t handle. Keep testWithScopeCleanupNoopsWhenPushFails unchanged and
preserve the current test execution flow.

🪄 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: cd440358-f955-416b-91cd-875e78609369

📥 Commits

Reviewing files that changed from the base of the PR and between bdcd93e and 89efa21.

📒 Files selected for processing (2)

• crates/core/tests/integration/native_plugin_tests.rs
• go/nemo_relay/scope/error_coverage_test.go

📜 Review details

⏰ Context from checks skipped due to timeout. (2)

• GitHub Check: Check / Run
• GitHub Check: Preview docs

🧰 Additional context used

📓 Path-based instructions (19)

go/nemo_relay/**/*.go

📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)

go/nemo_relay/**/*.go: Format changed Go packages with cd go/nemo_relay && go fmt ./...
Run Go tests with just test-go to build and test the NeMo Relay Go binding
Use just build-go when you want an explicit build-only pass or need the artifact for other work
Use just ci=true test-go when you need the CI-style coverage and JUnit path
On macOS, set DYLD_LIBRARY_PATH to the ../../target/release directory before running the raw go test command directly

Use PascalCase for public Go APIs.

Files:

• go/nemo_relay/scope/error_coverage_test.go

**/*.go

📄 CodeRabbit inference engine (CONTRIBUTING.md)

When changing the experimental Go binding, format Go code with gofmt and keep go vet ./... passing.

Files:

• go/nemo_relay/scope/error_coverage_test.go

**/*.{rs,py,go,js,ts,c,h}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Use language-appropriate naming conventions: Rust snake_case, C FFI exports prefixed nemo_relay_, Go PascalCase, Node.js camelCase, and Python snake_case.

Files:

• go/nemo_relay/scope/error_coverage_test.go
• crates/core/tests/integration/native_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:

• go/nemo_relay/scope/error_coverage_test.go
• crates/core/tests/integration/native_plugin_tests.rs

{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}

📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)

Update the language-native bindings for every exposed surface in Python, Go, and Node.js.

Files:

• go/nemo_relay/scope/error_coverage_test.go

{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}

📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)

Update language wrapper helpers such as Python wrapper modules, Python type stubs, and Go shorthand packages when the new behavior belongs in those helper layers.

Files:

• go/nemo_relay/scope/error_coverage_test.go

**/*.{py,go,js,ts}

📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)

Keep Python, Go, and Node.js config objects and subscriber/exporter methods aligned so all bindings expose the same logical knobs and semantics.

Files:

• go/nemo_relay/scope/error_coverage_test.go

go/nemo_relay/**

📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)

Keep shared plugin helpers in go/nemo_relay aligned with plugin registration, composition, and lifecycle behavior.

Files:

• go/nemo_relay/scope/error_coverage_test.go

**/*

📄 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, use maintain-dynamic-plugins and 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, prefer uv run pre-commit run --files <changed files...>.
Before review or handoff, run uv run pre-commit run --all-files.

Files:

• go/nemo_relay/scope/error_coverage_test.go
• crates/core/tests/integration/native_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.

Files:

• go/nemo_relay/scope/error_coverage_test.go
• crates/core/tests/integration/native_plugin_tests.rs

go/nemo_relay/**/*

⚙️ CodeRabbit configuration file

go/nemo_relay/**/*: Review Go binding changes for cgo memory ownership, race safety, callback cleanup, idiomatic exported APIs, and parity with Rust/FFI behavior.
Any API change should include focused Go tests and consider race-test behavior.

Files:

• go/nemo_relay/scope/error_coverage_test.go

{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:

• go/nemo_relay/scope/error_coverage_test.go
• crates/core/tests/integration/native_plugin_tests.rs

**/*.rs

📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)

**/*.rs: Any Rust change must run just test-rust
Any Rust change must run cargo fmt --all
Any Rust change must run cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all for all FFI work since it is Rust work
Run just test-rust to validate FFI changes
Run cargo clippy --workspace --all-targets -- -D warnings to enforce strict linting on FFI work

When Rust files changed as part of Go work, also run cargo fmt --all, just test-rust, and cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all when Rust files are changed as part of Node work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files are changed as part of Node work
Run just test-rust when Rust files are changed as part of Node work

When changing the core Rust runtime or Rust-facing API surface, format Rust code with cargo fmt (rustfmt defaults), keep cargo clippy -- -D warnings clean, and satisfy cargo deny check per deny.toml.

**/*.rs: If any Rust code changed, always run just test-rust.
If any Rust code changed, also run cargo fmt --all.
If any Rust code changed, also run cargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, run cargo fmt --all and cargo clippy --workspace --all-targets -- -D warnings even if relying on pre-commit.

Files:

• crates/core/tests/integration/native_plugin_tests.rs

{crates/core,crates/adaptive}/**/*

📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)

Changes to crates/core or crates/adaptive must run the full language matrix

Files:

• crates/core/tests/integration/native_plugin_tests.rs

crates/core/**/*.rs

📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)

If the change touched crates/core or shared runtime semantics, also use validate-change for broader validation

Files:

• crates/core/tests/integration/native_plugin_tests.rs

**/*.{rs,py}

📄 CodeRabbit inference engine (AGENTS.md)

Follow binding naming conventions in Rust and Python: use snake_case.

Files:

• crates/core/tests/integration/native_plugin_tests.rs

**/*.{rs,py,js,mjs,cjs,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{rs,py,js,mjs,cjs,ts,tsx}: Use Json = serde_json::Value in Rust-facing runtime APIs where the existing code expects JSON payloads.
Use Result<T> with FlowError in 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/integration/native_plugin_tests.rs

crates/{core,adaptive}/**/*

📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)

If crates/core or crates/adaptive changed, run the full validation matrix across Rust, Python, Go, and Node.js.

Files:

• crates/core/tests/integration/native_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/tests/integration/native_plugin_tests.rs

🔇 Additional comments (2)

crates/core/tests/integration/native_plugin_tests.rs (2)

6-6: LGTM!

Also applies to: 46-78, 1221-1264

---

1221-1264: 📐 Maintainability & Code Quality

Confirm the complete required validation matrix was run.

Because this change is under crates/core, confirm just test-rust, cargo fmt --all, strict Clippy, the full Rust/Python/Go/Node matrix, and final all-files pre-commit validation.

Source: Coding guidelines

[coderabbitai]

♻️ Duplicate comments (1)

crates/core/tests/integration/native_plugin_tests.rs (1)

1246-1249: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Still missing the post-clear "implementation remains registered" assertion.

A prior review on this same test requested two assertions after activation.clear(): lookup_plugin(STATIC_BASE_PLUGIN_KIND).is_some() and lookup_plugin("fixture_native").is_none(). Only the second was added here. Since STATIC_BASE_PLUGIN_KIND was registered via register_plugin outside the activation transaction (line 1230), clear() should only fire the runtime dereg callback (hence the counter check) without removing the plugin implementation from the registry — so lookup_plugin(STATIC_BASE_PLUGIN_KIND) should still resolve until the explicit deregister_plugin call on line 1249. Without this assertion, a regression where clear() incorrectly also drops the implementation from the registry would go undetected.

As per path instructions, tests matching {crates/**/tests/**,...} should "cover the behavior promised by the changed API surface" and "prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests."

✅ Proposed added assertion

     activation.clear().expect("discovered host should clear");
     assert_eq!(STATIC_BASE_DEREGISTRATIONS.load(Ordering::SeqCst), 1);
+    assert!(lookup_plugin(STATIC_BASE_PLUGIN_KIND).is_some());
     assert!(lookup_plugin("fixture_native").is_none());
     assert!(deregister_plugin(STATIC_BASE_PLUGIN_KIND));

🤖 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/tests/integration/native_plugin_tests.rs` around lines 1246 -
1249, Extend the test after activation.clear() to assert
lookup_plugin(STATIC_BASE_PLUGIN_KIND).is_some() before the existing
fixture_native absence assertion. Keep the registration-count check and final
deregister_plugin(STATIC_BASE_PLUGIN_KIND) unchanged so the test verifies the
implementation remains registered until explicit deregistration.

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.

Duplicate comments:
In `@crates/core/tests/integration/native_plugin_tests.rs`:
- Around line 1246-1249: Extend the test after activation.clear() to assert
lookup_plugin(STATIC_BASE_PLUGIN_KIND).is_some() before the existing
fixture_native absence assertion. Keep the registration-count check and final
deregister_plugin(STATIC_BASE_PLUGIN_KIND) unchanged so the test verifies the
implementation remains registered until explicit deregistration.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 91284c39-ee4f-4e3e-937d-2754814b8299

📥 Commits

Reviewing files that changed from the base of the PR and between 89efa21 and 2accdc6.

📒 Files selected for processing (1)

• crates/core/tests/integration/native_plugin_tests.rs

📜 Review details

🧰 Additional context used

📓 Path-based instructions (12)

**/*.rs

📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)

**/*.rs: Any Rust change must run just test-rust
Any Rust change must run cargo fmt --all
Any Rust change must run cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all for all FFI work since it is Rust work
Run just test-rust to validate FFI changes
Run cargo clippy --workspace --all-targets -- -D warnings to enforce strict linting on FFI work

When Rust files changed as part of Go work, also run cargo fmt --all, just test-rust, and cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all when Rust files are changed as part of Node work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files are changed as part of Node work
Run just test-rust when Rust files are changed as part of Node work

When changing the core Rust runtime or Rust-facing API surface, format Rust code with cargo fmt (rustfmt defaults), keep cargo clippy -- -D warnings clean, and satisfy cargo deny check per deny.toml.

**/*.rs: If any Rust code changed, always run just test-rust.
If any Rust code changed, also run cargo fmt --all.
If any Rust code changed, also run cargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, run cargo fmt --all and cargo clippy --workspace --all-targets -- -D warnings even if relying on pre-commit.

Files:

• crates/core/tests/integration/native_plugin_tests.rs

{crates/core,crates/adaptive}/**/*

📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)

Changes to crates/core or crates/adaptive must run the full language matrix

Files:

• crates/core/tests/integration/native_plugin_tests.rs

crates/core/**/*.rs

📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)

If the change touched crates/core or shared runtime semantics, also use validate-change for broader validation

Files:

• crates/core/tests/integration/native_plugin_tests.rs

**/*.{rs,py}

📄 CodeRabbit inference engine (AGENTS.md)

Follow binding naming conventions in Rust and Python: use snake_case.

Files:

• crates/core/tests/integration/native_plugin_tests.rs

**/*.{rs,py,js,mjs,cjs,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{rs,py,js,mjs,cjs,ts,tsx}: Use Json = serde_json::Value in Rust-facing runtime APIs where the existing code expects JSON payloads.
Use Result<T> with FlowError in 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/integration/native_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 prefixed nemo_relay_, Go PascalCase, Node.js camelCase, and Python snake_case.

Files:

• crates/core/tests/integration/native_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/tests/integration/native_plugin_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, use maintain-dynamic-plugins and 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, prefer uv run pre-commit run --files <changed files...>.
Before review or handoff, run uv run pre-commit run --all-files.

Files:

• crates/core/tests/integration/native_plugin_tests.rs

crates/{core,adaptive}/**/*

📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)

If crates/core or crates/adaptive changed, run the full validation matrix across Rust, Python, Go, and Node.js.

Files:

• crates/core/tests/integration/native_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.

Files:

• crates/core/tests/integration/native_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/tests/integration/native_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/integration/native_plugin_tests.rs

🔇 Additional comments (1)

crates/core/tests/integration/native_plugin_tests.rs (1)

44-44: LGTM! Subprocess isolation for env-sensitive discovery is well-structured.

The re-exec-via-current_exe() pattern cleanly isolates XDG_CONFIG_HOME/current_dir mutation to a child process, avoiding races with other tests sharing NATIVE_PLUGIN_TEST_LOCK and process-wide env state, and the TOML fixture matches the documented plugins.toml schema (version, [[components]], kind, enabled).

Also applies to: 1189-1245

[bbednarski9]

/merge