feat(worker): activate grpc-v1 worker plugins in relay#309
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds ChangesWorker gRPC dynamic plugins
Sequence Diagram(s)sequenceDiagram
participant PluginActivation
participant load_worker_plugins
participant load_one_worker_plugin
participant RelayHostRuntimeService
participant WorkerProcess
PluginActivation->>load_worker_plugins: pass worker_specs
load_worker_plugins->>load_one_worker_plugin: activate WorkerPluginLoadSpec
load_one_worker_plugin->>RelayHostRuntimeService: start local host runtime
load_one_worker_plugin->>WorkerProcess: spawn worker process
WorkerProcess-->>load_one_worker_plugin: connect and handshake
load_one_worker_plugin-->>load_worker_plugins: WorkerPluginInstance
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
size:XXL
willkill07
force-pushed
the
wkk_dynamic-plugin-5-worker-activation
branch
from
a6c7211 to
3610661
Compare
willkill07
force-pushed
the
wkk_dynamic-plugin-5-worker-activation
branch
from
3610661 to
60a0edd
Compare
willkill07
force-pushed
the
wkk_dynamic-plugin-5-worker-activation
branch
from
60a0edd to
c57449f
Compare
willkill07
force-pushed
the
wkk_dynamic-plugin-5-worker-activation
branch
from
c57449f to
c030fa6
Compare
willkill07
force-pushed
the
wkk_dynamic-plugin-5-worker-activation
branch
from
c030fa6 to
dbd653a
Compare
willkill07
force-pushed
the
wkk_dynamic-plugin-5-worker-activation
branch
from
dbd653a to
5122f1d
Compare
willkill07
force-pushed
the
wkk_dynamic-plugin-5-worker-activation
branch
from
5122f1d to
86e6aa8
Compare
willkill07
force-pushed
the
wkk_dynamic-plugin-5-worker-activation
branch
from
86e6aa8 to
712f900
Compare
willkill07
force-pushed
the
wkk_dynamic-plugin-5-worker-activation
branch
from
6a487ea to
38fa6ee
Compare
coderabbitai
Bot
added
the
DO NOT MERGE
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/dynamic/worker.rs`:
- Around line 116-126: The `WorkerPluginActivation::clear` doc comment
overstates what the method does, since `clear(self)` is empty and deregistration
actually occurs in `Drop`. Update the comment on `clear` to make it clear that
consuming the activation only triggers cleanup via the `Drop` implementation,
and keep the wording aligned with `NativePluginActivation` and the `Drop for
WorkerPluginActivation` behavior.
- Around line 433-458: The `WorkerEndpoints::new` function leaves
`activation_dir` unused in the `#[cfg(not(unix))]` branch, which triggers a
warning-as-error on Windows builds. Update the function so the parameter is
conditionally marked as used on non-unix targets (or otherwise remove the unused
binding in that cfg branch) while preserving the existing Unix and TCP endpoint
construction logic for `WorkerEndpoints::new`.
In `@crates/core/tests/integration/worker_plugin_tests.rs`:
- Around line 694-711: The test in
command_worker_entrypoint_is_resolved_relative_to_manifest only checks for a
generic spawn failure, so it does not prove relative resolution against the
manifest directory. Update the assertion in
command_worker_entrypoint_is_resolved_relative_to_manifest to verify the
reported worker path includes the manifest directory, or use a manifest-local
fixture that clearly distinguishes manifest-relative lookup from
current-directory/PATH resolution. Keep the check focused on load_worker_plugins
and the missing-worker entrypoint so the changed behavior is actually
observable.
- Around line 215-217: The manual cleanup at the end of the worker plugin test
is fragile because assertion panics can skip clear_plugin_configuration() and
activation.clear(), leaving state behind for later tests. Move this cleanup into
a Drop implementation for LoadedWorker so the activation and plugin
configuration are always released automatically, and update the large happy-path
test to use the LoadedWorker helper instead of calling clear() manually.
- Around line 114-178: The “all current surfaces” integration test in
worker_plugin_tests is missing assertions for some registered worker plugin
events, so it can pass even if proxy surfaces break. Extend the existing test
around flush_subscribers, assert_parent, find_event, and llm_call_execute to
verify the worker subscriber mark event (`fixture.worker.subscriber.mark`) plus
the LLM sanitize request/response flags (`worker_plugin_llm_sanitize_request`
and `worker_plugin_llm_sanitize_response`) after the LLM call and event flush.
Keep the new checks aligned with the existing event-capture flow so the test
covers lifecycle, scope, and middleware behavior end to end.
- Around line 621-623: The fixture manifests in worker_plugin_tests are
hard-coding relay compatibility, which will go stale as CARGO_PKG_VERSION
changes. Update the supported-case manifests to derive the relay range from the
host package version via a helper based on env!("CARGO_PKG_VERSION"), and use an
intentionally impossible range only in unsupported-compat tests; keep
[workspace.package].version as the version source of truth. Apply this pattern
consistently in the affected fixture blocks around the manifest setup used by
the worker plugin tests.
🪄 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: cf0b3c4c-1d1d-4fa4-9ad0-d2e256e39a91
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
crates/cli/Cargo.tomlcrates/cli/src/server.rscrates/core/Cargo.tomlcrates/core/src/plugin/dynamic.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/tests/unit/dynamic_worker_tests.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (16)
- GitHub Check: Go / Test (windows-arm64)
- GitHub Check: Go / Test (windows-amd64)
- GitHub Check: Rust / Test (linux-amd64)
- GitHub Check: Rust / Test (macos-arm64)
- GitHub Check: Node.js / Test (windows-arm64)
- GitHub Check: Rust / Test (windows-arm64)
- GitHub Check: Rust / Test (linux-arm64)
- GitHub Check: Python / Test (windows-amd64)
- GitHub Check: WebAssembly / Test (linux-amd64)
- GitHub Check: WebAssembly / Test (windows-amd64)
- GitHub Check: Python / Test (windows-arm64)
- GitHub Check: Node.js / Test (windows-amd64)
- GitHub Check: WebAssembly / Test (linux-arm64)
- GitHub Check: WebAssembly / Test (windows-arm64)
- GitHub Check: WebAssembly / Test (macos-arm64)
- GitHub Check: License Diff / Run
⚠️ CI failures not shown inline (3)
GitHub Actions: Fern Docs / Preview docs: feat(worker): activate grpc-v1 worker plugins in relay
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1mpr_number="${GITHUB_REF_NAME#pull-request/}"�[0m
�[36;1mpreview_id="pull-request-${pr_number}"�[0m
�[36;1mif OUTPUT="$("$GITHUB_WORKSPACE/source-checkout/node_modules/.bin/fern" generate --docs --preview --id "$preview_id" 2>&1)"; then�[0m
�[36;1m fern_exit=0�[0m
�[36;1melse�[0m
�[36;1m fern_exit=$?�[0m
�[36;1mfi�[0m
�[36;1mprintf '%s\n' "$OUTPUT"�[0m
�[36;1mif [[ "$fern_exit" -ne 0 ]]; then�[0m
�[36;1m echo "::error::Fern docs preview generation failed"�[0m
GitHub Actions: Fern Docs / 0_Preview docs.txt: feat(worker): activate grpc-v1 worker plugins in relay
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
GitHub Actions: Fern Docs / Preview docs: feat(worker): activate grpc-v1 worker plugins in relay
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
🧰 Additional context used
📓 Path-based instructions (19)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Use
snake_casenaming convention for Rust identifiers (e.g.,nemo_relay_tool_call)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node work
**/*.rs: Runcargo fmt --allto format all Rust code
Runcargo clippy --workspace --all-targets -- -D warningsto enforce all clippy lints as errors
**/*.rs: Runcargo fmt --allwhen Rust files changed as part of WebAssembly work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files changed as part of WebAssembly work
**/*.rs: If any Rust code changed, always runjust test-rust
If any Rust code changed, also runcargo fmt --all
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings
Run Rust formatting withcargo fmt --all
Run Rust linting withcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Usecargo fmtfor Rust code formatting
Runcargo clippy -- -D warningsto lint Rust code and treat all warnings as errors
Use Rust snake_case naming convention for Rust identifiers
Include SPDX license header in all Rust source files using double-slash comment syntax
Validate Rust code withuv run pre-commit run --all-filesto enforce cargo fmt formatting check, cargo clippy lints, and cargo deny aud...
Files:
crates/core/src/plugin/dynamic.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/cli/src/server.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_tests.rs
**/{Cargo.toml,**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Maintain consistency between Rust package names in
Cargo.tomland their actual usage across the codebase
Files:
crates/core/src/plugin/dynamic.rscrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/Cargo.tomlcrates/cli/src/server.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_tests.rs
**/*.{h,hpp,c,cpp,rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Ensure FFI header and library naming follows consistent conventions across platform-specific builds
Files:
crates/core/src/plugin/dynamic.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/cli/src/server.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_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.rscrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/Cargo.tomlcrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_tests.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Rust crate names and module prefixes during coordinated rename operations
Files:
crates/core/src/plugin/dynamic.rscrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/Cargo.tomlcrates/cli/src/server.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_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
crates/core/**/*.rs: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths. Keep errors explicit and binding-appropriate at the wrapper layer.
Files:
crates/core/src/plugin/dynamic.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_tests.rs
crates/{core,adaptive}/**
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full matrix across Rust, Python, Go, Node.js, and WebAssembly
Files:
crates/core/src/plugin/dynamic.rscrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/Cargo.tomlcrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_tests.rs
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.
Files:
crates/core/src/plugin/dynamic.rscrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/Cargo.tomlcrates/cli/src/server.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_tests.rs
**/*.{rs,py,go,js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions: Rust and Python use
snake_case, C FFI exports prefixednemo_relay_, Go usesPascalCasefor public APIs, Node.js usescamelCase.
Files:
crates/core/src/plugin/dynamic.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/cli/src/server.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_tests.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: Keep async behavior on the existing tokio-based model. Bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
UseJson = serde_json::Valuein Rust-facing runtime APIs for JSON payload handling.
Files:
crates/core/src/plugin/dynamic.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/cli/src/server.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_tests.rs
**
⚙️ CodeRabbit configuration file
**:AGENTS.md
This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.
Project Overview
NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go, WebAssembly, and the raw C FFI are experimental and source-first.
The shared runtime model is:
- Scope stacks decide where work belongs and which scope-local behavior is visible.
- Middleware registries decide what guardrails and intercepts run around managed calls.
- Plugins install reusable runtime behavior from configuration.
- Events record runtime behavior in ATOF form.
- Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.
Repository Structure
The repository layout separates the Rust runtime, language bindings, documentation,
integration patches, and agent-facing skills.crates/ core/ # Rust core runtime crate, published as nemo-relay adaptive/ # Adaptive runtime primitives and plugin components python/ # PyO3 native extension for the Python package ffi/ # Raw C ABI layer used by downstream bindings such as Go node/ # NAPI Node.js binding and JavaScript/TypeScript entry points wasm/ # wasm-bindgen WebAssembly binding and JS wrappers python/ nemo_relay/ # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers tests/ # Python tests go/ nemo_relay/ # Experimental Go CGo binding and tests fern/ # Fern documentation site scripts/ # Stable wrappers and helper scripts; build/test/docs entry points live in justfile third_party/ # P...
Files:
crates/core/src/plugin/dynamic.rscrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/Cargo.tomlcrates/cli/src/server.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_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.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_tests.rs
**/*.{py,txt,toml,cfg,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Python package names and top-level module imports during coordinated rename operations
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/Cargo.toml
**/Cargo.toml
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update WebAssembly crate names and generated package names during coordinated rename operations
Confirm or infer the target release version from
upstream/main:Cargo.toml. Derive the release branch asrelease/<major>.<minor>.
**/Cargo.toml: MaintainCargo.toml[workspace.package].versionas the source of truth for the Rust workspace and Python build versioning
KeepCargo.toml[workspace.dependencies]self-references aligned with the workspace version when the workspace version changes
After updating workspace package entries, runcargo check --workspaceto refreshCargo.lock
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/Cargo.toml
{docs/**,README.md,**/Cargo.toml,**/package.json,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Ensure renamed public surfaces are reflected consistently in manifests and docs for large or public-facing changes
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/Cargo.toml
**/*.{md,mdx,py,sh,yaml,yml,toml,json}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repo references, and build commands current
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/Cargo.toml
**/*.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license header in TOML configuration files using hash comment syntax
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/Cargo.toml
{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/fixtures/worker_plugin/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/tests/unit/dynamic_worker_tests.rs
{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Maintain documented and tested validation and report behavior for adaptive surfaces
Files:
crates/core/tests/integration/worker_plugin_tests.rscrates/core/tests/unit/dynamic_worker_tests.rs
🪛 GitHub Check: Rust / Test (windows-amd64)
crates/core/src/plugin/dynamic/worker.rs
[failure] 434-434:
unused variable: activation_dir
🔇 Additional comments (10)
crates/cli/Cargo.toml (1)
28-28: LGTM!crates/core/Cargo.toml (2)
138-142: LGTM!Also applies to: 186-187
76-89: 📐 Maintainability & Code QualityNo change needed —
tonicis already declared asoptional = true, sodep:tonicinworker-grpcis wired correctly.crates/core/src/plugin/dynamic.rs (1)
25-33: LGTM!crates/cli/src/server.rs (1)
13-14: LGTM!Also applies to: 220-220, 232-232, 257-273, 282-289, 313-313, 327-327
crates/core/tests/unit/dynamic_worker_tests.rs (1)
30-1092: LGTM!crates/core/tests/fixtures/worker_plugin/Cargo.toml (1)
1-22: LGTM!crates/core/tests/fixtures/worker_plugin/src/main.rs (3)
1-293: 📐 Maintainability & Code QualityVerify required Rust validation ran.
Please confirm this Rust change was validated with
cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings, andjust test-rust; for this cross-language worker surface,uv run pre-commit run --all-filesis also expected. As per coding guidelines, “Any Rust change must runjust test-rust”, “Any Rust change must runcargo fmt --all”, and “Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings.”Source: Coding guidelines
1-250: LGTM!Also applies to: 275-293
251-272: 🎯 Functional Correctness
emit_markdoes not take an isolated-stack argument. The fixture can’t be changed as suggested; if isolated mark coverage is needed, assert the emitted event’s parent/root instead.> Likely an incorrect or invalid review comment.
willkill07
force-pushed
the
wkk_dynamic-plugin-5-worker-activation
branch
from
38fa6ee to
41ce628
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/dynamic.rs`:
- Around line 25-33: The worker module export in dynamic.rs is only gated by
feature = "worker-grpc", but it also needs to be excluded on wasm32 because
worker.rs depends on host-only APIs. Update the gating around mod worker and pub
use worker::* so they are compiled only when worker-grpc is enabled and the
target is not wasm32, matching the existing native-only pattern used by native.
In `@crates/core/src/plugin/dynamic/worker.rs`:
- Around line 1171-1191: `invoke_blocking` leaks `continuation_id` and
`scope_stack_id` on transport/status failures because cleanup happens only after
a successful `client.invoke(...)`. Move the cleanup into `invoke_blocking` so
`WorkerHostRuntimeState::remove_continuation` and
`remove_invocation_scope_stack` run regardless of whether `block_on_handle`
returns `Ok` or `Err`, while keeping the existing
`InvokeRequest`/`FlowError::Internal` handling intact.
- Around line 583-592: The Python worker bootstrap in WorkerRuntime::Python
still defaults to python3, which breaks spawning on Windows when
NEMO_RELAY_PYTHON is unset. Update the default launcher selection in the command
निर्माण path to use a Windows-safe Python executable on Windows while keeping
the existing override via NEMO_RELAY_PYTHON and the current behavior elsewhere.
Use the WorkerRuntime::Python match arm and the Command::new setup as the place
to make the platform-specific default decision.
In `@crates/core/tests/unit/dynamic_worker_tests.rs`:
- Around line 451-470: The receiver-dropped cleanup path is not actually being
exercised because the fake worker stream fails immediately, so the test can pass
without proving teardown. Update
callback_stream_stops_when_host_receiver_is_dropped and the related fake worker
stream setup so the stream stays pending/blocking until the host drops it, then
use an explicit completion signal to assert the producer stopped because the
receiver was dropped. Keep the focus on the invoke_llm_stream_execution path and
the fake_callback_service / worker stream helpers so the test verifies the
lifecycle behavior instead of a shallow smoke check.
- Around line 646-669: The `drop_scope_stack` test in `dynamic_worker_tests.rs`
only verifies the ack and can miss a regression where `state.scope_stacks` is
not actually updated. After the successful `drop_scope_stack` call, re-query the
previously created `scope_stack_id` through the same `state.stack(...)`/lookup
path used earlier and assert it is no longer resolvable or returns `None`. Keep
the existing `create_scope_stack` and `drop_scope_stack` flow, but add the
post-drop verification so the test covers the actual deletion behavior of the
`drop_scope_stack` API.
🪄 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: 118e8f8a-5851-4673-b139-668ba8724ee8
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
crates/cli/Cargo.tomlcrates/cli/src/server.rscrates/core/Cargo.tomlcrates/core/src/plugin/dynamic.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/tests/unit/dynamic_worker_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (19)
**/{Cargo.toml,**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Maintain consistency between Rust package names in
Cargo.tomland their actual usage across the codebase
Files:
crates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/src/plugin/dynamic.rscrates/core/Cargo.tomlcrates/cli/src/server.rscrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Rust crate names and module prefixes during coordinated rename operations
Files:
crates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/src/plugin/dynamic.rscrates/core/Cargo.tomlcrates/cli/src/server.rscrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rs
**/*.{py,txt,toml,cfg,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Python package names and top-level module imports during coordinated rename operations
Files:
crates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/Cargo.toml
**/Cargo.toml
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update WebAssembly crate names and generated package names during coordinated rename operations
Confirm or infer the target release version from
upstream/main:Cargo.toml. Derive the release branch asrelease/<major>.<minor>.
**/Cargo.toml: MaintainCargo.toml[workspace.package].versionas the source of truth for the Rust workspace and Python build versioning
KeepCargo.toml[workspace.dependencies]self-references aligned with the workspace version when the workspace version changes
After updating workspace package entries, runcargo check --workspaceto refreshCargo.lock
Files:
crates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/Cargo.toml
{docs/**,README.md,**/Cargo.toml,**/package.json,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Ensure renamed public surfaces are reflected consistently in manifests and docs for large or public-facing changes
Files:
crates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/Cargo.toml
**/*.{md,mdx,py,sh,yaml,yml,toml,json}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repo references, and build commands current
Files:
crates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/Cargo.toml
**/*.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license header in TOML configuration files using hash comment syntax
Files:
crates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/Cargo.toml
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.
Files:
crates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/src/plugin/dynamic.rscrates/core/Cargo.tomlcrates/cli/src/server.rscrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rs
**
⚙️ CodeRabbit configuration file
**:AGENTS.md
This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.
Project Overview
NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go, WebAssembly, and the raw C FFI are experimental and source-first.
The shared runtime model is:
- Scope stacks decide where work belongs and which scope-local behavior is visible.
- Middleware registries decide what guardrails and intercepts run around managed calls.
- Plugins install reusable runtime behavior from configuration.
- Events record runtime behavior in ATOF form.
- Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.
Repository Structure
The repository layout separates the Rust runtime, language bindings, documentation,
integration patches, and agent-facing skills.crates/ core/ # Rust core runtime crate, published as nemo-relay adaptive/ # Adaptive runtime primitives and plugin components python/ # PyO3 native extension for the Python package ffi/ # Raw C ABI layer used by downstream bindings such as Go node/ # NAPI Node.js binding and JavaScript/TypeScript entry points wasm/ # wasm-bindgen WebAssembly binding and JS wrappers python/ nemo_relay/ # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers tests/ # Python tests go/ nemo_relay/ # Experimental Go CGo binding and tests fern/ # Fern documentation site scripts/ # Stable wrappers and helper scripts; build/test/docs entry points live in justfile third_party/ # P...
Files:
crates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/src/plugin/dynamic.rscrates/core/Cargo.tomlcrates/cli/src/server.rscrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_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/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/src/plugin/dynamic.rscrates/core/Cargo.tomlcrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rs
crates/{core,adaptive}/**
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full matrix across Rust, Python, Go, Node.js, and WebAssembly
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/src/plugin/dynamic.rscrates/core/Cargo.tomlcrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_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/fixtures/worker_plugin/Cargo.tomlcrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rs
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Use
snake_casenaming convention for Rust identifiers (e.g.,nemo_relay_tool_call)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node work
**/*.rs: Runcargo fmt --allto format all Rust code
Runcargo clippy --workspace --all-targets -- -D warningsto enforce all clippy lints as errors
**/*.rs: Runcargo fmt --allwhen Rust files changed as part of WebAssembly work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files changed as part of WebAssembly work
**/*.rs: If any Rust code changed, always runjust test-rust
If any Rust code changed, also runcargo fmt --all
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings
Run Rust formatting withcargo fmt --all
Run Rust linting withcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Usecargo fmtfor Rust code formatting
Runcargo clippy -- -D warningsto lint Rust code and treat all warnings as errors
Use Rust snake_case naming convention for Rust identifiers
Include SPDX license header in all Rust source files using double-slash comment syntax
Validate Rust code withuv run pre-commit run --all-filesto enforce cargo fmt formatting check, cargo clippy lints, and cargo deny aud...
Files:
crates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rs
**/*.{h,hpp,c,cpp,rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Ensure FFI header and library naming follows consistent conventions across platform-specific builds
Files:
crates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_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
crates/core/**/*.rs: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths. Keep errors explicit and binding-appropriate at the wrapper layer.
Files:
crates/core/src/plugin/dynamic.rscrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rs
**/*.{rs,py,go,js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions: Rust and Python use
snake_case, C FFI exports prefixednemo_relay_, Go usesPascalCasefor public APIs, Node.js usescamelCase.
Files:
crates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: Keep async behavior on the existing tokio-based model. Bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
UseJson = serde_json::Valuein Rust-facing runtime APIs for JSON payload handling.
Files:
crates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_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.rscrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rs
{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Maintain documented and tested validation and report behavior for adaptive surfaces
Files:
crates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/integration/worker_plugin_tests.rs
🔇 Additional comments (12)
crates/core/tests/integration/worker_plugin_tests.rs (5)
114-178: Already flagged: assert every registered surface actually ran.Still applies: the happy-path test does not assert the subscriber mark or LLM sanitize request/response events after the LLM call/flush.
215-217: Already flagged: use RAII cleanup for loaded workers.Still applies: assertion panics can skip
clear_plugin_configuration()andactivation.clear(), leaking global worker/plugin state into later tests.Also applies to: 787-791
621-623: Already flagged: derive supported relay compatibility from the host version.Still applies: supported fixture manifests hard-code
>=0.5,<1.0, and the unsupported case should use an intentionally impossible range.Also applies to: 654-659, 697-699, 722-727, 896-907
694-711: Already flagged: make relative-entrypoint resolution observable.Still applies: the assertion only checks a generic spawn failure, so it does not prove the entrypoint was resolved relative to the manifest directory.
4-25: 🎯 Functional CorrectnessNo additional gating needed
crates/core/Cargo.tomlalready setsrequired-features = ["worker-grpc"]for this test target.> Likely an incorrect or invalid review comment.crates/core/tests/fixtures/worker_plugin/Cargo.toml (1)
1-22: LGTM!crates/core/tests/fixtures/worker_plugin/src/main.rs (2)
1-293: 📐 Maintainability & Code QualityConfirm required Rust validation passed.
Please confirm
cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings, andjust test-rustpassed for this Rust change. As per coding guidelines, any Rust change must runjust test-rust,cargo fmt --all, andcargo clippy --workspace --all-targets -- -D warnings.Source: Coding guidelines
251-268: 🎯 Functional Correctness
emit_markcannot take the isolated stack handle here. The third argument is metadata (Option<Json>), not a scope reference, soSome(&isolated)is a type mismatch. If this fixture is meant to exercise isolated-stack emission, it needs a different API path.> Likely an incorrect or invalid review comment.crates/core/Cargo.toml (1)
76-93: 📐 Maintainability & Code QualityConfirm the required core validation matrix ran.
This change adds a new
crates/corefeature and test surface. Please confirmcargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,just test-rust,uv run pre-commit run --all-files,validate-change, and the Rust/Python/Go/Node/Wasm test matrix were run for this PR. As per coding guidelines: "**/*.rs: Any Rust change must runjust test-rust", "cargo fmt --all", and "cargo clippy --workspace --all-targets -- -D warnings", and "{crates/core,crates/adaptive}/**: Ifcrates/coreorcrates/adaptivechanged, run the full matrix across Rust, Python, Go, Node.js, and WebAssembly`."Also applies to: 138-142, 186-187
Source: Coding guidelines
crates/core/tests/unit/dynamic_worker_tests.rs (3)
1-448: LGTM!
472-645: LGTM!
671-1092: LGTM!
willkill07
force-pushed
the
wkk_dynamic-plugin-5-worker-activation
branch
from
41ce628 to
59c4f90
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/fixtures/worker_plugin/src/main.rs`:
- Around line 263-268: The isolated mark is being emitted without the isolated
scope, so it can attach to the current task stack instead of the isolated one.
Update the emit_mark call in the worker plugin fixture to use the same isolated
context established by fixture.worker.isolated.scope (the Some(&isolated)
task-local scope) and ensure the mark is emitted while that scope is active. Use
the emit_mark path in main.rs as the location to adjust the scope argument so
the root_uuid isolation behavior is preserved.
In `@crates/core/tests/integration/worker_plugin_tests.rs`:
- Around line 869-894: The worker plugin test is hardcoding a duplicated
Cargo.toml template instead of using the checked-in fixture manifest. Update the
fixture setup in worker_plugin_tests to read
tests/fixtures/worker_plugin/Cargo.toml, then only replace the nemo-relay-worker
path dependency with the copied build directory path before writing it into the
temp fixture; keep the rest of the manifest unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: de512a37-6c92-4330-9922-a9dcda35e944
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
crates/cli/Cargo.tomlcrates/cli/src/server.rscrates/core/Cargo.tomlcrates/core/src/plugin/dynamic.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/tests/unit/dynamic_worker_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (19)
**/{Cargo.toml,**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Maintain consistency between Rust package names in
Cargo.tomland their actual usage across the codebase
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/src/plugin/dynamic.rscrates/core/Cargo.tomlcrates/cli/src/server.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_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/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/src/plugin/dynamic.rscrates/core/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_tests.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Rust crate names and module prefixes during coordinated rename operations
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/src/plugin/dynamic.rscrates/core/Cargo.tomlcrates/cli/src/server.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_tests.rs
**/*.{py,txt,toml,cfg,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Python package names and top-level module imports during coordinated rename operations
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/Cargo.toml
**/Cargo.toml
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update WebAssembly crate names and generated package names during coordinated rename operations
Confirm or infer the target release version from
upstream/main:Cargo.toml. Derive the release branch asrelease/<major>.<minor>.
**/Cargo.toml: MaintainCargo.toml[workspace.package].versionas the source of truth for the Rust workspace and Python build versioning
KeepCargo.toml[workspace.dependencies]self-references aligned with the workspace version when the workspace version changes
After updating workspace package entries, runcargo check --workspaceto refreshCargo.lock
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/Cargo.toml
crates/{core,adaptive}/**
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full matrix across Rust, Python, Go, Node.js, and WebAssembly
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/src/plugin/dynamic.rscrates/core/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_tests.rs
{docs/**,README.md,**/Cargo.toml,**/package.json,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Ensure renamed public surfaces are reflected consistently in manifests and docs for large or public-facing changes
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/Cargo.toml
**/*.{md,mdx,py,sh,yaml,yml,toml,json}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repo references, and build commands current
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/Cargo.toml
**/*.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license header in TOML configuration files using hash comment syntax
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/Cargo.toml
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/src/plugin/dynamic.rscrates/core/Cargo.tomlcrates/cli/src/server.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_tests.rs
**
⚙️ CodeRabbit configuration file
**:AGENTS.md
This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.
Project Overview
NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go, WebAssembly, and the raw C FFI are experimental and source-first.
The shared runtime model is:
- Scope stacks decide where work belongs and which scope-local behavior is visible.
- Middleware registries decide what guardrails and intercepts run around managed calls.
- Plugins install reusable runtime behavior from configuration.
- Events record runtime behavior in ATOF form.
- Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.
Repository Structure
The repository layout separates the Rust runtime, language bindings, documentation,
integration patches, and agent-facing skills.crates/ core/ # Rust core runtime crate, published as nemo-relay adaptive/ # Adaptive runtime primitives and plugin components python/ # PyO3 native extension for the Python package ffi/ # Raw C ABI layer used by downstream bindings such as Go node/ # NAPI Node.js binding and JavaScript/TypeScript entry points wasm/ # wasm-bindgen WebAssembly binding and JS wrappers python/ nemo_relay/ # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers tests/ # Python tests go/ nemo_relay/ # Experimental Go CGo binding and tests fern/ # Fern documentation site scripts/ # Stable wrappers and helper scripts; build/test/docs entry points live in justfile third_party/ # P...
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/src/plugin/dynamic.rscrates/core/Cargo.tomlcrates/cli/src/server.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_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/fixtures/worker_plugin/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/tests/unit/dynamic_worker_tests.rs
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Use
snake_casenaming convention for Rust identifiers (e.g.,nemo_relay_tool_call)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node work
**/*.rs: Runcargo fmt --allto format all Rust code
Runcargo clippy --workspace --all-targets -- -D warningsto enforce all clippy lints as errors
**/*.rs: Runcargo fmt --allwhen Rust files changed as part of WebAssembly work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files changed as part of WebAssembly work
**/*.rs: If any Rust code changed, always runjust test-rust
If any Rust code changed, also runcargo fmt --all
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings
Run Rust formatting withcargo fmt --all
Run Rust linting withcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Usecargo fmtfor Rust code formatting
Runcargo clippy -- -D warningsto lint Rust code and treat all warnings as errors
Use Rust snake_case naming convention for Rust identifiers
Include SPDX license header in all Rust source files using double-slash comment syntax
Validate Rust code withuv run pre-commit run --all-filesto enforce cargo fmt formatting check, cargo clippy lints, and cargo deny aud...
Files:
crates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_tests.rs
**/*.{h,hpp,c,cpp,rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Ensure FFI header and library naming follows consistent conventions across platform-specific builds
Files:
crates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_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
crates/core/**/*.rs: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths. Keep errors explicit and binding-appropriate at the wrapper layer.
Files:
crates/core/src/plugin/dynamic.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_tests.rs
**/*.{rs,py,go,js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions: Rust and Python use
snake_case, C FFI exports prefixednemo_relay_, Go usesPascalCasefor public APIs, Node.js usescamelCase.
Files:
crates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_tests.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: Keep async behavior on the existing tokio-based model. Bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
UseJson = serde_json::Valuein Rust-facing runtime APIs for JSON payload handling.
Files:
crates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_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.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/unit/dynamic_worker_tests.rs
{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Maintain documented and tested validation and report behavior for adaptive surfaces
Files:
crates/core/tests/integration/worker_plugin_tests.rscrates/core/tests/unit/dynamic_worker_tests.rs
🔇 Additional comments (10)
crates/core/tests/unit/dynamic_worker_tests.rs (2)
451-470: Receiver-drop teardown is still unproven.
callback_stream_stops_when_host_receiver_is_droppedstill drops the host stream without ever making the producer block, becauseFakePluginWorker::invoke_streamexits immediately with a transport error. That means the test can pass even if the receiver-dropped cleanup path leaks or never cancels. Keep the fake stream pending and assert completion from an explicit signal when the receiver is dropped. As per path instructions, "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."Also applies to: 1064-1070
Source: Path instructions
646-669:drop_scope_stackstill is not verified end-to-end.This only checks the ack. If the implementation stops removing the entry from
state.scope_stacks, the test still passes and scope isolation regresses silently. Re-query the createdscope_stack_idafter the drop and assert it is no longer resolvable. As per path instructions, "Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant."Source: Path instructions
crates/core/Cargo.toml (1)
76-93: Feature/target mismatch reinforces the wasm gating concern.
worker-grpcunconditionally enablesdep:tower,dep:hyper-util, andtonic/transport, buttower/hyper-utilare only declared undercfg(not(target_arch = "wasm32")). Enablingworker-grpcfor a wasm build pulls in a transport stack that cannot compile there. The root-cause fix is gating the module/export (seecrates/core/src/plugin/dynamic.rs).Also applies to: 186-187
crates/core/src/plugin/dynamic.rs (1)
25-33: Gate the worker module/export offwasm32.
worker.rsdepends on host-only APIs (std::process, Tokio runtime/process, tonic transport, sockets). Gatingmod worker;/pub use worker::*;only onfeature = "worker-grpc"lets a wasm build with that feature enabled try to compile the host path. Match the native pattern and addnot(target_arch = "wasm32").crates/core/src/plugin/dynamic/worker.rs (2)
583-592: Windows-safe Python launcher default.
WorkerRuntime::Pythondefaults topython3whenNEMO_RELAY_PYTHONis unset. On Windows that executable is typically absent, so Python workers fail to spawn. Selectpythononcfg!(windows).
1173-1194: Continuations and scope stacks leak on failed invokes.Cleanup only runs after a successful
client.invoke(...); the?at the transport-error path returns early, socontinuation_id/scope_stack_idremain inWorkerHostRuntimeStatefor the rest of the activation. Capture the result, run removals unconditionally, then propagate.crates/cli/Cargo.toml (1)
28-28: LGTM!crates/cli/src/server.rs (1)
13-14: LGTM!Also applies to: 220-220, 232-232, 257-273, 282-289, 313-313, 327-327
crates/core/tests/fixtures/worker_plugin/Cargo.toml (1)
1-22: LGTM!crates/core/tests/fixtures/worker_plugin/src/main.rs (1)
1-293: 📐 Maintainability & Code QualityVerify the required Rust checks before merge.
Please confirm this Rust change was validated with
cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings, andjust test-rust.As per coding guidelines,
**/*.rs: “Any Rust change must runjust test-rust”, “Any Rust change must runcargo fmt --all”, and “Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings.”Source: Coding guidelines
willkill07
force-pushed
the
wkk_dynamic-plugin-5-worker-activation
branch
from
59c4f90 to
fc03b93
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/dynamic/worker.rs`:
- Around line 286-290: The host runtime server startup in load_one_worker_plugin
is fire-and-forget, so activation can succeed even if serve_host_runtime never
binds or exits early. Update the flow around runtime_handle.runtime().spawn and
serve_host_runtime to wait for a bind/readiness signal or surface startup errors
before continuing, using the existing host_server/host_state path so activation
fails fast when the endpoint is dead. Ensure the plugin activation path only
returns success after the host listener is confirmed ready, and propagate any
bind/server failure from the spawned task back to the caller.
- Around line 308-383: The worker RPC calls in worker registration and request
handling can hang indefinitely after connect because only the socket setup is
time-limited. Add a configured timeout/deadline wrapper around each client RPC
in the relevant paths, including the health, handshake, validate, register,
invoke, invoke_stream, and shutdown calls in the worker flow, and make sure the
timeout is applied through the existing runtime/block_on helpers so the async
callback paths still respect lifetimes and cancellation.
In `@crates/core/tests/unit/dynamic_worker_tests.rs`:
- Around line 106-119: Add coverage for the malformed StreamItem::Value case in
dynamic_worker_tests so json_from_stream_chunk is validated when the JSON
envelope cannot be decoded. Extend the existing assertions in the
json_from_stream_chunk test to construct a StreamChunk with a value payload that
triggers the decode failure and verify the returned error message/mapping,
alongside the current worker_error and empty-chunk checks, so the failure path
in json_from_stream_chunk stays locked in.
🪄 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: 8fdf35ea-4e05-4f9c-8515-b31856f00b3c
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
crates/cli/Cargo.tomlcrates/cli/src/server.rscrates/core/Cargo.tomlcrates/core/src/plugin/dynamic.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/tests/unit/dynamic_worker_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (19)
**/{Cargo.toml,**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Maintain consistency between Rust package names in
Cargo.tomland their actual usage across the codebase
Files:
crates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/core/Cargo.tomlcrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Rust crate names and module prefixes during coordinated rename operations
Files:
crates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/core/Cargo.tomlcrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rs
**/*.{py,txt,toml,cfg,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Python package names and top-level module imports during coordinated rename operations
Files:
crates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/Cargo.toml
**/Cargo.toml
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update WebAssembly crate names and generated package names during coordinated rename operations
Confirm or infer the target release version from
upstream/main:Cargo.toml. Derive the release branch asrelease/<major>.<minor>.
**/Cargo.toml: MaintainCargo.toml[workspace.package].versionas the source of truth for the Rust workspace and Python build versioning
KeepCargo.toml[workspace.dependencies]self-references aligned with the workspace version when the workspace version changes
After updating workspace package entries, runcargo check --workspaceto refreshCargo.lock
Files:
crates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/Cargo.toml
{docs/**,README.md,**/Cargo.toml,**/package.json,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Ensure renamed public surfaces are reflected consistently in manifests and docs for large or public-facing changes
Files:
crates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/Cargo.toml
**/*.{md,mdx,py,sh,yaml,yml,toml,json}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repo references, and build commands current
Files:
crates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/Cargo.toml
**/*.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license header in TOML configuration files using hash comment syntax
Files:
crates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/Cargo.toml
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.
Files:
crates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/core/Cargo.tomlcrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rs
**
⚙️ CodeRabbit configuration file
**:AGENTS.md
This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.
Project Overview
NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go, WebAssembly, and the raw C FFI are experimental and source-first.
The shared runtime model is:
- Scope stacks decide where work belongs and which scope-local behavior is visible.
- Middleware registries decide what guardrails and intercepts run around managed calls.
- Plugins install reusable runtime behavior from configuration.
- Events record runtime behavior in ATOF form.
- Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.
Repository Structure
The repository layout separates the Rust runtime, language bindings, documentation,
integration patches, and agent-facing skills.crates/ core/ # Rust core runtime crate, published as nemo-relay adaptive/ # Adaptive runtime primitives and plugin components python/ # PyO3 native extension for the Python package ffi/ # Raw C ABI layer used by downstream bindings such as Go node/ # NAPI Node.js binding and JavaScript/TypeScript entry points wasm/ # wasm-bindgen WebAssembly binding and JS wrappers python/ nemo_relay/ # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers tests/ # Python tests go/ nemo_relay/ # Experimental Go CGo binding and tests fern/ # Fern documentation site scripts/ # Stable wrappers and helper scripts; build/test/docs entry points live in justfile third_party/ # P...
Files:
crates/cli/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/core/Cargo.tomlcrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.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/fixtures/worker_plugin/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic.rscrates/core/Cargo.tomlcrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rs
crates/{core,adaptive}/**
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full matrix across Rust, Python, Go, Node.js, and WebAssembly
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic.rscrates/core/Cargo.tomlcrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.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/fixtures/worker_plugin/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/integration/worker_plugin_tests.rs
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Use
snake_casenaming convention for Rust identifiers (e.g.,nemo_relay_tool_call)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node work
**/*.rs: Runcargo fmt --allto format all Rust code
Runcargo clippy --workspace --all-targets -- -D warningsto enforce all clippy lints as errors
**/*.rs: Runcargo fmt --allwhen Rust files changed as part of WebAssembly work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files changed as part of WebAssembly work
**/*.rs: If any Rust code changed, always runjust test-rust
If any Rust code changed, also runcargo fmt --all
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings
Run Rust formatting withcargo fmt --all
Run Rust linting withcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Usecargo fmtfor Rust code formatting
Runcargo clippy -- -D warningsto lint Rust code and treat all warnings as errors
Use Rust snake_case naming convention for Rust identifiers
Include SPDX license header in all Rust source files using double-slash comment syntax
Validate Rust code withuv run pre-commit run --all-filesto enforce cargo fmt formatting check, cargo clippy lints, and cargo deny aud...
Files:
crates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rs
**/*.{h,hpp,c,cpp,rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Ensure FFI header and library naming follows consistent conventions across platform-specific builds
Files:
crates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.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
crates/core/**/*.rs: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths. Keep errors explicit and binding-appropriate at the wrapper layer.
Files:
crates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic.rscrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rs
**/*.{rs,py,go,js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions: Rust and Python use
snake_case, C FFI exports prefixednemo_relay_, Go usesPascalCasefor public APIs, Node.js usescamelCase.
Files:
crates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: Keep async behavior on the existing tokio-based model. Bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
UseJson = serde_json::Valuein Rust-facing runtime APIs for JSON payload handling.
Files:
crates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.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/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic.rscrates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/src/plugin/dynamic/worker.rs
{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Maintain documented and tested validation and report behavior for adaptive surfaces
Files:
crates/core/tests/unit/dynamic_worker_tests.rscrates/core/tests/integration/worker_plugin_tests.rs
🔇 Additional comments (9)
crates/core/src/plugin/dynamic/worker.rs (3)
583-586: Previous Windows Python launcher concern still applies.The fallback is still
python3; the earlier review already flagged using a Windows-safe default whenNEMO_RELAY_PYTHONis unset.
1173-1193: Previous failed-invoke cleanup concern still applies.The
?at Line 1184 still returns before removingcontinuation_idandscope_stack_idon transport/status errors.
1-2: 📐 Maintainability & Code QualityInclude the Rust/core validation results. This touches
crates/coreruntime behavior; attach the results forcargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,just test-rust, and the affected binding matrix.crates/core/tests/fixtures/worker_plugin/src/main.rs (1)
262-268: Previous isolated-mark scope concern still applies.
fixture.worker.isolated.markstill passesNone, so it is not emitted on the isolated stack created above.crates/cli/Cargo.toml (1)
28-28: LGTM!crates/core/Cargo.toml (1)
76-89: LGTM!Also applies to: 138-141, 186-187
crates/core/src/plugin/dynamic.rs (1)
25-33: LGTM!crates/cli/src/server.rs (1)
13-14: LGTM!Also applies to: 220-220, 232-232, 257-273, 282-289, 313-313, 327-327
crates/core/tests/fixtures/worker_plugin/Cargo.toml (1)
1-22: LGTM!
willkill07
force-pushed
the
wkk_dynamic-plugin-5-worker-activation
branch
from
fc03b93 to
0a24d90
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
crates/core/src/plugin/dynamic/worker.rs (1)
1173-1193: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRelease continuations and scope stacks on failed invokes.
?returns before cleanup whenclient.invoke(...)fails, socontinuationsandscope_stacksentries leak for the activation lifetime.Proposed fix
- let response = block_on_handle(&self.runtime, async move { + let result = block_on_handle(&self.runtime, async move { client.invoke(Request::new(request)).await }) - .map_err(|err| FlowError::Internal(format!("worker invoke failed: {err}")))? - .into_inner(); + .map(|response| response.into_inner()) + .map_err(|err| FlowError::Internal(format!("worker invoke failed: {err}"))); if !continuation_id.is_empty() { self.host_state.remove_continuation(&continuation_id); } @@ - Ok(response) + resultAs per path instructions,
crates/{core,adaptive}/**/*.rschanges should be reviewed for async correctness, scope isolation, and event lifecycle regressions.🤖 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/worker.rs` around lines 1173 - 1193, The invoke_blocking flow leaks activation state on invoke failure because the cleanup after the `client.invoke` call is skipped when `map_err(...)?` returns early. Update `invoke_blocking` to always release the `continuation_id` and `scope_stack_id` entries from `host_state` regardless of whether `block_on_handle`/`client.invoke` succeeds, and then return the invoke error afterward; keep the cleanup tied to the existing `continuation_id`, `scope_stack_id`, and `host_state.remove_*` calls so the lifecycle is handled consistently.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/core/src/plugin/dynamic/worker.rs`:
- Around line 452-458: The startup logic is reserving loopback ports too early
by calling unused_loopback_addr() and then dropping the listener before
HostRuntimeEndpoint::Tcp and WorkerConnectEndpoint::Tcp are used, which creates
a TOCTOU race. Update the worker setup in this constructor so the host listener
stays bound through startup, and avoid preselecting a released worker port. Use
the existing worker/host endpoint setup code to switch to a readiness or
advertisement flow that discovers or publishes the worker address without
requiring an unbound loopback port.
- Around line 1495-1503: The scope handle is being removed in the handler before
`optional_envelope_to_json` validates `request.output` and `request.metadata`,
so invalid payloads can consume `scope_handle_id` and break retry. In the
relevant method in `worker.rs`, move the `scope_handles` removal to after both
envelope conversions succeed, or otherwise ensure validation happens first and
only then call `remove(&request.scope_handle_id)`; keep the existing `Status`
error handling and preserve the host scope until the pop request is fully
validated.
In `@crates/core/tests/integration/worker_plugin_tests.rs`:
- Around line 859-891: The fixture build in build_fixture_worker is non-hermetic
because it creates an isolated temp crate and runs cargo build without a pinned
lockfile, causing dependency re-resolution on each test run. Update
build_fixture_worker to use a locked, repository-controlled dependency
resolution path, such as reusing the workspace build setup or invoking cargo
with a checked-in lockfile and locked mode, so the fixture no longer depends on
crates.io state.
- Around line 110-115: The test currently validates only the isolated scope path
and misses the actual regression surface in emit_runtime_events(), so extend
worker_plugin_tests to also assert the isolated mark behavior. After the first
flush_subscribers() in the existing test, add an assertion that
fixture.worker.isolated.mark is not attached to the ambient task-local stack and
that its parent/root isolation matches the isolated request context, using the
same captured_events assertions and outer_uuid/ScopeCategory pattern already
used for fixture.worker.isolated.scope.
---
Duplicate comments:
In `@crates/core/src/plugin/dynamic/worker.rs`:
- Around line 1173-1193: The invoke_blocking flow leaks activation state on
invoke failure because the cleanup after the `client.invoke` call is skipped
when `map_err(...)?` returns early. Update `invoke_blocking` to always release
the `continuation_id` and `scope_stack_id` entries from `host_state` regardless
of whether `block_on_handle`/`client.invoke` succeeds, and then return the
invoke error afterward; keep the cleanup tied to the existing `continuation_id`,
`scope_stack_id`, and `host_state.remove_*` calls so the lifecycle is handled
consistently.
🪄 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: 0081683e-d4da-4124-b562-0367aef87dea
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
crates/cli/Cargo.tomlcrates/cli/src/server.rscrates/core/Cargo.tomlcrates/core/src/plugin/dynamic.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/tests/unit/dynamic_worker_tests.rscrates/worker/src/lib.rscrates/worker/tests/worker_sdk_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 (19)
**/{Cargo.toml,**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Maintain consistency between Rust package names in
Cargo.tomland their actual usage across the codebase
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/worker/src/lib.rscrates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/worker/tests/worker_sdk_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/Cargo.tomlcrates/core/tests/unit/dynamic_worker_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/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/src/plugin/dynamic.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/Cargo.tomlcrates/core/tests/unit/dynamic_worker_tests.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Rust crate names and module prefixes during coordinated rename operations
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/worker/src/lib.rscrates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/worker/tests/worker_sdk_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/Cargo.tomlcrates/core/tests/unit/dynamic_worker_tests.rs
**/*.{py,txt,toml,cfg,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Python package names and top-level module imports during coordinated rename operations
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/Cargo.toml
**/Cargo.toml
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update WebAssembly crate names and generated package names during coordinated rename operations
Confirm or infer the target release version from
upstream/main:Cargo.toml. Derive the release branch asrelease/<major>.<minor>.
**/Cargo.toml: MaintainCargo.toml[workspace.package].versionas the source of truth for the Rust workspace and Python build versioning
KeepCargo.toml[workspace.dependencies]self-references aligned with the workspace version when the workspace version changes
After updating workspace package entries, runcargo check --workspaceto refreshCargo.lock
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/Cargo.toml
crates/{core,adaptive}/**
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full matrix across Rust, Python, Go, Node.js, and WebAssembly
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/core/src/plugin/dynamic.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/Cargo.tomlcrates/core/tests/unit/dynamic_worker_tests.rs
{docs/**,README.md,**/Cargo.toml,**/package.json,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Ensure renamed public surfaces are reflected consistently in manifests and docs for large or public-facing changes
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/Cargo.toml
**/*.{md,mdx,py,sh,yaml,yml,toml,json}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repo references, and build commands current
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/Cargo.toml
**/*.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license header in TOML configuration files using hash comment syntax
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/core/Cargo.toml
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/worker/src/lib.rscrates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/worker/tests/worker_sdk_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/Cargo.tomlcrates/core/tests/unit/dynamic_worker_tests.rs
**
⚙️ CodeRabbit configuration file
**:AGENTS.md
This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.
Project Overview
NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go, WebAssembly, and the raw C FFI are experimental and source-first.
The shared runtime model is:
- Scope stacks decide where work belongs and which scope-local behavior is visible.
- Middleware registries decide what guardrails and intercepts run around managed calls.
- Plugins install reusable runtime behavior from configuration.
- Events record runtime behavior in ATOF form.
- Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.
Repository Structure
The repository layout separates the Rust runtime, language bindings, documentation,
integration patches, and agent-facing skills.crates/ core/ # Rust core runtime crate, published as nemo-relay adaptive/ # Adaptive runtime primitives and plugin components python/ # PyO3 native extension for the Python package ffi/ # Raw C ABI layer used by downstream bindings such as Go node/ # NAPI Node.js binding and JavaScript/TypeScript entry points wasm/ # wasm-bindgen WebAssembly binding and JS wrappers python/ nemo_relay/ # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers tests/ # Python tests go/ nemo_relay/ # Experimental Go CGo binding and tests fern/ # Fern documentation site scripts/ # Stable wrappers and helper scripts; build/test/docs entry points live in justfile third_party/ # P...
Files:
crates/core/tests/fixtures/worker_plugin/Cargo.tomlcrates/cli/Cargo.tomlcrates/worker/src/lib.rscrates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/worker/tests/worker_sdk_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/Cargo.tomlcrates/core/tests/unit/dynamic_worker_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/fixtures/worker_plugin/Cargo.tomlcrates/worker/tests/worker_sdk_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/tests/unit/dynamic_worker_tests.rs
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Use
snake_casenaming convention for Rust identifiers (e.g.,nemo_relay_tool_call)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node work
**/*.rs: Runcargo fmt --allto format all Rust code
Runcargo clippy --workspace --all-targets -- -D warningsto enforce all clippy lints as errors
**/*.rs: Runcargo fmt --allwhen Rust files changed as part of WebAssembly work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files changed as part of WebAssembly work
**/*.rs: If any Rust code changed, always runjust test-rust
If any Rust code changed, also runcargo fmt --all
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings
Run Rust formatting withcargo fmt --all
Run Rust linting withcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Usecargo fmtfor Rust code formatting
Runcargo clippy -- -D warningsto lint Rust code and treat all warnings as errors
Use Rust snake_case naming convention for Rust identifiers
Include SPDX license header in all Rust source files using double-slash comment syntax
Validate Rust code withuv run pre-commit run --all-filesto enforce cargo fmt formatting check, cargo clippy lints, and cargo deny aud...
Files:
crates/worker/src/lib.rscrates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/worker/tests/worker_sdk_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/tests/unit/dynamic_worker_tests.rs
**/*.{h,hpp,c,cpp,rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Ensure FFI header and library naming follows consistent conventions across platform-specific builds
Files:
crates/worker/src/lib.rscrates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/worker/tests/worker_sdk_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/tests/unit/dynamic_worker_tests.rs
**/*.{rs,py,go,js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions: Rust and Python use
snake_case, C FFI exports prefixednemo_relay_, Go usesPascalCasefor public APIs, Node.js usescamelCase.
Files:
crates/worker/src/lib.rscrates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/worker/tests/worker_sdk_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/tests/unit/dynamic_worker_tests.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: Keep async behavior on the existing tokio-based model. Bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
UseJson = serde_json::Valuein Rust-facing runtime APIs for JSON payload handling.
Files:
crates/worker/src/lib.rscrates/core/src/plugin/dynamic.rscrates/cli/src/server.rscrates/worker/tests/worker_sdk_tests.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/tests/unit/dynamic_worker_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
crates/core/**/*.rs: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths. Keep errors explicit and binding-appropriate at the wrapper layer.
Files:
crates/core/src/plugin/dynamic.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/tests/unit/dynamic_worker_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.rscrates/core/tests/fixtures/worker_plugin/src/main.rscrates/core/src/plugin/dynamic/worker.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/tests/unit/dynamic_worker_tests.rs
{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Maintain documented and tested validation and report behavior for adaptive surfaces
Files:
crates/worker/tests/worker_sdk_tests.rscrates/core/tests/integration/worker_plugin_tests.rscrates/core/tests/unit/dynamic_worker_tests.rs
🔇 Additional comments (12)
crates/core/tests/unit/dynamic_worker_tests.rs (2)
106-119: Cover malformedStreamItem::Valuedecoding.
json_from_stream_chunkalso rejects a value chunk whose JSON envelope cannot be decoded, but this test only locks in the worker-error and empty-chunk branches. A regression in the decode/error-mapping path would still pass here.Suggested assertion
assert!( json_from_stream_chunk(StreamChunk { item: Some(StreamItem::Error(worker_error.clone())), }) .expect_err("stream worker error should surface") .to_string() .contains("worker.failed") ); + assert!( + json_from_stream_chunk(StreamChunk { + item: Some(StreamItem::Value(JsonEnvelope { + schema: JSON_SCHEMA.into(), + json: b"{".to_vec(), + })), + }) + .expect_err("invalid stream JSON envelope should fail") + .to_string() + .contains("invalid worker stream chunk") + ); assert!( json_from_stream_chunk(StreamChunk { item: None }) .expect_err("empty stream chunk should fail") .to_string() .contains("stream chunk was empty") );As per path instructions, "Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant."
Source: Path instructions
1-1205: 📐 Maintainability & Code QualityRun the required Rust checks. Attach the outputs for
cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,just test-rust, anduv run pre-commit run --all-files.crates/core/src/plugin/dynamic/worker.rs (3)
286-290: Still fail activation if the host runtime server never binds.The prior finding still applies:
serve_host_runtimeonly logs bind/server failures and the activation path continues with callbacks pointing at a potentially dead host endpoint.Also applies to: 464-503
584-586: Use a Windows-safe default Python launcher.The prior finding still applies: defaulting to
python3breaks Python workers on Windows unlessNEMO_RELAY_PYTHONis set.
308-383: Put deadlines on worker RPCs after connect.The prior finding still applies: health, handshake, validation, registration, invoke, stream-open, and shutdown RPCs can hang indefinitely after socket connection.
Also applies to: 1115-1117, 1173-1185
crates/core/Cargo.toml (2)
138-142: LGTM!Also applies to: 186-187
76-89: 📐 Maintainability & Code QualityRun the required Rust/core validation
Include the results forcargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,just test-rust, and the fullcrates/corematrix (just test-python,just test-go,just test-node,just test-wasm).crates/cli/Cargo.toml (1)
28-28: LGTM!crates/core/src/plugin/dynamic.rs (1)
25-33: LGTM!crates/cli/src/server.rs (1)
13-14: LGTM!Also applies to: 220-220, 232-232, 257-273, 282-289, 313-313, 327-327
crates/worker/src/lib.rs (1)
504-520: LGTM!crates/worker/tests/worker_sdk_tests.rs (1)
456-457: LGTM!Also applies to: 1207-1215
willkill07
force-pushed
the
wkk_dynamic-plugin-5-worker-activation
branch
from
0a24d90 to
b90f280
Compare
2 similar comments
Signed-off-by: Will Killian <wkillian@nvidia.com>
License DiffCompared against Lockfile license changesLockfile License ChangesRustAdded
Removed
Updated/Changed
NodeAdded
Removed
Updated/Changed
PythonAdded
Removed
Updated/Changed
Status output |
1 participant
Overview
Add the host activation and proxy layer for
grpc-v1worker dynamic plugins.Stack dependency: this PR cannot be merged until parent stacked PR #308 is merged: #308
Details
worker-grpc.Validation run across the completed stack:
cargo test -p nemo-relay-typescargo test -p nemo-relay-plugincargo test -p nemo-relay-worker-protocargo test -p nemo-relay-workercargo check -p nemo-relay-cliWhere should the reviewer start?
Start with
crates/core/src/plugin/dynamic/worker.rs, then reviewcrates/core/tests/integration/worker_plugin_tests.rs.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit