feat(plugin): add safe targeted LLM continuations to native API v2 - #594
feat(plugin): add safe targeted LLM continuations to native API v2#594bbednarski9 wants to merge 12 commits into
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:
WalkthroughNative plugin loading now supports API v1 and v2 negotiation. API v2 adds ABI-v4 typed unary and streaming LLM dispatch, target validation, structured failures, callback lifecycle handling, cancellation, and documentation. ChangesNative API v2 dispatch
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant NativePlugin
participant HostApiV4
participant DispatchContext
participant Provider
NativePlugin->>HostApiV4: Register unary or streaming middleware
NativePlugin->>HostApiV4: Submit typed continuation
HostApiV4->>DispatchContext: Scope validated target
DispatchContext->>Provider: Send targeted HTTP request
Provider-->>DispatchContext: Return response, stream event, or failure
DispatchContext-->>HostApiV4: Deliver typed outcome or event
HostApiV4-->>NativePlugin: Invoke callback
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
size:XL
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/core/src/plugin/dynamic/native.rs`:
- Around line 1773-1781: Update the blocking callback error path in the
`spawn_blocking(invoke).await` handling to reclaim `completion_ref`, `next_ref`,
and the owned host string in `invocation` before propagating the `JoinError`.
Mirror the cleanup performed by the inline panic branch, while preserving the
existing `FlowError::Internal` message for the join failure.
🪄 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: 65fd9b9c-c534-4ca1-b8ec-3e7d5a62ca43
📒 Files selected for processing (8)
crates/core/src/plugin/dynamic/native.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/README.mdcrates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Python / Test (windows-amd64)
🧰 Additional context used
📓 Path-based instructions (33)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.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/integration/native_plugin_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/plugin/README.mddocs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rs
**/*.{md,rst,html,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
**/*.{md,rst,html,txt}: Always spellNVIDIAin all caps. Do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun because the name starts with an 'en' sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company.
Use trademark symbols with product names only when the document type or legal guidance requires them.
Verify official capitalization, spacing, and hyphenation for product names.
Precede NVIDIA product names withNVIDIAon first mention when it is natural and accurate.
Do not rewrite product names for grammar or title-case rules.
Preserve third-party product names according to the owner's spelling.
Include the company name and full model qualifier on first use when it helps identify the model.
Preserve the official capitalization and punctuation of model names.
Use shorter family names only after the full name is established.
Spell out a term on first use and put the acronym in parentheses unless the acronym is widely understood by the intended audience.
Use the acronym on later mentions after it has been defined.
For long documents, reintroduce the full term if readers might lose context.
Form plurals of acronyms withs, not an apostrophe, such asGPUs.
In headings, common acronyms can remain abbreviated. Spell out the term in the first or second sentence of the body.
Common terms such asCPU,GPU,PC,API, andUIusually do not need to be spelled out for developer audiences.
Files:
crates/plugin/README.md
**/*.{md,rst,html}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
Link the first mention of a product name when the destination helps the reader.
Files:
crates/plugin/README.md
**/*.{md,rst,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
Spell
NVIDIAin all caps. Do not useNvidia,nvidia, orNV.
Files:
crates/plugin/README.md
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,rst}: Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text. Avoid raw URLs and weak anchors such as "here" or "read more."
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative steps. Keep steps parallel and split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce.
Preferrefer tooverseewhen the wording points readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical docs.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values.
Use numerals for 10 or greater and include commas in thousands.
Do not add trademark symbols to learning-oriented docs unless the source, platform, or legal guidance explicitly requires them.
Files:
crates/plugin/README.md
**/*.md
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.md: Use title case consistently in technical documentation headings
Avoid quotation marks, ampersands, and exclamation marks in headings
Keep product, event, research, and whitepaper names in their official title case
Use title case for table headers
Do not force social-media sentence case into technical docs
Format code elements, commands, parameters, package names, and expressions in monospace
Format directories, file names, and paths in monospace using backticks
Use angle brackets inside monospace for variables inside paths, such as/home/<username>/.login
Format error messages and strings in quotation marks, keeping literal code strings in code formatting when clearer
Format UI buttons, menus, fields, and labels in bold
Use angle brackets between UI labels for menu paths, such as File > Save As
Use italics for new terms on first use, sparingly and only when introducing the term
Use italics for publication titles
Format keyboard shortcuts in plain text, such as Press Ctrl+Alt+Delete
Use owner/repo link text for GitHub repositories, preferring[NVIDIA/NeMo](link)over prose references like 'the GitHub repo'
Introduce every code block with a complete sentence
Do not make a code block complete the grammar of the previous sentence
Do not continue a sentence after a code block
Use syntax highlighting when the format supports it for code blocks
Avoid the word 'snippet' unless the surrounding docs already use it as a term of art
Keep inline method, function, and class references consistent with nearby docs, omitting empty parentheses for prose readability when no call is shown
Use descriptive anchor text that matches the destination title when possible for links
Avoid raw URLs in running text
Avoid generic anchor text such as 'here,' 'this page,' and 'read more'
Include acronyms in link text when a linked term includes an acronym
Do not link long sentences or multiple sentences
Avoid links that pull readers away from a procedure unless the link is a p...
Files:
crates/plugin/README.md
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Update
README.md,fern/, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.
**/*.{md,mdx}: Prefer the documented public API, not internal shortcuts
Keep package names, repo references, and build commands current
Keep release-process and release-notes guidance in repo-maintainer docs such asRELEASING.md, not as user-facing docs pages orCHANGELOG.md
Keep stable user-facing wrappers atscripts/root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pagesIf links in documentation change, run
just docs-linkcheck.
Files:
crates/plugin/README.mddocs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
crates/plugin/README.mddocs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{crates/core/src/plugin/dynamic/**,crates/plugin/**,crates/worker/**,crates/worker-proto/**,crates/types/**,python/plugin/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Keep the stable boundary explicit: native plugins cross a C ABI, and worker plugins cross
grpc-v1.
Files:
crates/plugin/README.mddocs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.rs
**/*.{md,mdx,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Examples and documentation must use each exporter's documented flush/deregister order before shutdown.
Files:
crates/plugin/README.mddocs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
In MDX files, top-of-file comments must use JSX comment delimiters (
{/*to open and*/}to close); do not use HTML comments for MDX SPDX headers
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{crates/core/src/plugin/dynamic/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Native and worker plugins are trusted extensions; document that native plugins are in-process and unsandboxed, and worker plugins provide process isolation but not a security sandbox.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/core/src/plugin/dynamic/native.rs
{docs/build-plugins/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
When detailed dynamic plugin guides exist, keep Rust native, Python worker, and
grpc-v1protocol details on separate pages.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
docs/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If documentation examples or commands under
docs/change, run the targeted docs checks appropriate to the change.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.
Flag stale examples, missing SPDX headers where required, and instructions that no longer match CI or pre-commit behavior.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{crates/core/src/plugin/dynamic/**/*.rs,crates/plugin/**/*.rs,crates/worker/**/*.rs,crates/worker-proto/**/*.rs,python/plugin/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Manifest validation must cover kind, compatibility, load contract, integrity, capability mismatch, and disabled-plugin behavior.
Files:
crates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.rs
{crates/plugin/**/*.rs,python/plugin/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Rust and Python SDKs must expose every supported registration surface.
Files:
crates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/core/tests/fixtures/native_plugin/src/lib.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.rs
{crates/core/src/plugin/dynamic/**/*.rs,examples/rust-native-plugin/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not pass Rust runtime types, trait objects, futures, or allocator-owned strings across the native dynamic-library boundary.
Files:
crates/core/src/plugin/dynamic/native.rs
crates/core/src/plugin/dynamic/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
The native loader must keep libraries alive until registered callbacks are cleared and must deregister plugin kinds before unload.
Files:
crates/core/src/plugin/dynamic/native.rs
🧠 Learnings (1)
📚 Learning: 2026-07-28T20:07:29.880Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 571
File: crates/core/src/api/runtime/state.rs:996-1020
Timestamp: 2026-07-28T20:07:29.880Z
Learning: In NeMo Relay (RELAY-509), sanitizer callback failures must be treated as intentional fail-open behavior. When an event/tool (request/response) or LLM (request/response) sanitizer callback fails, the sanitizer chain should retain and publish the last valid event/payload snapshot (rather than dropping/invalidating the data) and log the failure including callback context (e.g., which sanitizer/callback failed and relevant identifiers). Apply this consistently across all sanitizer chains mentioned in the RELAY-509 documentation/migration guide.
Applied to files:
crates/core/src/plugin/dynamic/native.rs
🔇 Additional comments (29)
crates/plugin/src/lib.rs (9)
12-12: LGTM!Also applies to: 39-47
877-888: LGTM!
915-1087: LGTM!
1253-1324: LGTM!
2052-2059: LGTM!
2888-2935: LGTM!
3607-3629: LGTM!
3907-4013: LGTM!
4069-4099: LGTM!crates/plugin/tests/typed_callbacks.rs (1)
19-33: LGTM!Also applies to: 339-339, 374-379, 408-413, 423-441
crates/plugin/README.md (1)
34-34: LGTM!Also applies to: 52-54, 100-125
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx (1)
3-3: LGTM!Also applies to: 119-167, 169-175, 185-185
crates/core/src/plugin/dynamic/native.rs (11)
11-11: LGTM!Also applies to: 35-36, 45-72
335-342: LGTM!
389-405: LGTM!
814-829: LGTM!Also applies to: 890-890, 914-931
1433-1433: LGTM!
1535-1601: LGTM!
2295-2412: LGTM!
2415-2470: LGTM!
2626-2804: LGTM!
2806-2880: LGTM!
3136-3157: LGTM!Also applies to: 3246-3365, 3397-3458
crates/core/tests/fixtures/native_plugin/src/lib.rs (1)
300-328: LGTM!crates/core/tests/integration/native_plugin_tests.rs (1)
1201-1201: LGTM!Also applies to: 1249-1302
crates/core/tests/unit/native_plugin_tests.rs (4)
8-8: LGTM!Also applies to: 112-164
534-541: LGTM!
797-968: LGTM!
1030-1448: LGTM!
bbednarski9
changed the title
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx (1)
171-178: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse a complete sentence before the code block.
Line 171 relies on the code block to complete the phrase ending in
with:. Replace it with a standalone sentence, such as “Use the following macro to export a v2-only plugin.”As per coding guidelines, “Introduce every code block with a complete sentence.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx` around lines 171 - 178, Update the prose immediately before the nemo_relay_plugin_v2! code block to be a complete standalone sentence introducing the example, such as stating that the macro exports a v2-only plugin; leave the code block unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/cli/tests/coverage/shared/gateway_tests.rs`:
- Around line 692-756: Update the test
typed_dispatch_target_overrides_legacy_headers_without_exposing_transport_data
to assert effective.credential_policy equals
TargetCredentialPolicy::ExplicitTarget, matching
explicit_keyless_target_drops_source_credentials. Keep the existing header and
routing assertions unchanged.
In `@crates/core/src/api/runtime/continuation_context.rs`:
- Around line 109-121: Update invoke_with_llm_dispatch_target to delegate
through the existing invoke method instead of duplicating the callback future
wrapper. Preserve the current scoping order by keeping scope_llm_dispatch_target
as the outer operation while invoke handles Relay context restoration
internally.
In `@crates/core/src/api/runtime/llm_dispatch_context.rs`:
- Around line 18-25: Replace the derived Debug implementation for
LlmDispatchTargetContext with a manual one that prints method, route, and a URL
with its query string redacted, while representing headers by names only and
never values. Preserve the existing Clone, PartialEq, and Eq derives and ensure
formatting remains available to callers of current_llm_dispatch_target.
In `@crates/core/tests/unit/native_plugin_tests.rs`:
- Around line 946-993: Strengthen the assertions in
native_api_v2_rejects_prohibited_target_methods_and_headers and
native_api_v2_rejects_target_url_credentials by checking
assert_last_error_contains with the specific validation reason after each
prepare_typed_llm_dispatch failure, not just NemoRelayStatus::InvalidArg. Use
the existing assertion pattern near line 937 and match each case to its intended
method, header, or URL-credentials error message.
In `@crates/plugin/src/lib.rs`:
- Around line 963-1037: Update LlmNonHttpFailureKindV2 with an Unknown variant
and custom Deserialize implementation that maps unrecognized serialized strings
to Unknown, while preserving existing snake_case names and known-variant
behavior. Do not rely on #[serde(other)] or add #[non_exhaustive] to the public
structs LlmHttpFailureV2 and LlmNonHttpFailureV2.
---
Outside diff comments:
In `@docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx`:
- Around line 171-178: Update the prose immediately before the
nemo_relay_plugin_v2! code block to be a complete standalone sentence
introducing the example, such as stating that the macro exports a v2-only
plugin; leave the code block 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: 17d827ab-ebb7-4966-a303-9febc999ecdd
📒 Files selected for processing (12)
crates/cli/src/gateway/mod.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/README.mdcrates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (35)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/cli/src/gateway/mod.rscrates/core/src/plugin/dynamic/native.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/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/cli/src/gateway/mod.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/cli/src/gateway/mod.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/cli/src/gateway/mod.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/cli/src/gateway/mod.rscrates/core/src/plugin/dynamic/native.rs
crates/core/src/{api/**/*.rs,api/runtime/**/*.rs,codec/**/*.rs,json.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Implement the new or changed public runtime behavior first in the Rust core, especially under
crates/core/src/api/and related core modules such ascrates/core/src/api/runtime/,crates/core/src/codec/, andcrates/core/src/json.rs.
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/cli/src/server/mod.rscrates/plugin/src/lib.rscrates/cli/src/gateway/mod.rscrates/core/src/plugin/dynamic/native.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/core/src/api/runtime.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/cli/src/server/mod.rscrates/plugin/README.mdcrates/cli/tests/coverage/shared/gateway_tests.rscrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/cli/src/gateway/mod.rscrates/core/src/plugin/dynamic/native.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/cli/src/gateway/mod.rscrates/core/src/plugin/dynamic/native.rs
crates/core/src/api/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Preserve the documented pipeline order: conditional guardrails, request intercepts, request sanitization, execution intercepts, and response sanitization for tool and LLM execution; specialized sanitization, event creation, and dispatch for mark and scope events.
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/tests/unit/native_plugin_tests.rscrates/cli/src/gateway/mod.rscrates/core/src/plugin/dynamic/native.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/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
In MDX files, top-of-file comments must use JSX comment delimiters (
{/*to open and*/}to close); do not use HTML comments for MDX SPDX headers
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Update
README.md,fern/, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.
**/*.{md,mdx}: Prefer the documented public API, not internal shortcuts
Keep package names, repo references, and build commands current
Keep release-process and release-notes guidance in repo-maintainer docs such asRELEASING.md, not as user-facing docs pages orCHANGELOG.md
Keep stable user-facing wrappers atscripts/root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pagesIf links in documentation change, run
just docs-linkcheck.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/plugin/README.md
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/plugin/README.md
{crates/core/src/plugin/dynamic/**,crates/plugin/**,crates/worker/**,crates/worker-proto/**,crates/types/**,python/plugin/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Keep the stable boundary explicit: native plugins cross a C ABI, and worker plugins cross
grpc-v1.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/plugin/README.mdcrates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.rs
{crates/core/src/plugin/dynamic/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Native and worker plugins are trusted extensions; document that native plugins are in-process and unsandboxed, and worker plugins provide process isolation but not a security sandbox.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/core/src/plugin/dynamic/native.rs
{docs/build-plugins/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
When detailed dynamic plugin guides exist, keep Rust native, Python worker, and
grpc-v1protocol details on separate pages.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
docs/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If documentation examples or commands under
docs/change, run the targeted docs checks appropriate to the change.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
**/*.{md,mdx,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Examples and documentation must use each exporter's documented flush/deregister order before shutdown.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/plugin/README.md
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.
Flag stale examples, missing SPDX headers where required, and instructions that no longer match CI or pre-commit behavior.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
**/*.{md,rst,html,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
**/*.{md,rst,html,txt}: Always spellNVIDIAin all caps. Do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun because the name starts with an 'en' sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company.
Use trademark symbols with product names only when the document type or legal guidance requires them.
Verify official capitalization, spacing, and hyphenation for product names.
Precede NVIDIA product names withNVIDIAon first mention when it is natural and accurate.
Do not rewrite product names for grammar or title-case rules.
Preserve third-party product names according to the owner's spelling.
Include the company name and full model qualifier on first use when it helps identify the model.
Preserve the official capitalization and punctuation of model names.
Use shorter family names only after the full name is established.
Spell out a term on first use and put the acronym in parentheses unless the acronym is widely understood by the intended audience.
Use the acronym on later mentions after it has been defined.
For long documents, reintroduce the full term if readers might lose context.
Form plurals of acronyms withs, not an apostrophe, such asGPUs.
In headings, common acronyms can remain abbreviated. Spell out the term in the first or second sentence of the body.
Common terms such asCPU,GPU,PC,API, andUIusually do not need to be spelled out for developer audiences.
Files:
crates/plugin/README.md
**/*.{md,rst,html}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
Link the first mention of a product name when the destination helps the reader.
Files:
crates/plugin/README.md
**/*.{md,rst,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
Spell
NVIDIAin all caps. Do not useNvidia,nvidia, orNV.
Files:
crates/plugin/README.md
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,rst}: Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text. Avoid raw URLs and weak anchors such as "here" or "read more."
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative steps. Keep steps parallel and split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce.
Preferrefer tooverseewhen the wording points readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical docs.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values.
Use numerals for 10 or greater and include commas in thousands.
Do not add trademark symbols to learning-oriented docs unless the source, platform, or legal guidance explicitly requires them.
Files:
crates/plugin/README.md
**/*.md
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.md: Use title case consistently in technical documentation headings
Avoid quotation marks, ampersands, and exclamation marks in headings
Keep product, event, research, and whitepaper names in their official title case
Use title case for table headers
Do not force social-media sentence case into technical docs
Format code elements, commands, parameters, package names, and expressions in monospace
Format directories, file names, and paths in monospace using backticks
Use angle brackets inside monospace for variables inside paths, such as/home/<username>/.login
Format error messages and strings in quotation marks, keeping literal code strings in code formatting when clearer
Format UI buttons, menus, fields, and labels in bold
Use angle brackets between UI labels for menu paths, such as File > Save As
Use italics for new terms on first use, sparingly and only when introducing the term
Use italics for publication titles
Format keyboard shortcuts in plain text, such as Press Ctrl+Alt+Delete
Use owner/repo link text for GitHub repositories, preferring[NVIDIA/NeMo](link)over prose references like 'the GitHub repo'
Introduce every code block with a complete sentence
Do not make a code block complete the grammar of the previous sentence
Do not continue a sentence after a code block
Use syntax highlighting when the format supports it for code blocks
Avoid the word 'snippet' unless the surrounding docs already use it as a term of art
Keep inline method, function, and class references consistent with nearby docs, omitting empty parentheses for prose readability when no call is shown
Use descriptive anchor text that matches the destination title when possible for links
Avoid raw URLs in running text
Avoid generic anchor text such as 'here,' 'this page,' and 'read more'
Include acronyms in link text when a linked term includes an acronym
Do not link long sentences or multiple sentences
Avoid links that pull readers away from a procedure unless the link is a p...
Files:
crates/plugin/README.md
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/cli/tests/coverage/shared/gateway_tests.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_plugin_tests.rs
{crates/core/src/plugin/dynamic/**/*.rs,crates/plugin/**/*.rs,crates/worker/**/*.rs,crates/worker-proto/**/*.rs,python/plugin/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Manifest validation must cover kind, compatibility, load contract, integrity, capability mismatch, and disabled-plugin behavior.
Files:
crates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.rs
{crates/plugin/**/*.rs,python/plugin/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Rust and Python SDKs must expose every supported registration surface.
Files:
crates/plugin/tests/typed_callbacks.rscrates/plugin/src/lib.rs
{crates/core/src/plugin/dynamic/**/*.rs,examples/rust-native-plugin/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not pass Rust runtime types, trait objects, futures, or allocator-owned strings across the native dynamic-library boundary.
Files:
crates/core/src/plugin/dynamic/native.rs
crates/core/src/plugin/dynamic/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
The native loader must keep libraries alive until registered callbacks are cleared and must deregister plugin kinds before unload.
Files:
crates/core/src/plugin/dynamic/native.rs
🧠 Learnings (2)
📚 Learning: 2026-07-28T20:07:29.880Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 571
File: crates/core/src/api/runtime/state.rs:996-1020
Timestamp: 2026-07-28T20:07:29.880Z
Learning: In NeMo Relay (RELAY-509), sanitizer callback failures must be treated as intentional fail-open behavior. When an event/tool (request/response) or LLM (request/response) sanitizer callback fails, the sanitizer chain should retain and publish the last valid event/payload snapshot (rather than dropping/invalidating the data) and log the failure including callback context (e.g., which sanitizer/callback failed and relevant identifiers). Apply this consistently across all sanitizer chains mentioned in the RELAY-509 documentation/migration guide.
Applied to files:
crates/core/src/api/runtime.rscrates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
📚 Learning: 2026-07-28T03:31:05.964Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 564
File: crates/core/src/api/runtime/subscriber_dispatcher.rs:297-314
Timestamp: 2026-07-28T03:31:05.964Z
Learning: In this codebase’s runtime API, do not implement incremental native LLM stream forwarding via the native ABI v3 asynchronous middleware protocol (it can only settle a single JSON value via a one-shot completion handle and cannot forward stream chunks incrementally). If a latency-sensitive plugin needs streaming behavior, review for use of synchronous native stream intercepts or worker plugins instead of trying to chunk-deliver or incrementally forward over the ABI v3 async path.
Applied to files:
crates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rs
🔇 Additional comments (17)
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx (1)
174-182: 🎯 Functional CorrectnessThe documentation example is correct as written.
The
nemo_relay_plugin_v2!macro accepts any symbol name and generates a function with that name. Both native API v1 and v2 plugins can use the same symbol name (nemo_relay_register_plugin) because the safety mechanism depends on the manifest'scompat.native_apideclaration, not the symbol name. Whencompat.native_api = "2", the loader passes only the v4 host API; the generated plugin code validates the API version it receives. Symbol name reuse is safe because each plugin exists in a separate library with its own manifest configuration.> Likely an incorrect or invalid review comment.crates/plugin/src/lib.rs (1)
940-951: LGTM!Also applies to: 1039-1071
crates/plugin/README.md (1)
117-133: LGTM!crates/plugin/tests/typed_callbacks.rs (1)
38-98: LGTM!crates/core/src/api/runtime.rs (1)
9-9: LGTM!Also applies to: 27-28
crates/core/src/api/runtime/llm_dispatch_context.rs (1)
74-87: LGTM!crates/core/src/plugin/dynamic/native.rs (4)
2302-2372: LGTM!
2684-2762: LGTM!Also applies to: 2865-2913
2775-2801: 🗄️ Data Integrity & IntegrationNo target rebinding is needed in the producer task.
upstream.send().awaitcompletes beforesse_json_stream(response)constructs the returned stream. Producer polling only consumes the established response body.> Likely an incorrect or invalid review comment.
2383-2410: 🩺 Stability & AvailabilityNo cancellation arm is required
FlowErrorhas no cancellation, abort, or shutdown variant.LlmNonHttpFailureKindV2::Cancelledis emitted directly by stream lifecycle paths, not throughtyped_llm_failure.> Likely an incorrect or invalid review comment.crates/core/tests/unit/native_plugin_tests.rs (1)
131-168: LGTM!Also applies to: 815-944, 1015-1044, 1108-1320, 1323-1612
crates/cli/src/gateway/mod.rs (3)
29-30: LGTM!Also applies to: 145-145, 308-308, 317-317, 802-802, 812-823, 847-847, 875-893, 953-953, 963-970, 1169-1169
1-1: 🩺 Stability & Availability | 🏗️ Heavy liftPanic risk on malformed typed-target data is unverified and untested.
effective_dispatch_request_with_targetuses four.expect()calls that assume native-plugin-supplied header names, header values, HTTP method, and route strings were already validated. That validation is not visible in this review batch, and no test exercises the failure case.
crates/cli/src/gateway/mod.rs#L894-935: confirm (with the linked verification script) thatcrates/core/src/api/runtime/llm_dispatch_context.rsand the plugin SDK's route enum incrates/plugin/src/lib.rsfully guarantee these invariants; otherwise replace the.expect()calls with a gracefulFlowError/CliErrorpath.crates/cli/tests/coverage/shared/gateway_tests.rs#L692-756: add a test that constructs anLlmDispatchTargetContextwith an invalid header name/value, invalid method, or unrecognized route to prove the intended behavior (safe rejection, not a panic).
894-935: 🩺 Stability & AvailabilityRoute enum mismatch between plugin SDK and gateway is a latent maintenance risk; header and method validation is already enforced upstream.
LlmDispatchTargetContextcarries only untyped strings for method, route, and headers. The comments at lines 915, 917, and 926 state these values "were validated"—this is correct. Validation occurs incrates/core/src/plugin/dynamic/native.rsinprepare_typed_llm_dispatch()(lines 2302–2355) beforeLlmDispatchTargetContextis constructed:
- HTTP method (lines 2321–2328): validated via
reqwest::Method::from_bytes(), rejects CONNECT and TRACE.- Header names (lines 2330–2334): validated via
HeaderName::from_bytes().- Header values (lines 2341–2345): validated via
HeaderValue::from_str().The genuine risk is the route enum mismatch.
LlmDispatchRouteV2in the plugin SDK defines exactly three variants:OpenaiChat,OpenaiResponses,AnthropicMessages. All three map viaas_str()to strings thatProviderRoute::from_dispatch_override()accepts (lines 67, 71, 77). If the plugin SDK adds or renames a route variant without a corresponding update tofrom_dispatch_override(), the string will not match any arm, and the.expect()at line 924 panics.Enforcing sync between the plugin SDK's route enum and gateway route acceptance is the practical value. Header and method
.expect()calls are safe because validation is exhaustive upstream; the route.expect()is a maintenance invariant that requires code review discipline.crates/cli/src/server/mod.rs (1)
63-63: LGTM!Also applies to: 546-560
crates/cli/tests/coverage/shared/gateway_tests.rs (2)
16-16: LGTM!Also applies to: 2068-2068, 2107-2107
692-756: 🩺 Stability & Availability | 🏗️ Heavy liftMissing negative-path coverage for malformed dispatch targets.
This test only covers a well-formed
LlmDispatchTargetContext.effective_dispatch_request_with_targetincrates/cli/src/gateway/mod.rspanics via.expect()on invalid header names/values, an invalid method, or an unrecognized route string. Add a test constructing a target with one such invalid value to confirm the intended behavior (either a documented panic-is-unreachable invariant, backed by upstream validation, or a graceful error). This note is tied to the panic-risk finding raised oncrates/cli/src/gateway/mod.rs(lines 894-935); see the consolidated comment.As per path instructions, tests in this path should cover "error paths" for the changed API surface.
Source: Path instructions
| #[test] | ||
| fn typed_dispatch_target_overrides_legacy_headers_without_exposing_transport_data() { | ||
| let original_body = Bytes::from_static(br#"{"model":"original"}"#); | ||
| let mut original_headers = HeaderMap::new(); | ||
| original_headers.insert( | ||
| header::AUTHORIZATION, | ||
| HeaderValue::from_static("Bearer source-secret"), | ||
| ); | ||
| original_headers.insert("x-source", HeaderValue::from_static("source")); | ||
| let request = LlmRequest { | ||
| headers: Map::from_iter([ | ||
| ( | ||
| INTERNAL_DISPATCH_URL_HEADER.to_string(), | ||
| json!("http://attacker.invalid/v1/chat/completions"), | ||
| ), | ||
| ("authorization".into(), json!("Bearer request-secret")), | ||
| ("x-request".into(), json!("request")), | ||
| ]), | ||
| content: json!({"model": "selected"}), | ||
| }; | ||
| let target = LlmDispatchTargetContext::new( | ||
| "POST".into(), | ||
| "http://selected.invalid/v1/responses".into(), | ||
| "openai_responses".into(), | ||
| BTreeMap::from([ | ||
| ("authorization".into(), "Bearer target-secret".into()), | ||
| ("x-target".into(), "selected".into()), | ||
| ]), | ||
| ); | ||
|
|
||
| let effective = effective_dispatch_request_with_target( | ||
| &original_body, | ||
| &original_headers, | ||
| Some(&request), | ||
| Some(&target), | ||
| "http://default.invalid/v1/chat/completions", | ||
| &Method::GET, | ||
| ProviderRoute::OpenAiChatCompletions, | ||
| ); | ||
|
|
||
| assert_eq!(effective.method, Method::POST); | ||
| assert_eq!( | ||
| effective.url, | ||
| "http://selected.invalid/v1/responses".to_string() | ||
| ); | ||
| assert_eq!(effective.target_route, ProviderRoute::OpenAiResponses); | ||
| assert_eq!( | ||
| effective.headers.get(header::AUTHORIZATION).unwrap(), | ||
| "Bearer target-secret" | ||
| ); | ||
| assert_eq!(effective.headers.get("x-target").unwrap(), "selected"); | ||
| assert_eq!( | ||
| effective.headers.get(header::CONTENT_TYPE).unwrap(), | ||
| "application/json" | ||
| ); | ||
| assert!(effective.headers.get("x-source").is_none()); | ||
| assert!(effective.headers.get("x-request").is_none()); | ||
| assert!( | ||
| effective | ||
| .headers | ||
| .get(INTERNAL_DISPATCH_URL_HEADER) | ||
| .is_none() | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Add an assertion on credential_policy for the typed-target test.
The sibling test explicit_keyless_target_drops_source_credentials asserts effective.credential_policy, but this new test does not. Add the same assertion here to confirm the typed-target branch marks the request TargetCredentialPolicy::ExplicitTarget, keeping the contract check explicit rather than implied by the absence of source headers.
As per path instructions, tests matching {crates/**/tests/**,...} should "cover the behavior promised by the changed API surface" with assertions on the relevant contract fields rather than indirect checks.
✅ Suggested assertion addition
assert_eq!(effective.target_route, ProviderRoute::OpenAiResponses);
+ assert_eq!(
+ effective.credential_policy,
+ TargetCredentialPolicy::ExplicitTarget
+ );
assert_eq!(
effective.headers.get(header::AUTHORIZATION).unwrap(),
"Bearer target-secret"
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[test] | |
| fn typed_dispatch_target_overrides_legacy_headers_without_exposing_transport_data() { | |
| let original_body = Bytes::from_static(br#"{"model":"original"}"#); | |
| let mut original_headers = HeaderMap::new(); | |
| original_headers.insert( | |
| header::AUTHORIZATION, | |
| HeaderValue::from_static("Bearer source-secret"), | |
| ); | |
| original_headers.insert("x-source", HeaderValue::from_static("source")); | |
| let request = LlmRequest { | |
| headers: Map::from_iter([ | |
| ( | |
| INTERNAL_DISPATCH_URL_HEADER.to_string(), | |
| json!("http://attacker.invalid/v1/chat/completions"), | |
| ), | |
| ("authorization".into(), json!("Bearer request-secret")), | |
| ("x-request".into(), json!("request")), | |
| ]), | |
| content: json!({"model": "selected"}), | |
| }; | |
| let target = LlmDispatchTargetContext::new( | |
| "POST".into(), | |
| "http://selected.invalid/v1/responses".into(), | |
| "openai_responses".into(), | |
| BTreeMap::from([ | |
| ("authorization".into(), "Bearer target-secret".into()), | |
| ("x-target".into(), "selected".into()), | |
| ]), | |
| ); | |
| let effective = effective_dispatch_request_with_target( | |
| &original_body, | |
| &original_headers, | |
| Some(&request), | |
| Some(&target), | |
| "http://default.invalid/v1/chat/completions", | |
| &Method::GET, | |
| ProviderRoute::OpenAiChatCompletions, | |
| ); | |
| assert_eq!(effective.method, Method::POST); | |
| assert_eq!( | |
| effective.url, | |
| "http://selected.invalid/v1/responses".to_string() | |
| ); | |
| assert_eq!(effective.target_route, ProviderRoute::OpenAiResponses); | |
| assert_eq!( | |
| effective.headers.get(header::AUTHORIZATION).unwrap(), | |
| "Bearer target-secret" | |
| ); | |
| assert_eq!(effective.headers.get("x-target").unwrap(), "selected"); | |
| assert_eq!( | |
| effective.headers.get(header::CONTENT_TYPE).unwrap(), | |
| "application/json" | |
| ); | |
| assert!(effective.headers.get("x-source").is_none()); | |
| assert!(effective.headers.get("x-request").is_none()); | |
| assert!( | |
| effective | |
| .headers | |
| .get(INTERNAL_DISPATCH_URL_HEADER) | |
| .is_none() | |
| ); | |
| } | |
| #[test] | |
| fn typed_dispatch_target_overrides_legacy_headers_without_exposing_transport_data() { | |
| let original_body = Bytes::from_static(br#"{"model":"original"}"#); | |
| let mut original_headers = HeaderMap::new(); | |
| original_headers.insert( | |
| header::AUTHORIZATION, | |
| HeaderValue::from_static("Bearer source-secret"), | |
| ); | |
| original_headers.insert("x-source", HeaderValue::from_static("source")); | |
| let request = LlmRequest { | |
| headers: Map::from_iter([ | |
| ( | |
| INTERNAL_DISPATCH_URL_HEADER.to_string(), | |
| json!("http://attacker.invalid/v1/chat/completions"), | |
| ), | |
| ("authorization".into(), json!("Bearer request-secret")), | |
| ("x-request".into(), json!("request")), | |
| ]), | |
| content: json!({"model": "selected"}), | |
| }; | |
| let target = LlmDispatchTargetContext::new( | |
| "POST".into(), | |
| "http://selected.invalid/v1/responses".into(), | |
| "openai_responses".into(), | |
| BTreeMap::from([ | |
| ("authorization".into(), "Bearer target-secret".into()), | |
| ("x-target".into(), "selected".into()), | |
| ]), | |
| ); | |
| let effective = effective_dispatch_request_with_target( | |
| &original_body, | |
| &original_headers, | |
| Some(&request), | |
| Some(&target), | |
| "http://default.invalid/v1/chat/completions", | |
| &Method::GET, | |
| ProviderRoute::OpenAiChatCompletions, | |
| ); | |
| assert_eq!(effective.method, Method::POST); | |
| assert_eq!( | |
| effective.url, | |
| "http://selected.invalid/v1/responses".to_string() | |
| ); | |
| assert_eq!(effective.target_route, ProviderRoute::OpenAiResponses); | |
| assert_eq!( | |
| effective.credential_policy, | |
| TargetCredentialPolicy::ExplicitTarget | |
| ); | |
| assert_eq!( | |
| effective.headers.get(header::AUTHORIZATION).unwrap(), | |
| "Bearer target-secret" | |
| ); | |
| assert_eq!(effective.headers.get("x-target").unwrap(), "selected"); | |
| assert_eq!( | |
| effective.headers.get(header::CONTENT_TYPE).unwrap(), | |
| "application/json" | |
| ); | |
| assert!(effective.headers.get("x-source").is_none()); | |
| assert!(effective.headers.get("x-request").is_none()); | |
| assert!( | |
| effective | |
| .headers | |
| .get(INTERNAL_DISPATCH_URL_HEADER) | |
| .is_none() | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/cli/tests/coverage/shared/gateway_tests.rs` around lines 692 - 756,
Update the test
typed_dispatch_target_overrides_legacy_headers_without_exposing_transport_data
to assert effective.credential_policy equals
TargetCredentialPolicy::ExplicitTarget, matching
explicit_keyless_target_drops_source_credentials. Keep the existing header and
routing assertions unchanged.
Source: Path instructions
| /// Bounded HTTP failure returned by a provider. | ||
| #[derive(Debug, Clone, PartialEq, Eq, Serialize, serde::Deserialize)] | ||
| pub struct LlmHttpFailureV2 { | ||
| /// Provider HTTP status. | ||
| pub status: u16, | ||
| /// Bounded provider response body. | ||
| pub body: String, | ||
| /// Safe response headers with credential-bearing fields removed. | ||
| pub headers: BTreeMap<String, String>, | ||
| } | ||
|
|
||
| /// Stable non-HTTP failure classification exposed through native API v2. | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, serde::Deserialize)] | ||
| #[serde(rename_all = "snake_case")] | ||
| pub enum LlmNonHttpFailureKindV2 { | ||
| /// Provider connection could not be established or was interrupted. | ||
| Transport, | ||
| /// Provider request timed out. | ||
| Timeout, | ||
| /// The caller cancelled the operation. | ||
| Cancelled, | ||
| /// Relay rejected an invalid dispatch request. | ||
| InvalidRequest, | ||
| /// A guardrail rejected the provider call. | ||
| Guardrail, | ||
| /// Relay could not complete the operation. | ||
| Internal, | ||
| } | ||
|
|
||
| /// Bounded failure for an operation that produced no provider HTTP response. | ||
| #[derive(Debug, Clone, PartialEq, Eq, Serialize, serde::Deserialize)] | ||
| pub struct LlmNonHttpFailureV2 { | ||
| /// Stable failure kind. | ||
| pub kind: LlmNonHttpFailureKindV2, | ||
| /// Bounded human-readable context. | ||
| pub message: String, | ||
| } | ||
|
|
||
| /// Structured LLM continuation failure exposed through native API v2. | ||
| #[derive(Debug, Clone, PartialEq, Eq, Serialize, serde::Deserialize)] | ||
| #[serde(tag = "kind", rename_all = "snake_case")] | ||
| pub enum LlmCallFailureV2 { | ||
| /// A provider returned a non-success HTTP response. | ||
| Http { | ||
| /// Bounded HTTP failure details. | ||
| failure: LlmHttpFailureV2, | ||
| }, | ||
| /// No provider HTTP response was available. | ||
| NonHttp { | ||
| /// Bounded non-HTTP failure details. | ||
| failure: LlmNonHttpFailureV2, | ||
| }, | ||
| } | ||
|
|
||
| impl LlmCallFailureV2 { | ||
| /// Return Relay's provider-neutral retry disposition. | ||
| /// | ||
| /// The disposition is derived rather than serialized so the wire contract | ||
| /// contains only HTTP semantics and the minimal non-HTTP failure kind. | ||
| pub const fn is_retryable(&self) -> bool { | ||
| match self { | ||
| Self::Http { failure } => is_retryable_http_status_v2(failure.status), | ||
| Self::NonHttp { failure } => matches!( | ||
| failure.kind, | ||
| LlmNonHttpFailureKindV2::Transport | LlmNonHttpFailureKindV2::Timeout | ||
| ), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Return Relay's provider-neutral retry disposition for an HTTP status. | ||
| #[must_use] | ||
| pub const fn is_retryable_http_status_v2(status: u16) -> bool { | ||
| matches!(status, 408 | 425 | 429 | 500 | 502 | 503 | 504) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
serde other attribute unit variant deserialize unknown enum value requirements
💡 Result:
To deserialize an unknown enum value in Serde, the recommended approach depends on the enum representation and whether you want to capture the unknown value or simply map it to a catch-all variant. 1. Using #[serde(other)] For internally or adjacently tagged enums, you can use the #[serde(other)] attribute on a unit variant [1][2]. This variant will be deserialized whenever the input tag does not match any of the defined variants [1]. Example: #[derive(Deserialize)] #[serde(tag = "type")] enum MyEnum { VariantA, VariantB, #[serde(other)] Unknown, } 2. Capturing Unknown Values If you need to capture the unknown value (e.g., store it as a string), #[serde(other)] is typically limited to unit variants [3]. To capture the data, common patterns include: - Untagged Enums: Use #[serde(untagged)] on a variant that can hold the data (e.g., a newtype variant) [4][5][3]. - Custom Deserialization: Implement a custom Visitor or use the #[serde(from = "...")] attribute with a From implementation to handle the conversion logic manually [3]. 3. Other Considerations - The #[serde(default)] attribute is used for different purposes, such as providing default values for missing fields in internally tagged enums, and should not be confused with the requirement for handling unknown variant tags [6][7]. - For field-based deserialization (e.g., struct keys), the #[serde(deny_unknown_fields)] attribute controls whether unknown keys trigger an error, which is separate from the enum variant handling described above [4].
Citations:
- 1: https://serde.rs/variant-attrs.html
- 2: https://hkalbasi.github.io/serde-rs.github.io/variant-attrs.html
- 3: https://stackoverflow.com/questions/57469527/how-can-i-support-an-unknown-or-other-value-for-a-serde-enum
- 4: Tagged enums should support #[serde(other)] serde-rs/serde#912
- 5: https://serde.rs/enum-representations.html
- 6: Support #[serde(default)] as a variant attribute for when the tag is not present serde-rs/serde#2985
- 7: Add ability to use
#[serde(default)]to deserialize internally tagged enums when no tag is provided serde-rs/serde#2476
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- serde declarations ---'
rg -n --glob 'Cargo.toml' 'serde(\s*=|[[:space:]]*\{)|serde_json' .
printf '%s\n' '--- DTO definitions and uses ---'
rg -n -C 4 'LlmCallFailureV2|LlmNonHttpFailureKindV2|LlmHttpFailureV2|LlmNonHttpFailureV2|serde_json::from_value|from_value::<|from_value\(' crates python
printf '%s\n' '--- relevant manifest sections ---'
for f in $(rg -l --glob 'Cargo.toml' 'serde'); do
echo "### $f"
sed -n '1,180p' "$f"
doneRepository: NVIDIA/NeMo-Relay
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- workspace serde dependency declarations ---'
rg -n -C 3 --glob 'Cargo.toml' '^[[:space:]]*(serde|serde_json)[[:space:]]*=' .
printf '%s\n' '--- target DTO definition ---'
rg -n -C 12 'pub (struct|enum) Llm(HttpFailureV2|NonHttpFailureKindV2|NonHttpFailureV2|CallFailureV2)|impl LlmCallFailureV2' crates/plugin/src/lib.rs
printf '%s\n' '--- target DTO references only ---'
rg -n -C 5 'Llm(CallFailureV2|NonHttpFailureKindV2|HttpFailureV2|NonHttpFailureV2)' crates/plugin crates/core crates/worker crates/ffi crates/node crates/python
printf '%s\n' '--- plugin JSON deserialization references ---'
rg -n -C 6 'serde_json::from_value|from_slice|Deserialize' crates/plugin/src crates/plugin/testsRepository: NVIDIA/NeMo-Relay
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- locked serde versions ---'
rg -n -A 8 -B 2 'name = "(serde|serde_derive|serde_json)"' Cargo.lock
printf '%s\n' '--- all serde(other) uses and enum representations ---'
rg -n -C 5 '#\[serde\(other\)\]|#\[serde\(tag|#\[serde\(untagged\)|#\[serde\(content' crates examples pythonRepository: NVIDIA/NeMo-Relay
Length of output: 21426
🌐 Web query:
Serde #[serde(other)] externally tagged enum support exact error serde 1.x
💡 Result:
The attribute #[serde(other)] is officially supported only for unit variants within internally tagged or adjacently tagged enums [1]. It is not supported for externally tagged enums [2]. When used on an externally tagged enum, the attribute does not function as a catch-all for unknown variants [3]. While using #[serde(other)] in this context may compile without an explicit error in some versions of Serde, it will fail to produce the expected behavior (i.e., it will not capture unknown variants) [3]. There is no native "exact error" raised by the compiler for this specific misconfiguration in standard Serde 1.x, which has led to long-standing community requests for it to either be fully supported or to trigger a hard compile-time error [3][2]. For handling unknown variants in externally tagged enums, the common workarounds include: 1. Implementing a custom Deserialize trait using a visitor pattern [4]. 2. Using an "untagged" variant to capture raw data [5][4]. For example: #[derive(Deserialize)] #[serde(untagged)] enum MyEnum { VariantA, Other(serde_json::Value), // Captures everything that doesn't match VariantA } 3. Using external crates like serde-enum-str for specific string-based enum deserialization scenarios [4].
Citations:
- 1: https://serde.rs/variant-attrs.html
- 2: Give
#[serde(other)]functionality in externally tagged enums and on newtype variants serde-rs/serde#2365 - 3:
#[serde(other)]and externally tagged enum serde-rs/serde#2010 - 4: https://stackoverflow.com/questions/57469527/how-can-i-support-an-unknown-or-other-value-for-a-serde-enum
- 5: Tagged enums should support #[serde(other)] serde-rs/serde#912
Add a custom fallback for unknown failure kinds.
#[serde(other)] does not catch unknown values for the externally tagged representation used by LlmNonHttpFailureKindV2. #[non_exhaustive] only affects Rust pattern matching and does not change deserialization. Implement custom Deserialize to map unknown strings to an Unknown variant. Do not apply #[non_exhaustive] to public structs because it prevents downstream struct literals.
🤖 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/plugin/src/lib.rs` around lines 963 - 1037, Update
LlmNonHttpFailureKindV2 with an Unknown variant and custom Deserialize
implementation that maps unrecognized serialized strings to Unknown, while
preserving existing snake_case names and known-variant behavior. Do not rely on
#[serde(other)] or add #[non_exhaustive] to the public structs LlmHttpFailureV2
and LlmNonHttpFailureV2.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/plugin/README.md (1)
124-126: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSeparate normal completion from cancellation.
Document the normal path as requesting events through
DoneorFailure, then releasing the provider stream exactly once. If the caller stops before a terminal event, document calling cancellation once before releasing the stream exactly once.🤖 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/plugin/README.md` around lines 124 - 126, Update the streaming dispatch documentation to distinguish normal completion from early cancellation: request events until receiving Done or Failure, then release the provider stream exactly once; if the caller stops before a terminal event, cancel once and then release the stream exactly once.
♻️ Duplicate comments (1)
crates/core/tests/unit/native_plugin_tests.rs (1)
949-996: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPin the specific rejection reason in these validation tests.
native_api_v2_rejects_prohibited_target_methods_and_headers(Lines 949-996) andnative_api_v2_rejects_target_url_credentials(Lines 999-1015) each assert onlyNemoRelayStatus::InvalidArg.prepare_llm_continuation_invocationreturnsInvalidArgfor every validation failure: bad method, prohibited header, invalid header value, and non-absolute or credentialed URL. A regression that breaks one specific branch (for example, the header check silently passing through) still passes these tests because a different branch also producesInvalidArg.
native_api_v2_rejects_non_absolute_dispatch_targets_before_continuation(Line 939) already pins its reason withassert_last_error_contains("absolute HTTP(S) URL"). Apply the same pattern here.💚 Proposed test tightening
target.method = "CONNECT".into(); assert_eq!( prepare_llm_continuation_invocation(LlmContinuationInvocationV2 { request: request.clone(), target, }) .unwrap_err(), NemoRelayStatus::InvalidArg ); + assert_last_error_contains("method was invalid or prohibited"); let mut target = test_dispatch_target( "https://provider.example/v1/chat/completions", nemo_relay_plugin::LlmContinuationRouteV2::OpenaiChat, ); target .headers .insert("x-nemo-relay-internal-dispatch-url".into(), "secret".into()); assert_eq!( prepare_llm_continuation_invocation(LlmContinuationInvocationV2 { request: request.clone(), target, }) .unwrap_err(), NemoRelayStatus::InvalidArg ); + assert_last_error_contains("host-owned or prohibited"); let mut target = test_dispatch_target( "https://provider.example/v1/chat/completions", nemo_relay_plugin::LlmContinuationRouteV2::OpenaiChat, ); target .headers .insert("host".into(), "attacker.invalid".into()); assert_eq!( prepare_llm_continuation_invocation(LlmContinuationInvocationV2 { request, target }) .unwrap_err(), NemoRelayStatus::InvalidArg ); + assert_last_error_contains("host-owned or prohibited"); } #[test] fn native_api_v2_rejects_target_url_credentials() { let target = test_dispatch_target( "https://user:secret@provider.example/v1/chat/completions", nemo_relay_plugin::LlmContinuationRouteV2::OpenaiChat, ); assert_eq!( prepare_llm_continuation_invocation(LlmContinuationInvocationV2 { request: LlmRequest { headers: Map::new(), content: json!({}), }, target, }) .unwrap_err(), NemoRelayStatus::InvalidArg ); + assert_last_error_contains("absolute HTTP(S) URL without user info"); }Also applies to: 999-1015
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/core/tests/unit/native_plugin_tests.rs` around lines 949 - 996, Strengthen the validation tests by asserting each rejection’s specific error message, not only NemoRelayStatus::InvalidArg. In native_api_v2_rejects_prohibited_target_methods_and_headers, call the existing assert_last_error_contains pattern after each failed prepare_llm_continuation_invocation, using distinct expected text for the CONNECT method, prohibited internal-dispatch header, and host header; apply the same reason-specific assertion in native_api_v2_rejects_target_url_credentials, following native_api_v2_rejects_non_absolute_dispatch_targets_before_continuation.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/native.rs`:
- Around line 2330-2353: Update the target-header validation loop before
constructing LlmDispatchTargetContext to track normalized HeaderName values and
reject any case-insensitive duplicate with InvalidArg and an appropriate native
error. Keep the existing prohibited-name and invalid-value validation unchanged,
and ensure duplicates are detected before invocation.target.headers is consumed.
In `@crates/plugin/README.md`:
- Around line 113-115: Update the registration documentation in the plugin
README to present the typed v2 registration methods first, and move the *_raw
helper references into a clearly labeled advanced ABI section. Keep the existing
compat.native_api and PluginContext::host_api_v4 details accurate while
prioritizing the documented public API over raw registration helpers.
---
Outside diff comments:
In `@crates/plugin/README.md`:
- Around line 124-126: Update the streaming dispatch documentation to
distinguish normal completion from early cancellation: request events until
receiving Done or Failure, then release the provider stream exactly once; if the
caller stops before a terminal event, cancel once and then release the stream
exactly once.
---
Duplicate comments:
In `@crates/core/tests/unit/native_plugin_tests.rs`:
- Around line 949-996: Strengthen the validation tests by asserting each
rejection’s specific error message, not only NemoRelayStatus::InvalidArg. In
native_api_v2_rejects_prohibited_target_methods_and_headers, call the existing
assert_last_error_contains pattern after each failed
prepare_llm_continuation_invocation, using distinct expected text for the
CONNECT method, prohibited internal-dispatch header, and host header; apply the
same reason-specific assertion in native_api_v2_rejects_target_url_credentials,
following
native_api_v2_rejects_non_absolute_dispatch_targets_before_continuation.
🪄 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: 93f2d5b2-b3d9-4bd7-9deb-53a01cc78cb3
📒 Files selected for processing (5)
crates/core/src/plugin/dynamic/native.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/README.mdcrates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.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 (28)
**/*.{md,rst,html,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
**/*.{md,rst,html,txt}: Always spellNVIDIAin all caps. Do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun because the name starts with an 'en' sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company.
Use trademark symbols with product names only when the document type or legal guidance requires them.
Verify official capitalization, spacing, and hyphenation for product names.
Precede NVIDIA product names withNVIDIAon first mention when it is natural and accurate.
Do not rewrite product names for grammar or title-case rules.
Preserve third-party product names according to the owner's spelling.
Include the company name and full model qualifier on first use when it helps identify the model.
Preserve the official capitalization and punctuation of model names.
Use shorter family names only after the full name is established.
Spell out a term on first use and put the acronym in parentheses unless the acronym is widely understood by the intended audience.
Use the acronym on later mentions after it has been defined.
For long documents, reintroduce the full term if readers might lose context.
Form plurals of acronyms withs, not an apostrophe, such asGPUs.
In headings, common acronyms can remain abbreviated. Spell out the term in the first or second sentence of the body.
Common terms such asCPU,GPU,PC,API, andUIusually do not need to be spelled out for developer audiences.
Files:
crates/plugin/README.md
**/*.{md,rst,html}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
Link the first mention of a product name when the destination helps the reader.
Files:
crates/plugin/README.md
**/*.{md,rst,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
Spell
NVIDIAin all caps. Do not useNvidia,nvidia, orNV.
Files:
crates/plugin/README.md
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,rst}: Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text. Avoid raw URLs and weak anchors such as "here" or "read more."
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative steps. Keep steps parallel and split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce.
Preferrefer tooverseewhen the wording points readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical docs.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values.
Use numerals for 10 or greater and include commas in thousands.
Do not add trademark symbols to learning-oriented docs unless the source, platform, or legal guidance explicitly requires them.
Files:
crates/plugin/README.md
**/*.md
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.md: Use title case consistently in technical documentation headings
Avoid quotation marks, ampersands, and exclamation marks in headings
Keep product, event, research, and whitepaper names in their official title case
Use title case for table headers
Do not force social-media sentence case into technical docs
Format code elements, commands, parameters, package names, and expressions in monospace
Format directories, file names, and paths in monospace using backticks
Use angle brackets inside monospace for variables inside paths, such as/home/<username>/.login
Format error messages and strings in quotation marks, keeping literal code strings in code formatting when clearer
Format UI buttons, menus, fields, and labels in bold
Use angle brackets between UI labels for menu paths, such as File > Save As
Use italics for new terms on first use, sparingly and only when introducing the term
Use italics for publication titles
Format keyboard shortcuts in plain text, such as Press Ctrl+Alt+Delete
Use owner/repo link text for GitHub repositories, preferring[NVIDIA/NeMo](link)over prose references like 'the GitHub repo'
Introduce every code block with a complete sentence
Do not make a code block complete the grammar of the previous sentence
Do not continue a sentence after a code block
Use syntax highlighting when the format supports it for code blocks
Avoid the word 'snippet' unless the surrounding docs already use it as a term of art
Keep inline method, function, and class references consistent with nearby docs, omitting empty parentheses for prose readability when no call is shown
Use descriptive anchor text that matches the destination title when possible for links
Avoid raw URLs in running text
Avoid generic anchor text such as 'here,' 'this page,' and 'read more'
Include acronyms in link text when a linked term includes an acronym
Do not link long sentences or multiple sentences
Avoid links that pull readers away from a procedure unless the link is a p...
Files:
crates/plugin/README.md
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Update
README.md,fern/, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.
**/*.{md,mdx}: Prefer the documented public API, not internal shortcuts
Keep package names, repo references, and build commands current
Keep release-process and release-notes guidance in repo-maintainer docs such asRELEASING.md, not as user-facing docs pages orCHANGELOG.md
Keep stable user-facing wrappers atscripts/root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pagesIf links in documentation change, run
just docs-linkcheck.
Files:
crates/plugin/README.md
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
crates/plugin/README.md
{crates/core/src/plugin/dynamic/**,crates/plugin/**,crates/worker/**,crates/worker-proto/**,crates/types/**,python/plugin/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Keep the stable boundary explicit: native plugins cross a C ABI, and worker plugins cross
grpc-v1.
Files:
crates/plugin/README.mdcrates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/src/plugin/dynamic/native.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/plugin/README.mdcrates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.{md,mdx,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Examples and documentation must use each exporter's documented flush/deregister order before shutdown.
Files:
crates/plugin/README.md
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/plugin/src/lib.rscrates/core/src/plugin/dynamic/native.rs
{crates/core/src/plugin/dynamic/**/*.rs,crates/plugin/**/*.rs,crates/worker/**/*.rs,crates/worker-proto/**/*.rs,python/plugin/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Manifest validation must cover kind, compatibility, load contract, integrity, capability mismatch, and disabled-plugin behavior.
Files:
crates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/src/plugin/dynamic/native.rs
{crates/plugin/**/*.rs,python/plugin/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Rust and Python SDKs must expose every supported registration surface.
Files:
crates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
crates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/plugin/src/lib.rscrates/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.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/plugin/tests/typed_callbacks.rscrates/core/tests/unit/native_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/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/tests/unit/native_plugin_tests.rscrates/core/src/plugin/dynamic/native.rs
{crates/core/src/plugin/dynamic/**/*.rs,examples/rust-native-plugin/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not pass Rust runtime types, trait objects, futures, or allocator-owned strings across the native dynamic-library boundary.
Files:
crates/core/src/plugin/dynamic/native.rs
{crates/core/src/plugin/dynamic/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Native and worker plugins are trusted extensions; document that native plugins are in-process and unsandboxed, and worker plugins provide process isolation but not a security sandbox.
Files:
crates/core/src/plugin/dynamic/native.rs
crates/core/src/plugin/dynamic/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
The native loader must keep libraries alive until registered callbacks are cleared and must deregister plugin kinds before unload.
Files:
crates/core/src/plugin/dynamic/native.rs
🧠 Learnings (1)
📚 Learning: 2026-07-28T20:07:29.880Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 571
File: crates/core/src/api/runtime/state.rs:996-1020
Timestamp: 2026-07-28T20:07:29.880Z
Learning: In NeMo Relay (RELAY-509), sanitizer callback failures must be treated as intentional fail-open behavior. When an event/tool (request/response) or LLM (request/response) sanitizer callback fails, the sanitizer chain should retain and publish the last valid event/payload snapshot (rather than dropping/invalidating the data) and log the failure including callback context (e.g., which sanitizer/callback failed and relevant identifiers). Apply this consistently across all sanitizer chains mentioned in the RELAY-509 documentation/migration guide.
Applied to files:
crates/core/src/plugin/dynamic/native.rs
🔇 Additional comments (8)
crates/plugin/src/lib.rs (2)
977-990: Unknown non-HTTP failure kinds still fail deserialization.
LlmNonHttpFailureKindV2has no catch-all variant. A future host that adds a kind breaks older plugins that deserializeLlmContinuationFailureV2. This was raised on the previous commit; only the doc comment on Line 984 changed.
46-47: LGTM!Also applies to: 915-961, 1004-1031, 1042-1071, 1262-1329, 2061-2067, 2897-2943, 3619-3640, 3931-3947, 3985-4005, 4082-4082
crates/plugin/tests/typed_callbacks.rs (1)
17-21: LGTM!Also applies to: 39-98, 400-484
crates/core/src/plugin/dynamic/native.rs (2)
54-61: LGTM!Also applies to: 918-918, 1538-1538, 1577-1591, 2374-2413, 2456-2470, 2473-2531, 2688-2866, 2869-2917
916-932: 📐 Maintainability & Code QualityRun the full validation matrix before handoff.
This change touches
crates/core, so the repository rules requirecargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,just test-rust, and the Python, Go, and Node.js test targets. The PR description lists workspace Clippy, the documentation build, pre-commit, and full cross-language validation as still pending.As per coding guidelines: "If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js."Source: Coding guidelines
crates/plugin/README.md (2)
34-34: LGTM!Also applies to: 52-54, 100-112, 117-123, 127-133
113-115: 🗄️ Data Integrity & IntegrationKeep the Native API v2 documentation as written.
compat.native_api = "2"selects the ABI v4 host table, and the loader test covers this contract.host_api_v4()is optional only for hosts that do not provide ABI v4.> Likely an incorrect or invalid review comment.crates/core/tests/unit/native_plugin_tests.rs (1)
8-8: LGTM!Also applies to: 29-30, 112-183, 553-559, 816-893, 896-946, 1018-1047, 1110-1248, 1250-1324, 1326-1403, 1405-1486, 1488-1617
bbednarski9
changed the title
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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/api/llm.rs`:
- Around line 1485-1489: The core-owned dispatch path around
targeted_llm_execution and targeted_llm_stream_execution must preserve upstream
status, headers, and response body metadata instead of leaving gateway
upstream_info and response_bytes empty. Propagate the execution response
metadata through both buffered and streaming paths into the gateway response,
then add tests covering status, headers, and body metadata for each path.
In `@crates/core/src/api/runtime/llm_dispatch_context.rs`:
- Around line 212-222: Bound the successful response body in dispatch_buffered
instead of calling response.bytes() without a limit. Reuse bounded_response_body
with a dedicated success-size limit, or validate Content-Length before reading,
while preserving JSON parsing and existing transport-error handling.
- Around line 305-321: Update transport_error and its call sites to log
structured diagnostics including the target host and reqwest error while
preserving the existing redacted plugin-visible bodies. Extend timeout detection
to recognize streamed read timeouts wrapped in decode errors by checking the
decode error source chain, and classify those failures as
UpstreamFailureClass::Timeout rather than Connection.
- Around line 278-289: Remove the global Client timeout from
targeted_http_client(), leaving connect_timeout and read_timeout intact for
shared transport behavior. Apply HTTP_REQUEST_TIMEOUT via
RequestBuilder::timeout only in dispatch_buffered(), while keeping
dispatch_stream() governed by HTTP_READ_TIMEOUT for idle-read protection.
In `@crates/core/tests/fixtures/native_plugin/src/lib.rs`:
- Around line 332-347: Remove the manual Box::from_raw cleanup in the
failed-registration branch after register_async_llm_execution_v2_raw; rely on
the registration failure path to invoke drop_targeted_fixture_state for state,
while preserving the existing error return.
In `@crates/core/tests/integration/native_plugin_tests.rs`:
- Around line 2160-2221: The EmbeddedFakeProvider::spawn worker can block
indefinitely in listener.accept(), causing Drop’s thread join to hang when
dispatch never reaches the provider. Configure an accept timeout or otherwise
bound the initial connection wait before the existing read-timeout logic, and
propagate the resulting failure so the test reports normally instead of hanging.
- Around line 1332-1358: Add the existing NativePluginTestCleanup guard at the
start of this test, before the assertions and provider interaction, so its Drop
implementation clears the plugin configuration and activation even when an
assertion panics. Remove the reliance on the trailing clear_plugin_configuration
and activation.clear calls, while preserving the current success-path assertions
and cleanup behavior.
In `@crates/core/tests/unit/llm_dispatch_context_tests.rs`:
- Around line 16-60: The duplicated fake HTTP providers can hang indefinitely in
accept and duplicate request parsing. In
crates/core/tests/unit/llm_dispatch_context_tests.rs#L16-L60, extract
FakeProvider, read_http_request, and response handling into shared test support
and bound the listener’s wait so the worker exits when no dispatch connects; in
crates/core/tests/integration/native_plugin_tests.rs#L2160-L2221, remove
EmbeddedFakeProvider and use that shared helper, with no separate accept or
Content-Length parsing logic.
- Around line 16-60: Update FakeProvider and the corresponding
EmbeddedFakeProvider test helper to use a shared implementation instead of
duplicating the fake-server setup. Ensure the listener’s accept path has a
bounded timeout or otherwise cannot block indefinitely, and preserve request
capture, response writing, and thread cleanup behavior for both unit and
integration tests.
- Around line 173-202: Update the provider thread’s response-writing logic in
the helper used by buffered_http_failure_is_bounded_and_filters_headers to
tolerate a client disconnect during write_all, treating a short
write/broken-pipe result as expected instead of panicking through expect.
Preserve panic behavior for unrelated write failures and keep the test’s
existing assertions unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 6b988f20-41c0-4b49-850d-e384d67b27dd
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
crates/cli/src/gateway/mod.rscrates/core/Cargo.tomlcrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/error.rscrates/core/src/plugin/dynamic/native.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/plugin/README.mddocs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (37)
{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/Cargo.tomlcrates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/Cargo.toml
📄 CodeRabbit inference engine (.agents/skills/prepare-code-freeze/SKILL.md)
Confirm or infer the target release version from
upstream/main:Cargo.toml. Derive the release branch asrelease/<major>.<minor>.Keep Rust package names and workspace metadata in
Cargo.tomlinternally consistent across the project.OpenTelemetry and OpenInference dependencies must be unconditional rather than Cargo feature-gated.
Files:
crates/core/Cargo.toml
**/*.toml
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all TOML files using the
#comment form.
Files:
crates/core/Cargo.toml
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/core/Cargo.tomlcrates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/cli/src/gateway/mod.rsdocs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/core/tests/integration/native_plugin_tests.rscrates/plugin/README.mdcrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/Cargo.tomlcrates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/cli/src/gateway/mod.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/cli/src/gateway/mod.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/cli/src/gateway/mod.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/cli/src/gateway/mod.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/cli/src/gateway/mod.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/cli/src/gateway/mod.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
crates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/cli/src/gateway/mod.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/cli/src/gateway/mod.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.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/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
crates/core/src/{api/**/*.rs,api/runtime/**/*.rs,codec/**/*.rs,json.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Implement the new or changed public runtime behavior first in the Rust core, especially under
crates/core/src/api/and related core modules such ascrates/core/src/api/runtime/,crates/core/src/codec/, andcrates/core/src/json.rs.
Files:
crates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/core/src/api/runtime/llm_dispatch_context.rs
crates/core/src/api/{tool,llm,shared,scope}.rs
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Wire the new middleware chain into the appropriate lifecycle owner and pipeline stage: tool and LLM execution paths use
tool.rsorllm.rs; shared mark and scope event sanitization usesshared.rsand is called fromscope.rs.
Files:
crates/core/src/api/llm.rs
crates/core/src/api/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Preserve the documented pipeline order: conditional guardrails, request intercepts, request sanitization, execution intercepts, and response sanitization for tool and LLM execution; specialized sanitization, event creation, and dispatch for mark and scope events.
Files:
crates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/core/src/api/runtime/llm_dispatch_context.rs
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
In MDX files, top-of-file comments must use JSX comment delimiters (
{/*to open and*/}to close); do not use HTML comments for MDX SPDX headers
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Update
README.md,fern/, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.
**/*.{md,mdx}: Prefer the documented public API, not internal shortcuts
Keep package names, repo references, and build commands current
Keep release-process and release-notes guidance in repo-maintainer docs such asRELEASING.md, not as user-facing docs pages orCHANGELOG.md
Keep stable user-facing wrappers atscripts/root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pagesIf links in documentation change, run
just docs-linkcheck.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/plugin/README.md
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/plugin/README.md
{crates/core/src/plugin/dynamic/**,crates/plugin/**,crates/worker/**,crates/worker-proto/**,crates/types/**,python/plugin/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Keep the stable boundary explicit: native plugins cross a C ABI, and worker plugins cross
grpc-v1.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/plugin/README.mdcrates/core/src/plugin/dynamic/native.rs
{crates/core/src/plugin/dynamic/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Native and worker plugins are trusted extensions; document that native plugins are in-process and unsandboxed, and worker plugins provide process isolation but not a security sandbox.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/core/src/plugin/dynamic/native.rs
{docs/build-plugins/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
When detailed dynamic plugin guides exist, keep Rust native, Python worker, and
grpc-v1protocol details on separate pages.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
docs/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If documentation examples or commands under
docs/change, run the targeted docs checks appropriate to the change.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
**/*.{md,mdx,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Examples and documentation must use each exporter's documented flush/deregister order before shutdown.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdxcrates/plugin/README.md
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.
Flag stale examples, missing SPDX headers where required, and instructions that no longer match CI or pre-commit behavior.
Files:
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/fixtures/native_plugin/src/lib.rs
**/*.{md,rst,html,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
**/*.{md,rst,html,txt}: Always spellNVIDIAin all caps. Do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun because the name starts with an 'en' sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company.
Use trademark symbols with product names only when the document type or legal guidance requires them.
Verify official capitalization, spacing, and hyphenation for product names.
Precede NVIDIA product names withNVIDIAon first mention when it is natural and accurate.
Do not rewrite product names for grammar or title-case rules.
Preserve third-party product names according to the owner's spelling.
Include the company name and full model qualifier on first use when it helps identify the model.
Preserve the official capitalization and punctuation of model names.
Use shorter family names only after the full name is established.
Spell out a term on first use and put the acronym in parentheses unless the acronym is widely understood by the intended audience.
Use the acronym on later mentions after it has been defined.
For long documents, reintroduce the full term if readers might lose context.
Form plurals of acronyms withs, not an apostrophe, such asGPUs.
In headings, common acronyms can remain abbreviated. Spell out the term in the first or second sentence of the body.
Common terms such asCPU,GPU,PC,API, andUIusually do not need to be spelled out for developer audiences.
Files:
crates/plugin/README.md
**/*.{md,rst,html}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
Link the first mention of a product name when the destination helps the reader.
Files:
crates/plugin/README.md
**/*.{md,rst,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
Spell
NVIDIAin all caps. Do not useNvidia,nvidia, orNV.
Files:
crates/plugin/README.md
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,rst}: Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text. Avoid raw URLs and weak anchors such as "here" or "read more."
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative steps. Keep steps parallel and split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce.
Preferrefer tooverseewhen the wording points readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical docs.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values.
Use numerals for 10 or greater and include commas in thousands.
Do not add trademark symbols to learning-oriented docs unless the source, platform, or legal guidance explicitly requires them.
Files:
crates/plugin/README.md
**/*.md
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.md: Use title case consistently in technical documentation headings
Avoid quotation marks, ampersands, and exclamation marks in headings
Keep product, event, research, and whitepaper names in their official title case
Use title case for table headers
Do not force social-media sentence case into technical docs
Format code elements, commands, parameters, package names, and expressions in monospace
Format directories, file names, and paths in monospace using backticks
Use angle brackets inside monospace for variables inside paths, such as/home/<username>/.login
Format error messages and strings in quotation marks, keeping literal code strings in code formatting when clearer
Format UI buttons, menus, fields, and labels in bold
Use angle brackets between UI labels for menu paths, such as File > Save As
Use italics for new terms on first use, sparingly and only when introducing the term
Use italics for publication titles
Format keyboard shortcuts in plain text, such as Press Ctrl+Alt+Delete
Use owner/repo link text for GitHub repositories, preferring[NVIDIA/NeMo](link)over prose references like 'the GitHub repo'
Introduce every code block with a complete sentence
Do not make a code block complete the grammar of the previous sentence
Do not continue a sentence after a code block
Use syntax highlighting when the format supports it for code blocks
Avoid the word 'snippet' unless the surrounding docs already use it as a term of art
Keep inline method, function, and class references consistent with nearby docs, omitting empty parentheses for prose readability when no call is shown
Use descriptive anchor text that matches the destination title when possible for links
Avoid raw URLs in running text
Avoid generic anchor text such as 'here,' 'this page,' and 'read more'
Include acronyms in link text when a linked term includes an acronym
Do not link long sentences or multiple sentences
Avoid links that pull readers away from a procedure unless the link is a p...
Files:
crates/plugin/README.md
{crates/core/src/plugin/dynamic/**/*.rs,examples/rust-native-plugin/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not pass Rust runtime types, trait objects, futures, or allocator-owned strings across the native dynamic-library boundary.
Files:
crates/core/src/plugin/dynamic/native.rs
{crates/core/src/plugin/dynamic/**/*.rs,crates/plugin/**/*.rs,crates/worker/**/*.rs,crates/worker-proto/**/*.rs,python/plugin/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Manifest validation must cover kind, compatibility, load contract, integrity, capability mismatch, and disabled-plugin behavior.
Files:
crates/core/src/plugin/dynamic/native.rs
crates/core/src/plugin/dynamic/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
The native loader must keep libraries alive until registered callbacks are cleared and must deregister plugin kinds before unload.
Files:
crates/core/src/plugin/dynamic/native.rs
🧠 Learnings (2)
📚 Learning: 2026-07-28T20:07:29.880Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 571
File: crates/core/src/api/runtime/state.rs:996-1020
Timestamp: 2026-07-28T20:07:29.880Z
Learning: In NeMo Relay (RELAY-509), sanitizer callback failures must be treated as intentional fail-open behavior. When an event/tool (request/response) or LLM (request/response) sanitizer callback fails, the sanitizer chain should retain and publish the last valid event/payload snapshot (rather than dropping/invalidating the data) and log the failure including callback context (e.g., which sanitizer/callback failed and relevant identifiers). Apply this consistently across all sanitizer chains mentioned in the RELAY-509 documentation/migration guide.
Applied to files:
crates/core/src/error.rscrates/core/src/api/llm.rscrates/core/src/api/runtime.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
📚 Learning: 2026-07-28T03:31:05.964Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 564
File: crates/core/src/api/runtime/subscriber_dispatcher.rs:297-314
Timestamp: 2026-07-28T03:31:05.964Z
Learning: In this codebase’s runtime API, do not implement incremental native LLM stream forwarding via the native ABI v3 asynchronous middleware protocol (it can only settle a single JSON value via a one-shot completion handle and cannot forward stream chunks incrementally). If a latency-sensitive plugin needs streaming behavior, review for use of synchronous native stream intercepts or worker plugins instead of trying to chunk-deliver or incrementally forward over the ABI v3 async path.
Applied to files:
crates/core/src/api/runtime/llm_dispatch_context.rs
🔇 Additional comments (29)
docs/build-plugins/dynamic-plugins/native-dynamic/about.mdx (6)
186-188: Keep typed registration methods first.This section still points authors to raw v2 registration helpers. Present the typed
PluginContextregistration methods first. Move*_rawhelpers to an advanced ABI section.Source: Path instructions
3-3: LGTM!Also applies to: 16-16
119-131: LGTM!
133-170: LGTM!
172-185: LGTM!
190-196: LGTM!Also applies to: 206-206
crates/plugin/README.md (4)
113-115: Keep typed registration methods first.This section still points authors to raw v2 registration helpers. Present the typed
PluginContextregistration methods first. Move*_rawhelpers to an advanced ABI section.Source: Path instructions
34-34: LGTM!Also applies to: 52-54
100-112: LGTM!
117-139: LGTM!crates/core/src/api/runtime/llm_dispatch_context.rs (4)
70-88: 🗄️ Data Integrity & Integration | ⚡ Quick winCase-insensitive duplicate target headers are silently collapsed.
headersis aBTreeMap<String, String>, so"Authorization"and"authorization"are distinct keys.HeaderName::from_bytesnormalizes both toauthorization, andHeaderMap::insertreplaces the earlier value. The plugin gets no error, and which credential reaches the provider depends onBTreeMapordering. Reject case-insensitive duplicates instead of relying on replacement.🛡️ Proposed fix
let mut validated_headers = HeaderMap::new(); for (name, value) in headers { let name = HeaderName::from_bytes(name.as_bytes()).map_err(|_| { FlowError::InvalidArgument( "LLM continuation contained an invalid target header name".into(), ) })?; if prohibited_target_header(&name) { return Err(FlowError::InvalidArgument(format!( "LLM continuation target header {name} is host-owned or prohibited" ))); } + if validated_headers.contains_key(&name) { + return Err(FlowError::InvalidArgument(format!( + "LLM continuation target header {name} was specified more than once" + ))); + } let value = HeaderValue::from_str(&value).map_err(|_| {
4-31: LGTM!
127-147: LGTM!
337-381: LGTM!crates/core/src/api/runtime.rs (1)
27-31: LGTM!crates/core/Cargo.toml (1)
66-66: LGTM!crates/core/src/api/llm.rs (1)
30-31: LGTM!crates/core/src/plugin/dynamic/native.rs (1)
2305-2314: LGTM!crates/core/src/error.rs (1)
15-15: LGTM!Also applies to: 37-37, 51-51, 125-125
crates/cli/src/gateway/mod.rs (3)
323-333: LGTM!Also applies to: 527-537
799-810: LGTM!Also applies to: 912-912, 922-929
862-877: 🗄️ Data Integrity & IntegrationRemove this review comment.
effective_dispatch_requestis test-only, and production forwarding passes the real method tobuild_effective_dispatch_request. The wrapper cannot affect observability or ATIF capture.> Likely an incorrect or invalid review comment.crates/core/tests/fixtures/native_plugin/src/lib.rs (2)
10-15: LGTM!
351-371: LGTM!Also applies to: 373-463, 465-468
crates/core/tests/integration/native_plugin_tests.rs (1)
8-8: LGTM!crates/core/tests/unit/llm_dispatch_context_tests.rs (3)
1-14: LGTM!Also applies to: 93-123
125-171: LGTM!
204-221: LGTM!Also applies to: 223-283, 285-319, 321-347
crates/core/tests/unit/native_plugin_tests.rs (1)
828-839: LGTM!Also applies to: 1426-1430
| async fn dispatch_buffered(target: &LlmDispatchTargetContext, request: LlmRequest) -> Result<Json> { | ||
| let response = send(target, request).await?; | ||
| let status = response.status(); | ||
| let headers = safe_failure_headers(response.headers()); | ||
| if !status.is_success() { | ||
| let bytes = bounded_response_body(response).await?; | ||
| return Err(http_error(status, headers, &bytes)); | ||
| } | ||
| let bytes = response.bytes().await.map_err(transport_error)?; | ||
| serde_json::from_slice(&bytes).map_err(|_| http_error(status, headers, &bytes)) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound the buffered success body.
Line 217 caps failure bodies at MAX_UPSTREAM_ERROR_BODY_BYTES, but Line 220 calls response.bytes() with no limit on the success path. reqwest applies no default response size limit, so a target that streams an unbounded body allocates it entirely in the Relay process. The plugin selects this target, and Relay dispatches to it on a request thread, so a single continuation can exhaust memory.
Reuse a bounded read for the success path with a dedicated success limit, or reject responses whose Content-Length exceeds that limit before reading.
🤖 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/api/runtime/llm_dispatch_context.rs` around lines 212 - 222,
Bound the successful response body in dispatch_buffered instead of calling
response.bytes() without a limit. Reuse bounded_response_body with a dedicated
success-size limit, or validate Content-Length before reading, while preserving
JSON parsing and existing transport-error handling.
| fn transport_error(error: reqwest::Error) -> FlowError { | ||
| let timeout = error.is_timeout(); | ||
| FlowError::Upstream(UpstreamFailure { | ||
| status: None, | ||
| body: if timeout { | ||
| "provider request timed out".into() | ||
| } else { | ||
| "provider transport failed".into() | ||
| }, | ||
| headers: BTreeMap::new(), | ||
| class: if timeout { | ||
| UpstreamFailureClass::Timeout | ||
| } else { | ||
| UpstreamFailureClass::Connection | ||
| }, | ||
| }) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Transport failures carry no diagnostics and misclassify streamed read timeouts.
Two points:
- The failure body is a fixed string, and the function emits no log. An operator cannot tell which target host failed or why. Add a structured log with the target host and the
reqwesterror, and keep the plugin-visible body redacted as it is now. error.is_timeout()does not hold for read timeouts that expire while a body is streaming.reqwestwraps those as body decode errors, sodispatch_streamreportsUpstreamFailureClass::Connectionfor a stalled provider.UpstreamFailure::is_retryablereturnstruefor both classes, so retry behavior is unaffected, but the class is wrong for observability and policy. Checkerror.is_decode()and the error source chain, or confirm the classification you intend for streamed timeouts.
🤖 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/api/runtime/llm_dispatch_context.rs` around lines 305 - 321,
Update transport_error and its call sites to log structured diagnostics
including the target host and reqwest error while preserving the existing
redacted plugin-visible bodies. Extend timeout detection to recognize streamed
read timeouts wrapped in decode errors by checking the decode error source
chain, and classify those failures as UpstreamFailureClass::Timeout rather than
Connection.
License DiffCompared against Lockfile license changesLockfile License ChangesRustAdded
Removed
Updated/Changed
NodeAdded
Removed
Updated/Changed
PythonAdded
Removed
Updated/Changed
Status output |
coderabbitai
Bot
added
the
DO NOT MERGE
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/core/src/api/runtime/llm_dispatch_context.rs (1)
326-350: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRead-timeout misclassification during streaming remains open.
The new
log::warn!call fixes the missing-diagnostics half of the earlier concern by recordingprovider_hostandfailure_kind. The classification half is still open:error.is_timeout()does not detect a read timeout that fires whiledispatch_stream's body is being consumed —reqwestwraps that as a body/decode error, not a timeout error. Line 262 routes any stream-body error through this same function, so a stalled provider during streaming getsUpstreamFailureClass::Connectioninstead ofUpstreamFailureClass::Timeout.UpstreamFailure::is_retryabletreats both the same, so retries are unaffected, but the class is wrong for observability and policy.Check the decode-error source chain (or
error.is_decode()plus the wrapped source) to classify streamed read timeouts asTimeout.🤖 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/api/runtime/llm_dispatch_context.rs` around lines 326 - 350, Update transport_error to detect read timeouts wrapped inside streaming body/decode errors, using the reqwest decode status and its source error chain in addition to error.is_timeout(). Set timeout, failure_kind, and the resulting UpstreamFailureClass/response body consistently to Timeout for these cases, while preserving transport classification for non-timeout errors.
♻️ Duplicate comments (1)
crates/core/src/api/runtime/llm_dispatch_context.rs (1)
217-230: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound the buffered success-body read.
Line 222 bounds the failure body through
bounded_response_body, but line 225-229 still callsresponse.bytes()with no size limit on the success path.reqwesthas no default response-size cap, so a target that streams an unbounded body forces Relay to buffer the whole thing in memory. The plugin selects this target, so a single continuation can exhaust process memory.Reuse a bounded read (e.g.,
bounded_response_bodywith a dedicated success-size limit) or reject responses whoseContent-Lengthexceeds that limit before reading.🛡️ Proposed fix
- let bytes = response - .bytes() - .await - .map_err(|error| transport_error(target, error))?; + let bytes = bounded_response_body(target, response).await?; serde_json::from_slice(&bytes).map_err(|_| http_error(status, headers, &bytes))🤖 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/api/runtime/llm_dispatch_context.rs` around lines 217 - 230, Update dispatch_buffered so successful responses also use a dedicated maximum body-size limit instead of response.bytes(). Reuse bounded_response_body or an equivalent bounded read, preserving the existing transport-error mapping and JSON parsing while rejecting or reporting bodies that exceed the limit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/core/src/api/runtime/llm_dispatch_context.rs`:
- Around line 326-350: Update transport_error to detect read timeouts wrapped
inside streaming body/decode errors, using the reqwest decode status and its
source error chain in addition to error.is_timeout(). Set timeout, failure_kind,
and the resulting UpstreamFailureClass/response body consistently to Timeout for
these cases, while preserving transport classification for non-timeout errors.
---
Duplicate comments:
In `@crates/core/src/api/runtime/llm_dispatch_context.rs`:
- Around line 217-230: Update dispatch_buffered so successful responses also use
a dedicated maximum body-size limit instead of response.bytes(). Reuse
bounded_response_body or an equivalent bounded read, preserving the existing
transport-error mapping and JSON parsing while rejecting or reporting bodies
that exceed the limit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: af91c6ba-c4bf-4302-9f79-e7aaaa4219e0
📒 Files selected for processing (7)
crates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rscrates/core/tests/fixtures/native_plugin/src/lib.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/unit/native_plugin_tests.rs
💤 Files with no reviewable changes (1)
- crates/core/tests/fixtures/native_plugin/src/lib.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (21)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.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/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
crates/core/src/{api/**/*.rs,api/runtime/**/*.rs,codec/**/*.rs,json.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Implement the new or changed public runtime behavior first in the Rust core, especially under
crates/core/src/api/and related core modules such ascrates/core/src/api/runtime/,crates/core/src/codec/, andcrates/core/src/json.rs.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
crates/{core,adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full validation matrix across Rust, Python, Go, and Node.js.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
crates/core/src/api/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Preserve the documented pipeline order: conditional guardrails, request intercepts, request sanitization, execution intercepts, and response sanitization for tool and LLM execution; specialized sanitization, event creation, and dispatch for mark and scope events.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/core/src/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.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/api/runtime/continuation_context.rscrates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/core/tests/unit/llm_dispatch_context_tests.rscrates/core/tests/integration/native_plugin_tests.rscrates/core/tests/unit/native_plugin_tests.rs
{crates/core/src/plugin/dynamic/**,crates/plugin/**,crates/worker/**,crates/worker-proto/**,crates/types/**,python/plugin/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Keep the stable boundary explicit: native plugins cross a C ABI, and worker plugins cross
grpc-v1.
Files:
crates/core/src/plugin/dynamic/native.rs
{crates/core/src/plugin/dynamic/**/*.rs,examples/rust-native-plugin/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not pass Rust runtime types, trait objects, futures, or allocator-owned strings across the native dynamic-library boundary.
Files:
crates/core/src/plugin/dynamic/native.rs
{crates/core/src/plugin/dynamic/**,examples/rust-native-plugin/**,examples/python-grpc-worker-plugin/**,docs/build-plugins/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Native and worker plugins are trusted extensions; document that native plugins are in-process and unsandboxed, and worker plugins provide process isolation but not a security sandbox.
Files:
crates/core/src/plugin/dynamic/native.rs
{crates/core/src/plugin/dynamic/**/*.rs,crates/plugin/**/*.rs,crates/worker/**/*.rs,crates/worker-proto/**/*.rs,python/plugin/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Manifest validation must cover kind, compatibility, load contract, integrity, capability mismatch, and disabled-plugin behavior.
Files:
crates/core/src/plugin/dynamic/native.rs
crates/core/src/plugin/dynamic/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
The native loader must keep libraries alive until registered callbacks are cleared and must deregister plugin kinds before unload.
Files:
crates/core/src/plugin/dynamic/native.rs
🧠 Learnings (2)
📚 Learning: 2026-07-28T03:31:05.964Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 564
File: crates/core/src/api/runtime/subscriber_dispatcher.rs:297-314
Timestamp: 2026-07-28T03:31:05.964Z
Learning: In this codebase’s runtime API, do not implement incremental native LLM stream forwarding via the native ABI v3 asynchronous middleware protocol (it can only settle a single JSON value via a one-shot completion handle and cannot forward stream chunks incrementally). If a latency-sensitive plugin needs streaming behavior, review for use of synchronous native stream intercepts or worker plugins instead of trying to chunk-deliver or incrementally forward over the ABI v3 async path.
Applied to files:
crates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rs
📚 Learning: 2026-07-28T20:07:29.880Z
Learnt from: willkill07
Repo: NVIDIA/NeMo-Relay PR: 571
File: crates/core/src/api/runtime/state.rs:996-1020
Timestamp: 2026-07-28T20:07:29.880Z
Learning: In NeMo Relay (RELAY-509), sanitizer callback failures must be treated as intentional fail-open behavior. When an event/tool (request/response) or LLM (request/response) sanitizer callback fails, the sanitizer chain should retain and publish the last valid event/payload snapshot (rather than dropping/invalidating the data) and log the failure including callback context (e.g., which sanitizer/callback failed and relevant identifiers). Apply this consistently across all sanitizer chains mentioned in the RELAY-509 documentation/migration guide.
Applied to files:
crates/core/src/api/runtime/continuation_context.rscrates/core/src/api/runtime/llm_dispatch_context.rscrates/core/src/plugin/dynamic/native.rs
🔇 Additional comments (8)
crates/core/tests/integration/native_plugin_tests.rs (2)
2169-2190: LGTM!
1331-1332: 📐 Maintainability & Code QualityComplete the required
crates/corevalidation matrix before merge.The PR states that the cross-language matrix remains pending. Run
just test-rust,just test-python,just test-go,just test-node,cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings,cargo deny check,validate-change, anduv run pre-commit run --all-files.
crates/core/tests/integration/native_plugin_tests.rs#L1331-L1332: include this integration-test change in the validation results.crates/core/tests/unit/llm_dispatch_context_tests.rs#L25-L64: include this provider-lifecycle change in the validation results.crates/core/tests/unit/native_plugin_tests.rs#L973-L1024: include this native API v2 validation coverage in the validation results.As per coding guidelines: “Changes to
crates/coreorcrates/adaptivemust run the full language matrix.”Source: Coding guidelines
crates/core/tests/unit/native_plugin_tests.rs (1)
973-973: LGTM!Also applies to: 990-990, 1004-1004, 1024-1024
crates/core/src/api/runtime/llm_dispatch_context.rs (2)
82-86: LGTM!The case-insensitive duplicate-header check is correct here:
HeaderNamelookup inHeaderMapis case-insensitive, sovalidated_headers.contains_key(&name)catches variants likeAuthorization/authorization. This mirrors the same fix already applied incrates/core/src/plugin/dynamic/native.rs.
329-335: 🩺 Stability & AvailabilityNo change needed:
logenables thekvfeature incrates/core/Cargo.toml.> Likely an incorrect or invalid review comment.crates/core/src/api/runtime/continuation_context.rs (1)
11-13: LGTM!The prior review comment requesting delegation to
invokeinstead of duplicating its body is now applied at line 120. Scoping order is preserved: the dispatch target remains the outer task-local, andinvoke/runrestores the Relay context inside it.Also applies to: 109-121
crates/core/src/plugin/dynamic/native.rs (2)
1776-1793: LGTM!The
spawn_blockingjoin-error path now reclaimscompletion_ref,next_ref, and freesinvocationbefore returning, matching the previously requested fix.
1797-1822: 🩺 Stability & AvailabilityKeep
next_refownership with the callback.The callback owns
nextafter invocation and must callasync_next_releaseexactly once. The host must not reclaim it in the panic or invalid-state branches, because the callback may already have released it. The join-error branch is different because the callback did not execute.> Likely an incorrect or invalid review comment.
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
a3e78ac to
a11ff5e
Compare
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
github-actions
Bot
added
size:XXL
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
bbednarski9
changed the title
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
1 participant
Overview
Important
Native dynamic-plugin compatibility notice
Released native API v1 is unchanged: its symbols, layouts, export macro, manifest negotiation, and loader behavior remain binary-compatible.
Native API v2 and
NemoRelayNativeHostApiV4are unreleased draft contracts. This PR appends a direct stream-forwarding hook to the end of V4 and makes a safe Rust v2 facade the default authoring path. Plugins built against an earlier draft of v2 must rebuild against this revision; no released ABI is broken.External LLM routing policies need to invoke a captured Relay continuation against a policy-selected provider target, inspect its buffered or streaming result, and optionally leave unmanaged calls on Relay's ordinary path. The raw C ABI can represent that lifecycle, but requiring Rust plugin authors to allocate host strings, poll callback handles, settle completions, pump streams, and release opaque references makes every routing plugin repeat unsafe integration code.
This PR closes both parts of that gap:
nemo-relay-pluginwraps the raw v2 C tables in safe Rust continuations, provider streams, async registration methods, and a host-owned streaming pass-through outcome.The result is generic to native LLM routing policies. Switchyard is the first consumer, not a special case in Relay core.
Details
Safe Rust native API v2 facade
Rust plugins now register complete-only async LLM callbacks with:
PluginContext::register_async_llm_execution_v2PluginContext::register_async_llm_stream_execution_v2The callbacks receive typed
LlmRequestvalues plusLlmContinuationV2orLlmStreamContinuationV2. Targeted calls usecall()oropen_stream(); ordinary buffered calls usecall_passthrough(). Streaming callbacks return either a boxed RustStreamorLlmStreamExecutionOutcomeV2::Passthrough(request).LlmProviderStreamV2implementsStream, enforces one pending poll, cancels unfinished production on drop, and releases its host reference exactly once. Continuations are cloneable and support repeated or concurrent calls through shared RAII state. The SDK owns host strings, JSON conversion, callback trampolines, panic isolation, completion settlement, output pumping, cancellation, and handle release.Safe callbacks run to completion on Relay's existing blocking callback lane through a cancellation-aware, TLS-free current-thread executor.
nemo-relay-pluginaddsfuturesbut does not add Tokio or spawn a helper thread that could outlive the plugin library. The raw_rawregistrations remain available for advanced and non-Rust consumers, including theirPendingcontract.Targeted LLM continuation contract
LlmContinuationInvocationV2carries a replacementLlmRequestplus an explicitLlmContinuationTargetV2: HTTP method, absolute HTTP(S) URL, protocol route, and target-owned headers. Relay validates and binds the target to that continuation invocation. The terminal core step consumes it only after the remaining LLM intercepts run.The target is never inserted into
LlmRequest.headers, provider JSON, marks, spans, or logs. Validation rejects unsafe methods, URL credentials, redirects, hop-by-hop and host-owned headers, and allx-nemo-relay-internal-*names. Target credentials reach only the selected provider request; debug output redacts header values and URL query data.Buffered outcomes distinguish provider JSON, bounded HTTP failures, and non-HTTP failures. Streams expose decoded JSON events, completion, or a late non-HTTP failure. Retry disposition is derived by the shared SDK helper from HTTP status or non-HTTP kind and is not serialized. ABI status codes remain reserved for ABI faults.
Host-owned streaming pass-through
V4 appends
async_llm_next_forward_stream_v2without reordering any existing member. For an unmanaged streaming request, Relay connects the original downstream continuation directly to the caller-facing output stream through its bounded 32-event queue. Provider events do not cross into the plugin, slow consumers apply backpressure, cancellation propagates, and terminal settlement happens once.flowchart LR A["Caller or embedded host"] --> B["Relay LLM pipeline"] B --> C["Native routing plugin"] C --> D["Safe Rust v2 callback"] D -->|"managed"| E["LlmContinuationV2 call / open_stream"] E --> F["Raw C ABI V4 continuation"] F --> G["Remaining LLM middleware"] G --> H["Core targeted HTTP JSON / SSE dispatch"] H --> I["Selected provider"] I --> E E --> J["Plugin policy result stream"] D -->|"unmanaged stream"| K["Passthrough(request)"] K --> L["V4 direct-forward hook"] L --> G G --> M["Bounded Relay output queue"] J --> A M --> A X["No target, credentials, raw handles, or stream pump in plugin code"] -.-> DCompatibility and scope
compat.native_api = "1"continues through the unchanged v1 contract.compat.native_api = "2"selects the v2-only registration path.abi_version = 4).nemo_relay_plugin!remains the v1 export macro;nemo_relay_plugin_v2!remains the v2 export macro.serde_json::Value, allocator-owned Rust string, orFlowErrorcrosses the C boundary.Relationship to Switchyard
run_streamrouting plugin using those safe continuations.Validation
cargo test -p nemo-relay-plugin: 83 safe/typed SDK tests plus the optimization contract passed.cargo test -p nemo-relay --lib: 1,058 core tests passed.cargo deny check: advisories, bans, licenses, and sources passed; existing duplicate-dependency warnings remain informational.just test-python: 610 passed;just test-python-plugin: 125 passed with the worker integration suite.just test-go: passed;just test-node: 344/344 passed.just docsandjust docs-linkcheck: passed with zero Fern errors; authenticated redirect validation was skipped withoutFERN_TOKEN.just test-rustrun then reached an unrelated existing core doctest that references the nonexistentnemo_relay::Resultalias.cargo clippy -p nemo-relay-plugin -p nemo-relay --all-targets -- -D warnings,cargo fmt --all -- --check, andgit diff --check: passed.Where should the reviewer start?
crates/plugin/src/native_v2.rsfor the safe Rust facade and ownership model.crates/core/src/plugin/dynamic/native.rsfor the append-only host hook, bounded forwarding, and raw-to-core adapter.crates/core/tests/fixtures/native_plugin/src/lib.rsfor the intended plugin-author experience.crates/plugin/tests/typed_callbacks.rsfor safe wrapper contracts and failure cleanup.crates/core/tests/integration/native_plugin_tests.rsfor embedded-core targeted dispatch and pass-through proof.The key design boundary is that C remains the binary ABI, while Rust authors get a safe SDK over it. Targeted LLM execution remains a core continuation operation and does not depend on CLI gateway state.
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)