feat(relay): add Switchyard-owned HTTP dynamic plugin - #270
Conversation
0e3eb27 to
58b3186
Compare
58b3186 to
3dcee4d
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>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
WalkthroughThe pull request adds a native Switchyard NeMo Relay plugin with configuration, routing, translation, asynchronous host integration, streaming support, packaging, and documentation. It also adds HTTP transport limits, redirect rejection, timeout defaults, header redaction, and safer error reporting. ChangesTransport safety
NeMo Relay plugin
Estimated code review effort: 5 (Critical) | ~120 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (12)
crates/switchyard-nemo-relay-plugin/src/config.rs (2)
411-421: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winExtend the sensitive-header check beyond the exact-match list.
is_sensitive_target_headeruses a fixed denylist. A provider credential header outside that list, for examplex-provider-tokenoropenai-api-key, still passesvalidate_headersand gets stored as plaintext in Relay configuration. The guard at Line 92 exists to prevent exactly that.Add a substring heuristic so unlisted credential headers also route through
header_env.🔒 Proposed change
fn is_sensitive_target_header(name: &str) -> bool { matches!( name, "authorization" | "cookie" | "x-api-key" | "api-key" | "anthropic-api-key" | "x-goog-api-key" ) || name.contains("api-key") || name.contains("api_key") || name.contains("token") || name.contains("secret") || name.contains("password") }🤖 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/switchyard-nemo-relay-plugin/src/config.rs` around lines 411 - 421, Update is_sensitive_target_header to retain the existing exact-match denylist and also return true when the header name contains a credential-related substring, such as “api-key” or “token,” so unlisted provider credential headers are routed through header_env by validate_headers instead of stored as plaintext.
700-726: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a regression test for an invalid environment-supplied header value.
The tests cover an unset variable and invalid variable names. They do not cover
validate_header(name, &value)at Line 129, which rejects a malformed value read from the environment, for example a value that contains a newline or a control character. That path blocks header injection into the provider request.Add a case that sets a variable to an invalid value and asserts that
prepare()fails.Based on learnings from the coding guidelines: "Define verifiable success criteria, write regression tests for bugs and invalid inputs, and verify each implementation step."
🤖 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/switchyard-nemo-relay-plugin/src/config.rs` around lines 700 - 726, Add a regression test alongside validation_does_not_resolve_environment_backed_headers and invalid_environment_variable_names_are_rejected_before_resolution that sets the referenced environment variable to a malformed header value, such as one containing a newline or control character, then calls config.prepare() and asserts it returns an error. Keep the existing variable-name validation coverage unchanged and verify the failure identifies the invalid header value.Source: Coding guidelines
crates/switchyard-nemo-relay-plugin/src/translation.rs (2)
60-68: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a test for the Anthropic JSON-schema rejection.
request_policyis the runtime backstop that stops a JSON-schema response format from reaching ananthropic_messagestarget.config.rsat Line 311 rejects only an Anthropic classifier target at configuration time, so this policy is the sole guard for a routed Anthropic target. No test covers it.Add a case that builds an
LlmRequestwith a JSON-schema response format, then assertsvalidate_target_requestfails forWireFormat::AnthropicMessagesand succeeds forWireFormat::OpenAiChat.As per coding guidelines: "Define verifiable success criteria, write regression tests for bugs and invalid inputs, and verify each implementation step."
🤖 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/switchyard-nemo-relay-plugin/src/translation.rs` around lines 60 - 68, Add a regression test covering request_policy’s JSON-schema capability restriction: construct an LlmRequest using a JSON-schema response format, assert validate_target_request rejects it for WireFormat::AnthropicMessages, and assert validation succeeds for WireFormat::OpenAiChat.Source: Coding guidelines
1-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTwo new modules in this crate have no
//!module comment.client.rsstates its intent at Line 4, buttranslation.rsandruntime.rsdo not. The shared root cause is one missing convention pass over the new modules.
crates/switchyard-nemo-relay-plugin/src/translation.rs#L1-L11: add a//!comment stating that the module adapts Relay request and response bodies to Switchyard protocol types and applies the plugin translation policies.crates/switchyard-nemo-relay-plugin/src/runtime.rs#L1-L19: add a//!comment stating that the module decodes inbound requests, drives the libsy algorithm with retries and a trusted fallback, and encodes buffered or streaming responses back to the host.As per coding guidelines: "For Rust changes, document public items with
///comments and add concise comments for module intent, private helpers with non-obvious behavior, important tests, and complex validation, routing, configuration, async, lifecycle, or concurrency logic."🤖 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/switchyard-nemo-relay-plugin/src/translation.rs` around lines 1 - 11, Add concise //! module documentation to crates/switchyard-nemo-relay-plugin/src/translation.rs#L1-L11 describing adaptation between Relay request/response bodies and Switchyard protocol types, including application of plugin translation policies. Also document crates/switchyard-nemo-relay-plugin/src/runtime.rs#L1-L19 with its role in decoding inbound requests, driving libsy with retries and a trusted fallback, and encoding buffered or streaming responses; no other changes are needed.Source: Coding guidelines
crates/switchyard-nemo-relay-plugin/src/runtime.rs (2)
365-370: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
tracinginstead ofeprintln!.This code runs inside a plugin loaded by the Relay host.
eprintln!writes to raw stderr, so the message bypasses the host log pipeline and carries no level or structured fields. The workspace already usestracing, for example the spans incrates/libsy-llm-client/src/client.rs.Replace the call with
tracing::warn!.♻️ Proposed change
if let Err(error) = parent.emit_mark(name, &data, metadata) { - eprintln!("Switchyard could not emit routing mark {name:?}: {error}"); + tracing::warn!(mark = name, %error, "Switchyard could not emit routing mark"); }🤖 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/switchyard-nemo-relay-plugin/src/runtime.rs` around lines 365 - 370, Update the error handling in mark to replace the raw eprintln! call with tracing::warn!, preserving the existing routing-mark error message and including the name and error fields in the structured log.
565-601: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit tests for the retry and stream helpers.
This module holds the retry, fallback, and streaming state machine, and has one test that covers a pure helper.
libsy_error_retryable,failure_mark_data, andreturned_eventsneed no Relay host and are testable now.Cover at minimum:
libsy_error_retryablereturnstruefor each listed status andfalsefor 400, 401, and 404.returned_eventsrejects an emptyLlmResponse::Streamand preserves the first chunk otherwise.failure_mark_datasetsfailure_kindtohttp,non_http, andalgorithmfor the three branches.These tests would have caught the two defects flagged at Lines 134-197 and Lines 259-266.
As per coding guidelines: "Define verifiable success criteria, write regression tests for bugs and invalid inputs, and verify each implementation step."
🤖 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/switchyard-nemo-relay-plugin/src/runtime.rs` around lines 565 - 601, Extend the existing tests module with unit tests for the pure helpers libsy_error_retryable, returned_events, and failure_mark_data. Verify retryability for every listed status plus false for 400, 401, and 404; ensure returned_events rejects an empty LlmResponse::Stream and retains the first chunk for a non-empty stream; and assert failure_mark_data produces http, non_http, and algorithm for its three branches without requiring a Relay host.Source: Coding guidelines
crates/switchyard-nemo-relay-plugin/scripts/package_bundle.py (2)
16-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider
hashlib.file_digestinstead of the manual chunk loop.The coding guidelines target Python 3.12+.
hashlib.file_digestis available from 3.11 and removes the read loop.♻️ Proposed change
def digest(path: Path) -> str: """Return the lowercase SHA-256 digest for a file.""" - value = hashlib.sha256() - with path.open("rb") as stream: - for chunk in iter(lambda: stream.read(1024 * 1024), b""): - value.update(chunk) - return value.hexdigest() + with path.open("rb") as stream: + return hashlib.file_digest(stream, "sha256").hexdigest()🤖 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/switchyard-nemo-relay-plugin/scripts/package_bundle.py` around lines 16 - 22, Update the digest function to use hashlib.file_digest with the opened file stream and SHA-256, replacing the manual chunk-reading loop while preserving the lowercase hexadecimal digest returned by hexdigest().Source: Coding guidelines
36-51: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winValidate
config.schema.jsonbefore the script copies files.The script validates the library and the manifest placeholders before it mutates the output directory. It does not validate
config.schema.json. If that file is missing,shutil.copy2raisesFileNotFoundErrorafter the library copy already ran. The output directory is then partially populated and no longer empty, so a rerun fails the emptiness check.♻️ Proposed change
manifest = (CRATE_ROOT / "relay-plugin.toml").read_text(encoding="utf-8") placeholders = ("<platform-library-file>", "<artifact-sha256>") missing = [placeholder for placeholder in placeholders if placeholder not in manifest] if missing: parser.error(f"plugin manifest is missing placeholders: {', '.join(missing)}") + schema = CRATE_ROOT / "config.schema.json" + if not schema.is_file(): + parser.error(f"plugin configuration schema does not exist: {schema}") + output = args.output.resolve() @@ shutil.copy2(library, artifact) - shutil.copy2(CRATE_ROOT / "config.schema.json", output / "config.schema.json") + shutil.copy2(schema, output / "config.schema.json")🤖 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/switchyard-nemo-relay-plugin/scripts/package_bundle.py` around lines 36 - 51, Validate the existence and required file condition for config.schema.json before creating or mutating the output directory in the packaging flow. Update the logic around the existing manifest and library validation, using the config.schema.json source path, so missing-file errors are reported through parser.error before shutil.copy2 performs either copy; preserve the existing output-directory checks and copy behavior otherwise.crates/switchyard-nemo-relay-plugin/src/executor.rs (1)
113-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a bounded wait in this test.
receiver.recv()blocks forever if the spawned task never runs. The test then hangs CI instead of failing. The second test already usesrecv_timeout. Apply the same pattern here.♻️ Proposed change
- assert_eq!(receiver.recv().unwrap(), "done"); + assert_eq!( + receiver + .recv_timeout(std::time::Duration::from_secs(5)) + .expect("spawned work must complete"), + "done" + );🤖 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/switchyard-nemo-relay-plugin/src/executor.rs` around lines 113 - 121, Update executor_runs_buffered_and_spawned_work to replace the unbounded receiver.recv() call with receiver.recv_timeout(), using the same bounded-wait pattern and timeout established by the neighboring test.crates/switchyard-nemo-relay-plugin/src/lib.rs (2)
365-401: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for an unsupported future version.
The tests cover version 1 and a non-integer version. The
Some(version)branch for any other integer is untested. Add a case forversion = 3and assert the "unsupported Switchyard config version" message. The coding guidelines require regression tests for invalid inputs.🤖 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/switchyard-nemo-relay-plugin/src/lib.rs` around lines 365 - 401, Add a regression test alongside version_one_service_config_gets_a_migration_error_before_v2_deserialization and version_must_be_an_integer that passes {"version": 3} to parse_config, asserts parsing fails, and verifies the error contains the “unsupported Switchyard config version” message.Source: Coding guidelines
4-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd module intent comments to the new modules. The new crate omits module-level comments in two files.
ffi.rsincludes a//!comment; these two do not.
crates/switchyard-nemo-relay-plugin/src/lib.rs#L4-L9: add a crate-level//!comment that states the plugin's purpose and the host ABI it targets.crates/switchyard-nemo-relay-plugin/src/executor.rs#L4-L11: add a//!comment that states why the plugin owns a dedicated Tokio runtime thread.The coding guidelines require concise comments for module intent in
crates/**/*.rs.🤖 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/switchyard-nemo-relay-plugin/src/lib.rs` around lines 4 - 9, Add concise module-level intent comments at both affected sites: in crates/switchyard-nemo-relay-plugin/src/lib.rs lines 4-9, add a crate-level //! comment describing the plugin’s purpose and targeted host ABI; in crates/switchyard-nemo-relay-plugin/src/executor.rs lines 4-11, add a //! comment explaining why the plugin owns a dedicated Tokio runtime thread.Source: Coding guidelines
crates/switchyard-nemo-relay-plugin/src/ffi.rs (1)
323-337: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider the wakeup cost of cancellation polling at high concurrency.
Each in-flight call adds one timer wakeup every 10 ms on the two-worker runtime. With thousands of concurrent calls this becomes a constant background load. The host table exposes no cancellation notification, so polling is reasonable now. Consider a backoff that starts short and grows to a longer interval, or make the interval configurable.
🤖 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/switchyard-nemo-relay-plugin/src/ffi.rs` around lines 323 - 337, Reduce cancellation polling overhead in wait_for_completion_cancellation and wait_for_stream_cancellation by replacing the fixed CANCELLATION_POLL delay with a short initial interval that backs off to a configurable or bounded maximum. Preserve prompt cancellation detection while preventing thousands of in-flight calls from waking every 10 ms indefinitely.
🤖 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/libsy-llm-client/src/client.rs`:
- Around line 285-306: The response handling tests need coverage for an
oversized successful response. Add a regression test in the existing client test
suite that returns a 2xx response whose body exceeds
MAX_BUFFERED_RESPONSE_BYTES, then assert the request fails with
LlmClientError::InvalidResponse while preserving the existing oversized
error-body tests.
In `@crates/switchyard-nemo-relay-plugin/src/ffi.rs`:
- Around line 368-423: Bound the Internal-status retry loops in push_stream and
reject_stream using the existing timing imports and a shared
MAX_BACKPRESSURE_WAIT duration near the other limits. Stop retrying once the
deadline is reached and return an error/status that lets the callback settle the
stream, while preserving cancellation handling and normal successful or
non-Internal responses.
In `@crates/switchyard-nemo-relay-plugin/src/runtime.rs`:
- Around line 372-392: The emit_decision method currently sends prompt-derived
decision.reasoning through ParentScope::emit_mark; constrain this field before
including it in the routing mark. Prefer truncating reasoning to a fixed maximum
length (or gate it behind an off-by-default configuration flag), while
preserving identifier-only metadata and existing decision fields.
- Around line 92-100: Add exponential backoff before retry iterations in the
routing retry loops, including both non-streaming and streaming paths such as
the visible retry branch and execute_stream. When the upstream failure includes
a Retry-After value, use that delay instead of the calculated backoff; otherwise
apply the existing retry-attempt count to compute an exponentially increasing
sleep before re-driving the request.
- Around line 259-266: Bound the outer event-processing loop in the runtime flow
around committed and retry handling so a pass that ends with committed == false
and no retry arm cannot restart indefinitely. Track whether a retry occurred
during the pass, or otherwise detect that no progress was made, and return an
appropriate error before re-entering the outer loop; preserve normal retry and
successful commitment behavior.
- Around line 134-197: Make the fallback_used binding mutable in the surrounding
request loop, and set it to true in the Err(failure) if !fallback_used arm
immediately before switching to the trusted fallback stream via
fallback_response. Preserve the existing retry and error handling for failures
that occur before fallback activation.
---
Nitpick comments:
In `@crates/switchyard-nemo-relay-plugin/scripts/package_bundle.py`:
- Around line 16-22: Update the digest function to use hashlib.file_digest with
the opened file stream and SHA-256, replacing the manual chunk-reading loop
while preserving the lowercase hexadecimal digest returned by hexdigest().
- Around line 36-51: Validate the existence and required file condition for
config.schema.json before creating or mutating the output directory in the
packaging flow. Update the logic around the existing manifest and library
validation, using the config.schema.json source path, so missing-file errors are
reported through parser.error before shutil.copy2 performs either copy; preserve
the existing output-directory checks and copy behavior otherwise.
In `@crates/switchyard-nemo-relay-plugin/src/config.rs`:
- Around line 411-421: Update is_sensitive_target_header to retain the existing
exact-match denylist and also return true when the header name contains a
credential-related substring, such as “api-key” or “token,” so unlisted provider
credential headers are routed through header_env by validate_headers instead of
stored as plaintext.
- Around line 700-726: Add a regression test alongside
validation_does_not_resolve_environment_backed_headers and
invalid_environment_variable_names_are_rejected_before_resolution that sets the
referenced environment variable to a malformed header value, such as one
containing a newline or control character, then calls config.prepare() and
asserts it returns an error. Keep the existing variable-name validation coverage
unchanged and verify the failure identifies the invalid header value.
In `@crates/switchyard-nemo-relay-plugin/src/executor.rs`:
- Around line 113-121: Update executor_runs_buffered_and_spawned_work to replace
the unbounded receiver.recv() call with receiver.recv_timeout(), using the same
bounded-wait pattern and timeout established by the neighboring test.
In `@crates/switchyard-nemo-relay-plugin/src/ffi.rs`:
- Around line 323-337: Reduce cancellation polling overhead in
wait_for_completion_cancellation and wait_for_stream_cancellation by replacing
the fixed CANCELLATION_POLL delay with a short initial interval that backs off
to a configurable or bounded maximum. Preserve prompt cancellation detection
while preventing thousands of in-flight calls from waking every 10 ms
indefinitely.
In `@crates/switchyard-nemo-relay-plugin/src/lib.rs`:
- Around line 365-401: Add a regression test alongside
version_one_service_config_gets_a_migration_error_before_v2_deserialization and
version_must_be_an_integer that passes {"version": 3} to parse_config, asserts
parsing fails, and verifies the error contains the “unsupported Switchyard
config version” message.
- Around line 4-9: Add concise module-level intent comments at both affected
sites: in crates/switchyard-nemo-relay-plugin/src/lib.rs lines 4-9, add a
crate-level //! comment describing the plugin’s purpose and targeted host ABI;
in crates/switchyard-nemo-relay-plugin/src/executor.rs lines 4-11, add a //!
comment explaining why the plugin owns a dedicated Tokio runtime thread.
In `@crates/switchyard-nemo-relay-plugin/src/runtime.rs`:
- Around line 365-370: Update the error handling in mark to replace the raw
eprintln! call with tracing::warn!, preserving the existing routing-mark error
message and including the name and error fields in the structured log.
- Around line 565-601: Extend the existing tests module with unit tests for the
pure helpers libsy_error_retryable, returned_events, and failure_mark_data.
Verify retryability for every listed status plus false for 400, 401, and 404;
ensure returned_events rejects an empty LlmResponse::Stream and retains the
first chunk for a non-empty stream; and assert failure_mark_data produces http,
non_http, and algorithm for its three branches without requiring a Relay host.
In `@crates/switchyard-nemo-relay-plugin/src/translation.rs`:
- Around line 60-68: Add a regression test covering request_policy’s JSON-schema
capability restriction: construct an LlmRequest using a JSON-schema response
format, assert validate_target_request rejects it for
WireFormat::AnthropicMessages, and assert validation succeeds for
WireFormat::OpenAiChat.
- Around line 1-11: Add concise //! module documentation to
crates/switchyard-nemo-relay-plugin/src/translation.rs#L1-L11 describing
adaptation between Relay request/response bodies and Switchyard protocol types,
including application of plugin translation policies. Also document
crates/switchyard-nemo-relay-plugin/src/runtime.rs#L1-L19 with its role in
decoding inbound requests, driving libsy with retries and a trusted fallback,
and encoding buffered or streaming responses; no other changes are needed.
🪄 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: CHILL
Plan: Enterprise
Run ID: 3570ba9b-fc47-4a61-b0ff-3d3a77c017b1
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!Cargo.lock
📒 Files selected for processing (22)
CHANGELOG.mdCargo.tomlREADME.mdcrates/libsy-llm-client/README.mdcrates/libsy-llm-client/src/backend.rscrates/libsy-llm-client/src/client.rscrates/libsy/src/core/algorithm.rscrates/protocol/src/client.rscrates/switchyard-nemo-relay-plugin/Cargo.tomlcrates/switchyard-nemo-relay-plugin/README.mdcrates/switchyard-nemo-relay-plugin/config.schema.jsoncrates/switchyard-nemo-relay-plugin/relay-plugin.tomlcrates/switchyard-nemo-relay-plugin/scripts/package_bundle.pycrates/switchyard-nemo-relay-plugin/src/client.rscrates/switchyard-nemo-relay-plugin/src/config.rscrates/switchyard-nemo-relay-plugin/src/executor.rscrates/switchyard-nemo-relay-plugin/src/ffi.rscrates/switchyard-nemo-relay-plugin/src/lib.rscrates/switchyard-nemo-relay-plugin/src/runtime.rscrates/switchyard-nemo-relay-plugin/src/translation.rscrates/switchyard-translation/src/helpers.rsdocs/index.md
3dcee4d to
1815a45
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>
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>
| &metadata, | ||
| ); | ||
| let fallback = self | ||
| .fallback_response(inbound, request.clone(), &mut marks, &metadata) |
There was a problem hiding this comment.
[P1] If fallback_response(...).await fails here, we return the terminal error but never flush the switchyard.routing.error and switchyard.routing.fallback marks we just accumulated because send_marks() is only reached on the success path. I reproduced this with a selected empty stream plus a failing fallback HTTP client.
| let mut normalized = BTreeSet::new(); | ||
| for (name, value) in &self.headers { | ||
| let canonical = validate_header(name, value)?; | ||
| if is_sensitive_target_header(&canonical) { |
There was a problem hiding this comment.
[P1] This check is narrower than the config contract. The docs and schema say credential-bearing headers should go through header_env, but custom secret-like headers such as x-provider-token still pass validation here and end up stored as static plaintext config. I reproduced that with a focused config test.
There was a problem hiding this comment.
Removed literal headers configuration and the sensitive-name allowlist.
Made header_env the sole custom provider-header source.
Preserved forbidden transport-header and case-insensitive duplicate checks.
Added regression coverage for x-provider-token, including verification that its value is not echoed in errors.
Updated the schema, README, client documentation, and PR description.
Reduced the change by 24 net lines.
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
4 participants
What
Adds the external
nvidia.switchyardNeMo Relay native plugin. The plugin embedsswitchyard-libsy, drivesAlgorithm::run_stream, and usesswitchyard-llm-clientfor provider HTTP dispatch rather than depending on aSwitchyard service or a Relay targeted-provider continuation.
The initial integration supports seeded weighted-random and LLM-classifier
routing for buffered and streaming OpenAI Chat, OpenAI Responses, and Anthropic
Messages traffic. It consumes the stream-preservation contract landed in #192,
and sends buffered final-response failures through the existing routing error
and exactly-once trusted-fallback path. All custom provider headers are sourced
through
header_env, so literal header values are never stored in pluginconfiguration.
Why
Switchyard algorithms need to own the complete
run_streamlifecycle so futurepolicies can inspect intermediate responses and make multiple provider calls.
Owning provider HTTP dispatch inside the plugin keeps that lifecycle intact
while using NeMo Relay's released native API v1 Rust SDK. It removes the need
for the targeted LLM continuation contract proposed in NVIDIA/NeMo-Relay#594.
Related: #192, #220, #271, #274, NVIDIA/NeMo-Relay#594
How tested
uv run ruff check .clean (N/A: Rust-only plugin)uv run mypy switchyardclean (N/A: Rust-only plugin)uv run pytest tests/green (N/A: Rust-only plugin)cargo test -p switchyard-nemo-relay-plugin— 31 focused unit testscargo test --workspace --all-targetscargo clippy --workspace --all-targets -- -D warningscargo fmt --all -- --checkgit diff --checkChecklist
snake_caseof the primary class. (N/A: no Python classes added.)switchyard/__init__.py.__all__if intended for downstream use. (N/A: Rust plugin surface.)--helpupdated if customer-facing surface changed.Signed-off-by: Your Name <email>) per the DCO.Notes for reviewers
0.7.0-rc.5crate anddeclares
>=0.7.0-rc.5,<1.0. These move to stable0.7.0before merge. RC5also removes the platform-specific timezone dependency expansion present in
the RC4 lockfile.
intercept and Relay's provider callback. Their HTTP subcalls therefore do not
create nested Relay LLM lifecycle spans; the outer LLM span and genuine
Switchyard routing marks remain in the same exported trace.
event, including provider-specific fields. This preserves parsed JSON, not raw
SSE bytes or framing. Cross-protocol streams still use normalized chunks
without reject-lossy diagnostics; replacing normalized content or aggregating
the stream drops its per-event preservation envelope.
header_envexclusively, including non-secretrouting and tenancy metadata. Literal
headersconfiguration is rejected.api-version, arerejected by the initial client contract.
Adjacent hardening
This PR is self-contained. Two independent follow-ups deliberately keep broader
library changes out of its review scope:
switchyard-llm-clientfor alllibrary consumers. It is not required to build or run this plugin.
LossyConversionPolicywhile encoding buffered responses. Itcomplements this PR's finalization path: a strict cross-protocol conversion
that would discard response data becomes a translation error, and this plugin
can then use its existing exactly-once trusted fallback.