feat: add provider-specific gateway authentication#440
Conversation
github-actions
Bot
added
size:L
WalkthroughGateway configuration now supports validated OpenAI and Anthropic authentication headers from TOML and environment variables. Headers participate in endpoint merging, managed fingerprints, and provider-specific request forwarding, with expanded configuration and gateway test coverage. ChangesProvider authentication
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway
participant ProviderForwarding
participant AuthInjection
participant Provider
Client->>Gateway: Send provider request
Gateway->>ProviderForwarding: Resolve target route and configured header
ProviderForwarding->>AuthInjection: Supply route-specific auth
AuthInjection->>Provider: Inject or replace Authorization
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/cli/src/gateway/routes.rs`:
- Around line 17-49: The route-to-auth-header mapping is duplicated between
ProviderForwarding::configured_auth_header and
ProviderRoute::configured_auth_header. Extract a shared private helper accepting
the route plus OpenAI and Anthropic header options, then delegate both methods
to it while preserving their existing return behavior and route mappings.
In `@crates/cli/tests/coverage/shared/config_tests.rs`:
- Around line 525-571: Update
invalid_provider_auth_header_errors_do_not_expose_secret_values to use valid
TOML while still passing a newline-containing header to validate_auth_header, so
the test exercises validation and secret redaction rather than TOML parsing. Add
an equivalent config-based invalid-header test for Anthropic, asserting the
Anthropic field is identified, the valid HTTP header error appears, and both
secret fragments are absent.
🪄 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: 92c09def-54d0-493a-8f91-14920c3f5886
📒 Files selected for processing (15)
crates/cli/src/configuration/mod.rscrates/cli/src/configuration/types.rscrates/cli/src/gateway/mod.rscrates/cli/src/gateway/request.rscrates/cli/src/gateway/routes.rscrates/cli/src/mcp_environment.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/session_tests.rsdocs/nemo-relay-cli/basic-usage.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/codex.mdxintegrations/coding-agents/codex/.mcp.json
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (15)
**/*.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/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/basic-usage.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/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/basic-usage.mdx
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
docs/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/basic-usage.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/basic-usage.mdx
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
docs/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/claude-code.mdxcrates/cli/src/configuration/types.rscrates/cli/src/gateway/request.rscrates/cli/src/mcp_environment.rscrates/cli/src/server/mod.rsdocs/nemo-relay-cli/basic-usage.mdxcrates/cli/src/gateway/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/src/gateway/routes.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/session_tests.rs
docs/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If documentation examples or commands under
docs/change, run the targeted docs checks appropriate to the change.
Files:
docs/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/basic-usage.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/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/basic-usage.mdx
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/cli/src/configuration/types.rscrates/cli/src/gateway/request.rscrates/cli/src/mcp_environment.rscrates/cli/src/server/mod.rscrates/cli/src/gateway/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/src/gateway/routes.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/session_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/cli/src/configuration/types.rscrates/cli/src/gateway/request.rscrates/cli/src/mcp_environment.rscrates/cli/src/server/mod.rscrates/cli/src/gateway/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/src/gateway/routes.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/session_tests.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/cli/src/configuration/types.rscrates/cli/src/gateway/request.rscrates/cli/src/mcp_environment.rscrates/cli/src/server/mod.rscrates/cli/src/gateway/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/src/gateway/routes.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/session_tests.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/cli/src/configuration/types.rscrates/cli/src/gateway/request.rscrates/cli/src/mcp_environment.rscrates/cli/src/server/mod.rscrates/cli/src/gateway/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/src/gateway/routes.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/session_tests.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/cli/src/configuration/types.rscrates/cli/src/gateway/request.rscrates/cli/src/mcp_environment.rscrates/cli/src/server/mod.rscrates/cli/src/gateway/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/src/gateway/routes.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/session_tests.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/cli/src/configuration/types.rscrates/cli/src/gateway/request.rscrates/cli/src/mcp_environment.rscrates/cli/src/server/mod.rscrates/cli/src/gateway/mod.rscrates/cli/src/gateway/routes.rscrates/cli/src/configuration/mod.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/cli/src/configuration/types.rscrates/cli/src/gateway/request.rscrates/cli/src/mcp_environment.rscrates/cli/src/server/mod.rscrates/cli/src/gateway/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/src/gateway/routes.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/session_tests.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/session_tests.rs
🔇 Additional comments (18)
docs/nemo-relay-cli/basic-usage.mdx (1)
145-181: LGTM!Also applies to: 269-271
docs/nemo-relay-cli/claude-code.mdx (1)
73-82: LGTM!docs/nemo-relay-cli/codex.mdx (1)
120-122: LGTM!integrations/coding-agents/codex/.mcp.json (1)
39-45: LGTM!crates/cli/tests/coverage/shared/config_tests.rs (1)
29-37: LGTM!Also applies to: 43-100, 103-160, 425-476, 491-523, 1978-2020
crates/cli/src/configuration/mod.rs (2)
16-16: LGTM!Also applies to: 63-68, 202-204, 1124-1124, 1149-1172, 1433-1456
1468-1476: 🎯 Functional Correctness | ⚡ Quick winTrim the value before validating/storing it.
validate_auth_headerchecksvalue.trim().is_empty()but returns the original, untrimmedvalue. Stray leading/trailing whitespace (e.g. from a TOML multiline string or env var with a trailing newline) will pass validation and then be sent verbatim as theAuthorizationheader value downstream ininject_provider_auth_with_env— which, for the env-var fallback path, explicitly trims for this exact reason ("sendingBearerwith leading whitespace can confuse upstream auth parsers").🐛 Proposed fix
fn validate_auth_header(name: &str, value: String) -> Result<String, CliError> { - if value.trim().is_empty() { + let value = value.trim().to_string(); + if value.is_empty() { return Err(CliError::Config(format!("{name} must not be empty"))); } HeaderValue::from_str(&value) .map_err(|_| CliError::Config(format!("{name} must be a valid HTTP header value")))?; Ok(value) }crates/cli/src/configuration/types.rs (1)
24-26: LGTM!Also applies to: 115-117
crates/cli/src/gateway/request.rs (1)
33-39: LGTM!Also applies to: 59-59, 75-75
crates/cli/src/server/mod.rs (1)
442-457: LGTM!crates/cli/src/mcp_environment.rs (1)
41-46: LGTM!crates/cli/src/gateway/mod.rs (2)
71-73: LGTM!Also applies to: 134-138, 260-267, 447-454, 720-724, 733-739, 955-959, 1103-1128
872-914: 🎯 Functional CorrectnessConfirm configured auth headers are documented as full header values, not bare keys.
When
configured_auth_headeris set, it's written verbatim toAuthorizationfor every route (including Anthropic, which otherwise usesx-api-key), with no scheme normalization — unlike the env-var fallback a few lines below, which prependsBearerfor OpenAI routes. This appears intentional (openai_auth_header/anthropic_auth_headerare meant to hold a complete header value), but please confirm the docs make this explicit so users don't configure a bare API key expecting automaticBearerprefixing.crates/cli/tests/coverage/shared/gateway_tests.rs (3)
98-98: LGTM!Also applies to: 202-207, 219-225, 238-243, 594-598, 785-789, 911-923, 941-945, 1021-1034, 1044-1056, 1068-1072, 1192-1198, 1229-1235
992-1012: LGTM! Good coverage of provider-isolation and inbound-suppression precedence per path instructions on cross-request isolation testing.
956-990: 🎯 Functional CorrectnessNo issue:
configured_auth_headeris route-drivenProviderForwarding::newonly snapshots the configured auth headers;configured_auth_header(route)matches on the route argument, so reusing the same forwarding value across different routes is valid.> Likely an incorrect or invalid review comment.crates/cli/tests/coverage/shared/server_tests.rs (1)
160-172: LGTM!Also applies to: 175-203, 426-449, 458-458
crates/cli/tests/coverage/shared/session_tests.rs (1)
724-726: LGTM!Also applies to: 1977-1979, 2237-2239, 2318-2320, 2402-2404, 3709-3711, 3787-3789, 3861-3863, 3919-3921, 4404-4406, 4533-4535, 4632-4634, 4749-4751, 4844-4846, 6675-6677, 6691-6693
|
Codex comment worth investigating: Configured credentials can leak to a plugin-selected URL |
|
Another potentially valid comment from codex regarding OpenAI Oauth vs. OPENAI_API_KEY and a mixup in provenance Configured OpenAI authentication does not replace Codex OAuth. In gateway/routes.rs:180-190, Relay considers only OPENAI_API_KEY when deciding whether it can replace a Codex ChatGPT token: allow_environment_provider_auth && env_var_is_nonempty("OPENAI_API_KEY") NEMO_RELAY_OPENAI_AUTH_HEADER and [upstream].openai_auth_header are ignored. Consequently, a Codex request carrying ChatGPT OAuth can: Continue using its inbound JWT. Be redirected to the ChatGPT backend. Never use the configured OpenAI endpoint or configured authorization header. A valid configured OpenAI header must count as replacement authentication alongside OPENAI_API_KEY. |
bbednarski9
left a comment
bbednarski9
left a comment
There was a problem hiding this comment.
merging #449 first, plus a couple comments
6442bed to
5df30e4
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/cli/tests/coverage/shared/gateway_tests.rs`:
- Around line 848-891: Update configured_openai_auth_replaces_chatgpt_jwt to
exercise the real request-preparation decision path instead of hard-coding
replacement authorization as true. Use the JWT inbound Authorization header with
configured openai_auth_header and no OPENAI_API_KEY, then assert the JWT is
removed and the configured auth is injected into the prepared request.
- Around line 1002-1035: Add a regression test covering plugin dispatch and
target selection rather than only the direct inject_provider_auth_with_env
helper. Exercise a plugin-selected upstream URL, then assert configured route
credentials are withheld unless explicitly bound to that target, while
preserving the existing provider-specific credential expectations for explicitly
bound targets.
🪄 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: 36590605-98de-4c59-a857-3ec917709926
📒 Files selected for processing (15)
crates/cli/src/configuration/mod.rscrates/cli/src/configuration/types.rscrates/cli/src/gateway/mod.rscrates/cli/src/gateway/request.rscrates/cli/src/gateway/routes.rscrates/cli/src/mcp_environment.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/session_tests.rsdocs/nemo-relay-cli/basic-usage.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/codex.mdxintegrations/coding-agents/codex/.mcp.json
📜 Review details
🧰 Additional context used
📓 Path-based instructions (15)
**/*.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/cli/src/mcp_environment.rscrates/cli/src/configuration/types.rscrates/cli/src/gateway/request.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/src/gateway/routes.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/session_tests.rscrates/cli/src/gateway/mod.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/cli/src/mcp_environment.rscrates/cli/src/configuration/types.rscrates/cli/src/gateway/request.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/src/gateway/routes.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/session_tests.rscrates/cli/src/gateway/mod.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/cli/src/mcp_environment.rscrates/cli/src/configuration/types.rscrates/cli/src/gateway/request.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/src/gateway/routes.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/session_tests.rscrates/cli/src/gateway/mod.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/cli/src/mcp_environment.rscrates/cli/src/configuration/types.rscrates/cli/src/gateway/request.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/src/gateway/routes.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/session_tests.rscrates/cli/src/gateway/mod.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/cli/src/mcp_environment.rscrates/cli/src/configuration/types.rscrates/cli/src/gateway/request.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/src/gateway/routes.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/session_tests.rscrates/cli/src/gateway/mod.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/cli/src/mcp_environment.rscrates/cli/src/configuration/types.rscrates/cli/src/gateway/request.rscrates/cli/src/server/mod.rscrates/cli/src/gateway/routes.rscrates/cli/src/configuration/mod.rscrates/cli/src/gateway/mod.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/cli/src/mcp_environment.rsdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/codex.mdxcrates/cli/src/configuration/types.rscrates/cli/src/gateway/request.rsdocs/nemo-relay-cli/basic-usage.mdxcrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/src/gateway/routes.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/session_tests.rscrates/cli/src/gateway/mod.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/cli/src/mcp_environment.rscrates/cli/src/configuration/types.rscrates/cli/src/gateway/request.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/src/gateway/routes.rscrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/session_tests.rscrates/cli/src/gateway/mod.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/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/basic-usage.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/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/basic-usage.mdx
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
docs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/basic-usage.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/basic-usage.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/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/basic-usage.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/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/basic-usage.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/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/session_tests.rs
🔇 Additional comments (10)
crates/cli/src/configuration/mod.rs (1)
1479-1496: Keep endpoint and credential precedence atomic.An environment base-URL override can retain an auth header inherited from TOML, sending a credential to a host selected by a different precedence layer. Clear inherited auth when the URL changes without auth, or resolve each provider’s URL and credential as one unit; cover both providers.
crates/cli/src/gateway/mod.rs (1)
720-740: Do not send configured credentials after plugin target dispatch.
effective_dispatch_requestmay rewrite the route and URL before configured auth is selected, so a Relay-configured credential can be sent to an explicit plugin-selected target. Bind configured auth to Relay’s original/default upstream; explicit targets must use target-owned headers.docs/nemo-relay-cli/basic-usage.mdx (1)
145-181: LGTM!Also applies to: 269-271, 283-294, 312-322, 360-363
docs/nemo-relay-cli/claude-code.mdx (2)
73-78: LGTM!
80-81: 📐 Maintainability & Code QualityRun
just docs-linkcheckfor the new Provider Upstreams links.
docs/nemo-relay-cli/claude-code.mdx#L80-L81: validate/nemo-relay-cli/basic-usage#provider-upstreams.docs/nemo-relay-cli/codex.mdx#L120-L121: validate/nemo-relay-cli/basic-usage#provider-upstreams.As per coding guidelines, changed links in MDX files require
just docs-linkcheck.Source: Coding guidelines
integrations/coding-agents/codex/.mcp.json (1)
39-44: LGTM!crates/cli/tests/coverage/shared/config_tests.rs (1)
12-15: LGTM!Also applies to: 40-41, 56-65, 77-78, 97-104, 123-130, 147-164, 207-212, 443-445, 478-486, 501-602, 864-909, 2054-2096, 3097-3407
crates/cli/tests/coverage/shared/gateway_tests.rs (1)
98-98: LGTM!Also applies to: 205-207, 222-225, 241-243, 597-597, 788-788, 960-969, 990-990, 1038-1057, 1071-1080, 1093-1102, 1117-1117, 1241-1243, 1278-1280
crates/cli/tests/coverage/shared/server_tests.rs (1)
164-166: LGTM!Also applies to: 177-178, 201-202, 436-449, 458-458
crates/cli/tests/coverage/shared/session_tests.rs (1)
268-456: LGTM!Also applies to: 914-916, 2167-2169, 2179-2179, 2224-2235, 2439-2441, 2520-2522, 2604-2606, 3911-3913, 3989-3991, 4063-4065, 4121-4123, 4606-4608, 4735-4737, 4834-4836, 4951-4953, 5046-5048, 6877-6879, 6893-6895
Signed-off-by: Will Killian <wkillian@nvidia.com>
5df30e4 to
f885435
Compare
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Signed-off-by: Will Killian <wkillian@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/cli/src/gateway/mod.rs (1)
693-736: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winConfigured-auth selection is coupled to the post-dispatch target route — currently gated safely, but fragile.
configured_auth_headeris computed fromeffective.target_route(line 716), yet it's only ever consumed whencredential_policy == SourceOrEnvironment, which today is guaranteed to coincide withtarget_route == forwarding.source_route. The leak the previous review flagged (configured/global credentials following a plugin-selected target) doesn't currently manifest becauseTargetCredentialPolicy::ExplicitTargetdisables the whole injection path — but that safety is incidental to howeffective_dispatch_requesthappens to set both fields together, not an explicit invariant. Please confirm this fully satisfies the earlier concern, and consider removing the coupling by deriving the header fromforwarding.source_route(the field it's actually safe to use) so the "configured creds are Relay-default-upstream-only" guarantee doesn't depend on two independently-evolving pieces of logic staying in sync.🔒️ Suggested fix: decouple from target_route
- let configured_auth_header = forwarding.configured_auth_header(effective.target_route); + // Configured credentials are scoped to Relay's original/default upstream only; explicit + // plugin targets must supply their own auth via target-specific binding headers. + let configured_auth_header = forwarding.configured_auth_header(forwarding.source_route);🤖 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/src/gateway/mod.rs` around lines 693 - 736, Update forward_upstream_request so configured_auth_header is derived from forwarding.source_route rather than effective.target_route. Keep the existing credential-policy gating and inject_provider_auth flow unchanged, ensuring configured credentials remain limited to the relay’s source route independently of effective_dispatch_request’s target-route and policy coupling.
🤖 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/src/configuration/mod.rs`:
- Around line 1189-1213: Update apply_file_upstream_config and the corresponding
apply_env_config logic so that when a configuration layer changes an upstream
endpoint without providing that layer’s auth header, it clears the inherited
gateway auth header for that provider. Preserve supplied auth headers after
validation, and apply the same endpoint/auth coupling independently for OpenAI
and Anthropic.
In `@crates/cli/tests/coverage/shared/gateway_tests.rs`:
- Around line 428-519: Add a regression test alongside
explicit_keyless_target_drops_source_credentials and
cross_protocol_target_uses_only_binding_owned_authentication covering
openai_chat/openai_responses dispatch to a different host with GatewayConfig
configured authentication. Verify ProviderForwarding::configured_auth_header
does not forward the configured secret when the target is not explicitly bound,
while preserving forwarding only for an explicitly bound target.
In `@docs/nemo-relay-cli/basic-usage.mdx`:
- Around line 166-185: Update the authentication precedence documentation around
the upstream authorization description to explicitly call out the Codex ChatGPT
OAuth exception. State that when an inbound OAuth-style OpenAI Authorization is
present and a replacement OpenAI credential is configured, Relay strips the
OAuth token and forwards using the configured credential to the ChatGPT backend,
including credentials configured in the file or through the OpenAI auth
environment variable.
---
Outside diff comments:
In `@crates/cli/src/gateway/mod.rs`:
- Around line 693-736: Update forward_upstream_request so configured_auth_header
is derived from forwarding.source_route rather than effective.target_route. Keep
the existing credential-policy gating and inject_provider_auth flow unchanged,
ensuring configured credentials remain limited to the relay’s source route
independently of effective_dispatch_request’s target-route and policy coupling.
🪄 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: 50626514-6ce9-46da-b995-acc57354e5ed
📒 Files selected for processing (14)
crates/cli/src/configuration/mod.rscrates/cli/src/configuration/types.rscrates/cli/src/gateway/mod.rscrates/cli/src/gateway/request.rscrates/cli/src/gateway/routes.rscrates/cli/src/mcp_environment.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/session_tests.rsdocs/nemo-relay-cli/basic-usage.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/codex.mdxintegrations/coding-agents/codex/.mcp.json
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (15)
**/*.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/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/basic-usage.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/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/basic-usage.mdx
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
docs/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/basic-usage.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/basic-usage.mdx
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
docs/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/claude-code.mdxcrates/cli/src/mcp_environment.rscrates/cli/src/configuration/types.rsdocs/nemo-relay-cli/basic-usage.mdxcrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/session_tests.rscrates/cli/src/gateway/request.rscrates/cli/src/configuration/mod.rscrates/cli/src/gateway/mod.rscrates/cli/src/gateway/routes.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rs
docs/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If documentation examples or commands under
docs/change, run the targeted docs checks appropriate to the change.
Files:
docs/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/basic-usage.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/nemo-relay-cli/codex.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/basic-usage.mdx
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/cli/src/mcp_environment.rscrates/cli/src/configuration/types.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/session_tests.rscrates/cli/src/gateway/request.rscrates/cli/src/configuration/mod.rscrates/cli/src/gateway/mod.rscrates/cli/src/gateway/routes.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/cli/src/mcp_environment.rscrates/cli/src/configuration/types.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/session_tests.rscrates/cli/src/gateway/request.rscrates/cli/src/configuration/mod.rscrates/cli/src/gateway/mod.rscrates/cli/src/gateway/routes.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/cli/src/mcp_environment.rscrates/cli/src/configuration/types.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/session_tests.rscrates/cli/src/gateway/request.rscrates/cli/src/configuration/mod.rscrates/cli/src/gateway/mod.rscrates/cli/src/gateway/routes.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/cli/src/mcp_environment.rscrates/cli/src/configuration/types.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/session_tests.rscrates/cli/src/gateway/request.rscrates/cli/src/configuration/mod.rscrates/cli/src/gateway/mod.rscrates/cli/src/gateway/routes.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/cli/src/mcp_environment.rscrates/cli/src/configuration/types.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/session_tests.rscrates/cli/src/gateway/request.rscrates/cli/src/configuration/mod.rscrates/cli/src/gateway/mod.rscrates/cli/src/gateway/routes.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.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/cli/src/mcp_environment.rscrates/cli/src/configuration/types.rscrates/cli/src/gateway/request.rscrates/cli/src/configuration/mod.rscrates/cli/src/gateway/mod.rscrates/cli/src/gateway/routes.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/cli/src/mcp_environment.rscrates/cli/src/configuration/types.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/session_tests.rscrates/cli/src/gateway/request.rscrates/cli/src/configuration/mod.rscrates/cli/src/gateway/mod.rscrates/cli/src/gateway/routes.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/session_tests.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rs
🔇 Additional comments (23)
crates/cli/src/mcp_environment.rs (1)
41-46: LGTM!docs/nemo-relay-cli/basic-usage.mdx (3)
145-165: LGTM!
271-276: LGTM!
166-185: 🔒 Security & PrivacyConfigured auth headers are already target-scoped. The gateway picks
openai_auth_header/anthropic_auth_headerfrom the effective target route, and explicit target dispatch removes source credentials before injecting target-specific auth.> Likely an incorrect or invalid review comment.docs/nemo-relay-cli/claude-code.mdx (1)
73-81: LGTM!docs/nemo-relay-cli/codex.mdx (1)
120-121: LGTM!integrations/coding-agents/codex/.mcp.json (1)
39-44: LGTM!crates/cli/src/configuration/mod.rs (2)
1508-1516: Already resolved: trimmed value is returned.The prior question about missing trimming on the returned value no longer applies —
valueis trimmed on line 1509 and the trimmed value is what's returned on line 1515.
17-17: LGTM!Also applies to: 66-71, 241-243, 1158-1167
crates/cli/src/configuration/types.rs (1)
25-27: LGTM!Also applies to: 117-119
crates/cli/src/gateway/routes.rs (1)
17-46: LGTM!Also applies to: 122-131, 160-173, 194-206, 225-240, 257-270
crates/cli/src/gateway/request.rs (1)
24-87: LGTM!crates/cli/src/gateway/mod.rs (3)
119-140: LGTM!Also applies to: 243-318, 428-492, 961-984
889-904: LGTM!Also applies to: 908-956
1097-1152: LGTM!crates/cli/tests/coverage/shared/config_tests.rs (2)
33-104: LGTM!Also applies to: 143-164
443-486: LGTM! The previously-flagged invalid-TOML fixture issue is fixed — the multi-line backslash-continuation syntax parses to a valid TOML value containing an embedded newline, sovalidate_auth_headeris exercised as intended, and the Anthropic counterpart was added.Also applies to: 501-602, 2054-2096
crates/cli/tests/coverage/shared/gateway_tests.rs (4)
12-113: LGTM!
214-216: LGTM! These are mechanical updates matching upstream API changes (ProviderRequestAuthorization,target_routerename, extrainject_provider_auth_with_envargument,transparent_proxy_credentialfield).Also applies to: 246-248, 410-410, 577-577, 612-612, 725-728, 919-922, 953-956, 965-965, 1170-1179, 1200-1200, 1301-1365, 1482-1484, 1495-1495, 1520-1522, 1533-1533
1043-1159: LGTM!Also applies to: 1212-1252
1254-1298: 🔒 Security & PrivacyNo extra env-var case needed here.
strip_replaceable_agent_auth_headersalready computes the replacement decision fromconfigured_auth_header, and a configuredopenai_auth_headeris enough to replace the JWT even whenOPENAI_API_KEYis unset.> Likely an incorrect or invalid review comment.crates/cli/tests/coverage/shared/server_tests.rs (1)
160-171: LGTM!Also applies to: 391-399, 423-495, 498-507, 563-575, 2409-2492, 2998-3017
crates/cli/tests/coverage/shared/session_tests.rs (1)
910-921: LGTM!Also applies to: 2159-2174, 2427-2446, 2512-2527, 2596-2611, 3907-3918, 3985-3997, 4059-4070, 4117-4128, 4602-4613, 4731-4742, 4830-4841, 4947-4958, 5042-5053, 6873-6885, 6888-6900
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/cli/src/gateway/mod.rs (1)
693-736: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winConfigured-auth selection is coupled to the post-dispatch target route — currently gated safely, but fragile.
configured_auth_headeris computed fromeffective.target_route(line 716), yet it's only ever consumed whencredential_policy == SourceOrEnvironment, which today is guaranteed to coincide withtarget_route == forwarding.source_route. The leak the previous review flagged (configured/global credentials following a plugin-selected target) doesn't currently manifest becauseTargetCredentialPolicy::ExplicitTargetdisables the whole injection path — but that safety is incidental to howeffective_dispatch_requesthappens to set both fields together, not an explicit invariant. Please confirm this fully satisfies the earlier concern, and consider removing the coupling by deriving the header fromforwarding.source_route(the field it's actually safe to use) so the "configured creds are Relay-default-upstream-only" guarantee doesn't depend on two independently-evolving pieces of logic staying in sync.🔒️ Suggested fix: decouple from target_route
- let configured_auth_header = forwarding.configured_auth_header(effective.target_route); + // Configured credentials are scoped to Relay's original/default upstream only; explicit + // plugin targets must supply their own auth via target-specific binding headers. + let configured_auth_header = forwarding.configured_auth_header(forwarding.source_route);🤖 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/src/gateway/mod.rs` around lines 693 - 736, Update forward_upstream_request so configured_auth_header is derived from forwarding.source_route rather than effective.target_route. Keep the existing credential-policy gating and inject_provider_auth flow unchanged, ensuring configured credentials remain limited to the relay’s source route independently of effective_dispatch_request’s target-route and policy coupling.
🤖 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/src/configuration/mod.rs`:
- Around line 1189-1213: Update apply_file_upstream_config and the corresponding
apply_env_config logic so that when a configuration layer changes an upstream
endpoint without providing that layer’s auth header, it clears the inherited
gateway auth header for that provider. Preserve supplied auth headers after
validation, and apply the same endpoint/auth coupling independently for OpenAI
and Anthropic.
In `@crates/cli/tests/coverage/shared/gateway_tests.rs`:
- Around line 428-519: Add a regression test alongside
explicit_keyless_target_drops_source_credentials and
cross_protocol_target_uses_only_binding_owned_authentication covering
openai_chat/openai_responses dispatch to a different host with GatewayConfig
configured authentication. Verify ProviderForwarding::configured_auth_header
does not forward the configured secret when the target is not explicitly bound,
while preserving forwarding only for an explicitly bound target.
In `@docs/nemo-relay-cli/basic-usage.mdx`:
- Around line 166-185: Update the authentication precedence documentation around
the upstream authorization description to explicitly call out the Codex ChatGPT
OAuth exception. State that when an inbound OAuth-style OpenAI Authorization is
present and a replacement OpenAI credential is configured, Relay strips the
OAuth token and forwards using the configured credential to the ChatGPT backend,
including credentials configured in the file or through the OpenAI auth
environment variable.
---
Outside diff comments:
In `@crates/cli/src/gateway/mod.rs`:
- Around line 693-736: Update forward_upstream_request so configured_auth_header
is derived from forwarding.source_route rather than effective.target_route. Keep
the existing credential-policy gating and inject_provider_auth flow unchanged,
ensuring configured credentials remain limited to the relay’s source route
independently of effective_dispatch_request’s target-route and policy coupling.
🪄 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: 50626514-6ce9-46da-b995-acc57354e5ed
📒 Files selected for processing (14)
crates/cli/src/configuration/mod.rscrates/cli/src/configuration/types.rscrates/cli/src/gateway/mod.rscrates/cli/src/gateway/request.rscrates/cli/src/gateway/routes.rscrates/cli/src/mcp_environment.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/session_tests.rsdocs/nemo-relay-cli/basic-usage.mdxdocs/nemo-relay-cli/claude-code.mdxdocs/nemo-relay-cli/codex.mdxintegrations/coding-agents/codex/.mcp.json
📜 Review details
🔇 Additional comments (23)
crates/cli/src/mcp_environment.rs (1)
41-46: LGTM!docs/nemo-relay-cli/basic-usage.mdx (3)
145-165: LGTM!
271-276: LGTM!
166-185: 🔒 Security & PrivacyConfigured auth headers are already target-scoped. The gateway picks
openai_auth_header/anthropic_auth_headerfrom the effective target route, and explicit target dispatch removes source credentials before injecting target-specific auth.> Likely an incorrect or invalid review comment.docs/nemo-relay-cli/claude-code.mdx (1)
73-81: LGTM!docs/nemo-relay-cli/codex.mdx (1)
120-121: LGTM!integrations/coding-agents/codex/.mcp.json (1)
39-44: LGTM!crates/cli/src/configuration/mod.rs (2)
1508-1516: Already resolved: trimmed value is returned.The prior question about missing trimming on the returned value no longer applies —
valueis trimmed on line 1509 and the trimmed value is what's returned on line 1515.
17-17: LGTM!Also applies to: 66-71, 241-243, 1158-1167
crates/cli/src/configuration/types.rs (1)
25-27: LGTM!Also applies to: 117-119
crates/cli/src/gateway/routes.rs (1)
17-46: LGTM!Also applies to: 122-131, 160-173, 194-206, 225-240, 257-270
crates/cli/src/gateway/request.rs (1)
24-87: LGTM!crates/cli/src/gateway/mod.rs (3)
119-140: LGTM!Also applies to: 243-318, 428-492, 961-984
889-904: LGTM!Also applies to: 908-956
1097-1152: LGTM!crates/cli/tests/coverage/shared/config_tests.rs (2)
33-104: LGTM!Also applies to: 143-164
443-486: LGTM! The previously-flagged invalid-TOML fixture issue is fixed — the multi-line backslash-continuation syntax parses to a valid TOML value containing an embedded newline, sovalidate_auth_headeris exercised as intended, and the Anthropic counterpart was added.Also applies to: 501-602, 2054-2096
crates/cli/tests/coverage/shared/gateway_tests.rs (4)
12-113: LGTM!
214-216: LGTM! These are mechanical updates matching upstream API changes (ProviderRequestAuthorization,target_routerename, extrainject_provider_auth_with_envargument,transparent_proxy_credentialfield).Also applies to: 246-248, 410-410, 577-577, 612-612, 725-728, 919-922, 953-956, 965-965, 1170-1179, 1200-1200, 1301-1365, 1482-1484, 1495-1495, 1520-1522, 1533-1533
1043-1159: LGTM!Also applies to: 1212-1252
1254-1298: 🔒 Security & PrivacyNo extra env-var case needed here.
strip_replaceable_agent_auth_headersalready computes the replacement decision fromconfigured_auth_header, and a configuredopenai_auth_headeris enough to replace the JWT even whenOPENAI_API_KEYis unset.> Likely an incorrect or invalid review comment.crates/cli/tests/coverage/shared/server_tests.rs (1)
160-171: LGTM!Also applies to: 391-399, 423-495, 498-507, 563-575, 2409-2492, 2998-3017
crates/cli/tests/coverage/shared/session_tests.rs (1)
910-921: LGTM!Also applies to: 2159-2174, 2427-2446, 2512-2527, 2596-2611, 3907-3918, 3985-3997, 4059-4070, 4117-4128, 4602-4613, 4731-4742, 4830-4841, 4947-4958, 5042-5053, 6873-6885, 6888-6900
🛑 Comments failed to post (3)
crates/cli/src/configuration/mod.rs (1)
1189-1213: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Endpoint and credential still resolve independently across precedence layers.
apply_file_upstream_configsetsopenai_base_urlandopenai_auth_headerfrom independentOptionfields, andapply_env_configruns afterward doing the same forNEMO_RELAY_OPENAI_BASE_URL/NEMO_RELAY_OPENAI_AUTH_HEADER. If a higher-precedence layer (env) overrides only the base URL, the auth header configured at a lower-precedence layer (file) is still forwarded to the new host, since nothing clearsgateway.openai_auth_headerwhen only the URL changes. This is the same gap flagged on a previous revision and still applies to the current code.🔒️ One approach: clear inherited auth when a layer changes the endpoint without also supplying auth
fn apply_file_upstream_config( gateway: &mut GatewayConfig, upstream: Option<FileUpstreamConfig>, ) -> Result<(), CliError> { let Some(upstream) = upstream else { return Ok(()); }; if let Some(value) = upstream.openai_base_url { gateway.openai_base_url = value; + if upstream.openai_auth_header.is_none() { + gateway.openai_auth_header = None; + } } if let Some(value) = upstream.openai_auth_header { gateway.openai_auth_header = Some(validate_auth_header("upstream.openai_auth_header", value)?); } // ...same pattern for anthropic, and mirror in apply_env_configAlso applies to: 1473-1496
🤖 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/src/configuration/mod.rs` around lines 1189 - 1213, Update apply_file_upstream_config and the corresponding apply_env_config logic so that when a configuration layer changes an upstream endpoint without providing that layer’s auth header, it clears the inherited gateway auth header for that provider. Preserve supplied auth headers after validation, and apply the same endpoint/auth coupling independently for OpenAI and Anthropic.crates/cli/tests/coverage/shared/gateway_tests.rs (1)
428-519: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
rg -n 'configured_auth_header|effective_dispatch_request|TargetCredentialPolicy' crates/cli/src/gateway/routes.rs crates/cli/src/gateway/request.rs -A5 -B5Repository: NVIDIA/NeMo-Relay
Length of output: 6949
🏁 Script executed:
#!/bin/bash set -euo pipefail ast-grep outline crates/cli/src/gateway/routes.rs --view expanded ast-grep outline crates/cli/src/gateway/request.rs --view expanded printf '\n--- routes.rs relevant section ---\n' sed -n '150,290p' crates/cli/src/gateway/routes.rs printf '\n--- request.rs relevant section ---\n' sed -n '1,140p' crates/cli/src/gateway/request.rs printf '\n--- tests mentioning GatewayConfig / provider auth / dispatch ---\n' rg -n 'GatewayConfig|openai_auth_header|anthropic_auth_header|ProviderForwarding::new|inject_provider_auth_with_env|INTERNAL_DISPATCH_URL_HEADER|INTERNAL_DISPATCH_ROUTE_HEADER|configured_auth_header' crates/cli/tests crates/cli/src -A3 -B3Repository: NVIDIA/NeMo-Relay
Length of output: 50374
🏁 Script executed:
#!/bin/bash set -euo pipefail # Find any existing tests that cover same-provider plugin dispatch or configured auth forwarding. rg -n 'ProviderForwarding::new|inject_provider_auth_with_env|configured_auth_header|same-provider|plugin-selected|dispatch override|INTERNAL_DISPATCH_URL_HEADER|INTERNAL_DISPATCH_ROUTE_HEADER' \ crates/cli/tests/coverage/shared crates/cli/src/gateway -A4 -B4 printf '\n--- gateway_tests.rs relevant range ---\n' sed -n '380,560p' crates/cli/tests/coverage/shared/gateway_tests.rsRepository: NVIDIA/NeMo-Relay
Length of output: 41359
Add a same-provider dispatch regression for configured auth
The current tests cover keyless dispatch and a cross-protocol override, but not
GatewayConfig.openai_auth_header/anthropic_auth_headerflowing through a plugin-selected URL while keeping the sameProviderRoute.ProviderForwarding::configured_auth_headeris route-only, so this path can still forward a configured secret to an untrusted upstream. Add a regression that dispatchesopenai_chat/openai_responsesto a different host with config auth set and asserts the configured header is withheld unless the target is explicitly bound.🤖 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 428 - 519, Add a regression test alongside explicit_keyless_target_drops_source_credentials and cross_protocol_target_uses_only_binding_owned_authentication covering openai_chat/openai_responses dispatch to a different host with GatewayConfig configured authentication. Verify ProviderForwarding::configured_auth_header does not forward the configured secret when the target is not explicitly bound, while preserving forwarding only for an explicitly bound target.docs/nemo-relay-cli/basic-usage.mdx (1)
166-185: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail printf '\n## File excerpt\n' sed -n '150,210p' docs/nemo-relay-cli/basic-usage.mdx | cat -n printf '\n## Search for relevant auth terms\n' rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' \ -e 'Codex OAuth|openai_auth_header|anthropic_auth_header|Authorization|api-key|anthropic-api-key|NEMO_RELAY_OPENAI_AUTH_HEADER|NEMO_RELAY_ANTHROPIC_AUTH_HEADER|replace.*credential|replacement authentication|precedence' \ docs src . 2>/dev/null | sed -n '1,220p'Repository: NVIDIA/NeMo-Relay
Length of output: 29096
🏁 Script executed:
set -euo pipefail printf '\n## provider_auth.rs slice\n' sed -n '1,260p' crates/cli/src/provider_auth.rs | cat -n printf '\n## gateway/mod.rs slice\n' sed -n '860,980p' crates/cli/src/gateway/mod.rs | cat -n printf '\n## gateway/routes.rs slice\n' sed -n '1,220p' crates/cli/src/gateway/routes.rs | cat -n printf '\n## config tests slice around auth env/file precedence\n' sed -n '430,610p' crates/cli/tests/coverage/shared/config_tests.rs | cat -n printf '\n## search Codex OAuth references\n' rg -n --hidden --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' \ -e 'Codex OAuth|OAuth|codex.*oauth|oauth.*codex|replacement authentication|replacement auth|JWT-shaped|Bearer eyJ|replace.*Bearer|authorization value' \ crates docs . 2>/dev/null | sed -n '1,220p'Repository: NVIDIA/NeMo-Relay
Length of output: 34393
🏁 Script executed:
set -euo pipefail printf '\n## codex alignment slice\n' sed -n '1,240p' crates/cli/src/agents/codex/alignment.rs | cat -n printf '\n## codex alignment tests slice\n' sed -n '1,260p' crates/cli/tests/coverage/agents/alignment_codex_tests.rs | cat -n printf '\n## gateway tests slice around OAuth JWT stripping\n' sed -n '940,1010p' crates/cli/tests/coverage/shared/gateway_tests.rs | cat -nRepository: NVIDIA/NeMo-Relay
Length of output: 24882
🏁 Script executed:
set -euo pipefail rg -n "has_openai_replacement_auth|strip_replaceable_agent_auth_headers_with_openai_key_state" crates/cli/src/gateway/mod.rs crates/cli/src/gateway -n sed -n '980,1060p' crates/cli/src/gateway/mod.rs | cat -nRepository: NVIDIA/NeMo-Relay
Length of output: 4221
🏁 Script executed:
set -euo pipefail sed -n '236,290p' crates/cli/src/gateway/routes.rs | cat -nRepository: NVIDIA/NeMo-Relay
Length of output: 2230
Call out the Codex OAuth exception in the auth precedence docs. OpenAI routes preserve inbound provider auth by default, but when a Codex ChatGPT OAuth-style
Authorizationis present and a replacement OpenAI credential is configured, Relay strips that token and forwards to the ChatGPT backend instead. Mention that both file-configured and env-configured OpenAI auth participate in that replacement path.🤖 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/nemo-relay-cli/basic-usage.mdx` around lines 166 - 185, Update the authentication precedence documentation around the upstream authorization description to explicitly call out the Codex ChatGPT OAuth exception. State that when an inbound OAuth-style OpenAI Authorization is present and a replacement OpenAI credential is configured, Relay strips the OAuth token and forwards using the configured credential to the ChatGPT backend, including credentials configured in the file or through the OpenAI auth environment variable.Source: Path instructions
mnajafian-nv
left a comment
mnajafian-nv
left a comment
There was a problem hiding this comment.
LGTM, feedback is addressed.
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/cli/tests/coverage/shared/gateway_tests.rs`:
- Around line 126-128: Update all six prepare_gateway_request calls in the
gateway tests to pass a ProviderRequestAuthorization value instead of the bare
true boolean. Reuse the existing authorization struct construction shown in the
nearby tests, preferably by introducing or using a shared test helper, while
preserving each test’s current request behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 9c4f65ac-533e-4919-a75f-79817170262f
📒 Files selected for processing (3)
crates/cli/src/gateway/mod.rscrates/cli/src/gateway/request.rscrates/cli/tests/coverage/shared/gateway_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*.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/cli/src/gateway/request.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/src/gateway/mod.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/cli/src/gateway/request.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/src/gateway/mod.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/cli/src/gateway/request.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/src/gateway/mod.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/cli/src/gateway/request.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/src/gateway/mod.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/cli/src/gateway/request.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/src/gateway/mod.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/cli/src/gateway/request.rscrates/cli/src/gateway/mod.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/cli/src/gateway/request.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/src/gateway/mod.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/cli/src/gateway/request.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/src/gateway/mod.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/cli/tests/coverage/shared/gateway_tests.rs
🔇 Additional comments (5)
crates/cli/tests/coverage/shared/gateway_tests.rs (2)
1414-1457: Exercise configured OpenAI auth throughprepare_gateway_request.This test calls the normalization and injection helpers directly and hard-codes the permission flag to
true; it will pass even if the real preparation path fails to recognize configured auth whenOPENAI_API_KEYis absent. Build the request throughprepare_gateway_requestwith configured auth and no environment key, then assert the JWT is removed and the configured header is forwarded.As per path instructions, “Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.”
Source: Path instructions
350-356: LGTM!Also applies to: 382-390, 450-468, 668-675, 1326-1363, 1371-1411, 1460-1521, 1638-1644, 1675-1682
crates/cli/src/gateway/request.rs (1)
6-11: LGTM!Also applies to: 51-57, 68-75, 97-150
crates/cli/src/gateway/mod.rs (2)
20-22: LGTM!Also applies to: 135-135, 257-261, 441-445, 715-736, 790-798, 903-940, 981-981, 1120-1141, 1154-1154
849-851: 🗄️ Data Integrity & IntegrationNo
Content-Lengthcleanup needed here.
should_forward_request_headeralready filters outContent-Length, andreqwestderives the outgoing length from.body(...), so removingContent-Encodingis sufficient.> Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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/cli/tests/coverage/shared/gateway_tests.rs`:
- Around line 126-128: Update all six prepare_gateway_request calls in the
gateway tests to pass a ProviderRequestAuthorization value instead of the bare
true boolean. Reuse the existing authorization struct construction shown in the
nearby tests, preferably by introducing or using a shared test helper, while
preserving each test’s current request behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 9c4f65ac-533e-4919-a75f-79817170262f
📒 Files selected for processing (3)
crates/cli/src/gateway/mod.rscrates/cli/src/gateway/request.rscrates/cli/tests/coverage/shared/gateway_tests.rs
📜 Review details
🔇 Additional comments (5)
crates/cli/tests/coverage/shared/gateway_tests.rs (2)
1414-1457: Exercise configured OpenAI auth throughprepare_gateway_request.This test calls the normalization and injection helpers directly and hard-codes the permission flag to
true; it will pass even if the real preparation path fails to recognize configured auth whenOPENAI_API_KEYis absent. Build the request throughprepare_gateway_requestwith configured auth and no environment key, then assert the JWT is removed and the configured header is forwarded.As per path instructions, “Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.”
Source: Path instructions
350-356: LGTM!Also applies to: 382-390, 450-468, 668-675, 1326-1363, 1371-1411, 1460-1521, 1638-1644, 1675-1682
crates/cli/src/gateway/request.rs (1)
6-11: LGTM!Also applies to: 51-57, 68-75, 97-150
crates/cli/src/gateway/mod.rs (2)
20-22: LGTM!Also applies to: 135-135, 257-261, 441-445, 715-736, 790-798, 903-940, 981-981, 1120-1141, 1154-1154
849-851: 🗄️ Data Integrity & IntegrationNo
Content-Lengthcleanup needed here.
should_forward_request_headeralready filters outContent-Length, andreqwestderives the outgoing length from.body(...), so removingContent-Encodingis sufficient.> Likely an incorrect or invalid review comment.
🛑 Comments failed to post (1)
crates/cli/tests/coverage/shared/gateway_tests.rs (1)
126-128: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Pass
ProviderRequestAuthorization, nottrue.
prepare_gateway_requestrequiresProviderRequestAuthorizationat Line [41] ofcrates/cli/src/gateway/request.rs, but all six new calls pass a bare boolean. These tests will not compile. Use the struct construction already shown at Lines [1379]-[1383], preferably through a shared test helper.Proposed test setup
- let prepared = prepare_gateway_request(&config, request, true) + let prepared = prepare_gateway_request( + &config, + request, + crate::provider_auth::ProviderRequestAuthorization { + source_credential: + crate::provider_auth::SourceCredentialDisposition::Absent, + allow_environment_provider_auth: true, + }, + )Also applies to: 157-159, 200-202, 211-213, 222-224, 241-243
🤖 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 126 - 128, Update all six prepare_gateway_request calls in the gateway tests to pass a ProviderRequestAuthorization value instead of the bare true boolean. Reuse the existing authorization struct construction shown in the nearby tests, preferably by introducing or using a shared test helper, while preserving each test’s current request behavior.
Signed-off-by: Will Killian <wkillian@nvidia.com>
Signed-off-by: Will Killian <wkillian@nvidia.com>
coderabbitai
Bot
added
the
DO NOT MERGE
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/cli/src/configuration/mod.rs (1)
1491-1515: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winOnly clear inherited credentials when the environment changes the endpoint.
Unlike
merge_gateway_config_toml, this clears file credentials even when the environment base URL equals the configured URL. That can silently remove valid authentication. Compare the URLs before clearing, for both providers, and cover equal-URL inheritance in tests.Proposed fix
if let Ok(value) = std::env::var("NEMO_RELAY_OPENAI_BASE_URL") { + let endpoint_changed = config.openai_base_url != value; config.openai_base_url = value; - if openai_auth_header.is_none() { + if endpoint_changed && openai_auth_header.is_none() { config.openai_auth_header = None; } } if let Ok(value) = std::env::var("NEMO_RELAY_ANTHROPIC_BASE_URL") { + let endpoint_changed = config.anthropic_base_url != value; config.anthropic_base_url = value; - if anthropic_auth_header.is_none() { + if endpoint_changed && anthropic_auth_header.is_none() { config.anthropic_auth_header = None; } }As per path instructions, tests should cover the behavior promised by the changed API surface.
🤖 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/src/configuration/mod.rs` around lines 1491 - 1515, Update the environment override logic in the configuration-loading function so inherited OpenAI and Anthropic auth headers are cleared only when the corresponding environment base URL differs from the configured URL. Preserve credentials when the URLs are equal, while retaining current override and validation behavior for changed endpoints; add tests covering equal-URL credential inheritance for both providers.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/cli/src/configuration/mod.rs`:
- Around line 1491-1515: Update the environment override logic in the
configuration-loading function so inherited OpenAI and Anthropic auth headers
are cleared only when the corresponding environment base URL differs from the
configured URL. Preserve credentials when the URLs are equal, while retaining
current override and validation behavior for changed endpoints; add tests
covering equal-URL credential inheritance for both providers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: d131f8c0-b306-476e-89a1-cda948f9a01c
📒 Files selected for processing (4)
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rsdocs/nemo-relay-cli/basic-usage.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 (15)
**/*.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/nemo-relay-cli/basic-usage.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/nemo-relay-cli/basic-usage.mdx
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
docs/nemo-relay-cli/basic-usage.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/nemo-relay-cli/basic-usage.mdx
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
docs/nemo-relay-cli/basic-usage.mdxcrates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rs
docs/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If documentation examples or commands under
docs/change, run the targeted docs checks appropriate to the change.
Files:
docs/nemo-relay-cli/basic-usage.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/nemo-relay-cli/basic-usage.mdx
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.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/cli/src/configuration/mod.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rs
🔇 Additional comments (6)
crates/cli/tests/coverage/shared/gateway_tests.rs (1)
22-28: LGTM!Also applies to: 133-139, 168-174, 215-237, 256-262, 372-374, 404-406, 688-692, 1348-1357, 1374-1378, 1390-1477, 1487-1499, 1512-1521, 1530-1539, 1657-1662, 1695-1700
crates/cli/src/configuration/mod.rs (1)
17-17: LGTM!Also applies to: 68-70, 241-243, 989-989, 1163-1163, 1189-1224, 1528-1537, 1570-1591
crates/cli/tests/coverage/shared/config_tests.rs (1)
33-164: LGTM!Also applies to: 559-632
docs/nemo-relay-cli/basic-usage.mdx (3)
280-282: LGTM!
166-174: 🔒 Security & PrivacyRoute-scoped auth is correct here.
openai_auth_header/anthropic_auth_headerare selected from the effective provider route, not a plugin target, so there’s no cross-route credential forwarding issue in this path. The Codex OAuth replacement language also matches the alignment logic.> Likely an incorrect or invalid review comment.
187-191: 🔒 Security & PrivacyDrop this concern.
NEMO_RELAY_OPENAI_AUTH_HEADERalready resolves intoopenai_auth_header, and the gateway uses that field for the ChatGPT OAuth replacement path.> Likely an incorrect or invalid review comment.
Signed-off-by: Will Killian <wkillian@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/cli/src/configuration/mod.rs (1)
1491-1516: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEnv-var endpoint override always clears the auth header, even when the URL is unchanged.
merge_gateway_config_toml(lines 1582-1587) only clears an inherited auth header when the base URL actually changes between layers, matching the documented invariant at lines 1570-1572.apply_env_configdoesn't apply the same check: any presence ofNEMO_RELAY_OPENAI_BASE_URL/NEMO_RELAY_ANTHROPIC_BASE_URLwithout the matching*_AUTH_HEADERenv var unconditionally wipes a previously configured (e.g., file-based) auth header — even if the env value is identical to what's already resolved. This silently breaks outbound auth in setups that always inject the base-URL env var (e.g. templated container envs) regardless of whether it changed.🐛 Proposed fix to mirror the TOML-merge invariant
let openai_auth_header = std::env::var("NEMO_RELAY_OPENAI_AUTH_HEADER").ok(); if let Ok(value) = std::env::var("NEMO_RELAY_OPENAI_BASE_URL") { + let endpoint_changed = value != config.openai_base_url; config.openai_base_url = value; - if openai_auth_header.is_none() { + if endpoint_changed && openai_auth_header.is_none() { config.openai_auth_header = None; } } ... let anthropic_auth_header = std::env::var("NEMO_RELAY_ANTHROPIC_AUTH_HEADER").ok(); if let Ok(value) = std::env::var("NEMO_RELAY_ANTHROPIC_BASE_URL") { + let endpoint_changed = value != config.anthropic_base_url; config.anthropic_base_url = value; - if anthropic_auth_header.is_none() { + if endpoint_changed && anthropic_auth_header.is_none() { config.anthropic_auth_header = 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/src/configuration/mod.rs` around lines 1491 - 1516, Update apply_env_config so NEMO_RELAY_OPENAI_BASE_URL and NEMO_RELAY_ANTHROPIC_BASE_URL clear their inherited auth headers only when the supplied URL differs from the currently resolved config URL, matching merge_gateway_config_toml. Preserve existing auth headers when the URL is unchanged and no corresponding auth-header environment variable is provided.
🤖 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/cli/src/configuration/mod.rs`:
- Around line 1491-1516: Update apply_env_config so NEMO_RELAY_OPENAI_BASE_URL
and NEMO_RELAY_ANTHROPIC_BASE_URL clear their inherited auth headers only when
the supplied URL differs from the currently resolved config URL, matching
merge_gateway_config_toml. Preserve existing auth headers when the URL is
unchanged and no corresponding auth-header environment variable is provided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: bcc5cc00-fb74-430b-9bec-5b685d2e9a8c
📒 Files selected for processing (3)
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/gateway_tests.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (9)
**/*.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/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/config_tests.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/cli/src/configuration/mod.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/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/config_tests.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
crates/cli/src/configuration/mod.rscrates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/config_tests.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/cli/tests/coverage/shared/gateway_tests.rscrates/cli/tests/coverage/shared/config_tests.rs
🔇 Additional comments (9)
crates/cli/tests/coverage/shared/gateway_tests.rs (1)
22-28: LGTM!Also applies to: 133-139, 168-174, 215-237, 256-262, 372-374, 404-406, 688-692, 1348-1357, 1374-1378, 1390-1477, 1487-1499, 1512-1521, 1530-1539, 1657-1662, 1695-1700
crates/cli/src/configuration/mod.rs (4)
67-70: LGTM!Also applies to: 1189-1225
1528-1536: LGTM! UsingHeaderValue::from_strcorrectly rejects CR/LF and other illegal header bytes, guarding against header injection via configured values.
989-989: LGTM! The endpoint-change check correctly scopes credential clearing to real overrides rather than unrelated re-declarations.Also applies to: 1570-1592
241-243: 🔒 Security & PrivacyNo plaintext secret persistence here. The auth headers are only used as HMAC input; the persisted value is the
hmac-sha256:...fingerprint, not the JSON document itself.> Likely an incorrect or invalid review comment.crates/cli/tests/coverage/shared/config_tests.rs (4)
33-46: LGTM!Also applies to: 48-121, 123-165
559-632: LGTM! Precedence and clearing behavior are asserted for both file-layer and environment-layer overrides.
680-701: LGTM!
2153-2194: LGTM!
|
/merge |
#### Overview Documents provider routing and credential-override behavior introduced by #440, alongside the updated Claude Code integration context. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Documents route mapping and precedence for inbound credentials, configured credentials, and API-key overrides. - Clarifies the environment endpoint override rule, managed-mode proof, and the JWT/AT exception. - Updates the Claude Code guide for the shared gateway and links it to the provider reference. - Contains no implementation changes. - Breaking changes: none. Validation: - `just docs` - `just docs-linkcheck` - targeted `uv run pre-commit run --files ...` #### Where should the reviewer start? Start with `docs/nemo-relay-cli/basic-usage.mdx`. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to: #440 ## Summary by CodeRabbit * **Documentation** * Clarified transparent and persistent CLI workflows, including process ownership, terminal behavior, and signal forwarding. * Expanded guidance for Claude Code, Codex, and Hermes setup, isolation, configuration, and gateway integration. * Documented provider routing, credential precedence, authentication headers, MCP gateways, and common environment variables. * Added details on correlation metadata, startup probes, hook forwarding, failure handling, and flag-to-header mappings. * Updated Claude Code configuration examples, upstream settings, observability guidance, ATIF exports, and troubleshooting steps. Authors: - Will Killian (https://github.com/willkill07) Approvers: - https://github.com/lvojtku URL: #456
3 participants
Overview
Adds provider-specific custom authentication for OpenAI- and Anthropic-compatible CLI gateway routes and documents endpoint-based upstream selection.
This is intentionally an opt-in escape hatch, not a replacement for standard provider API keys. It is needed when Relay forwards to an enterprise gateway, proxy, or compatible endpoint that requires a complete nonstandard
Authorizationvalue (for example,Basic ...), or when a local client credential must be replaced before forwarding upstream. Ordinary OpenAI and Anthropic API-key deployments continue to useOPENAI_API_KEYandANTHROPIC_API_KEY.Details
openai_auth_headerandanthropic_auth_headerunder[upstream], with environment overrides, validation, managed-gateway fingerprinting, and MCP environment forwarding.Authorizationvalue, then falls back to the existing provider API-key injection.openai_base_url, while Messages usesanthropic_base_url, independent of the compatible client.cargo fmt --all,just test-rust,cargo clippy --workspace --all-targets -- -D warnings,just docs, anduv run pre-commit run --all-files. The docs build passed with the remote FDR redirect comparison skipped after an HTTP 403 warning.Where should the reviewer start?
Start with
crates/cli/src/gateway/mod.rsfor authentication precedence and managed-gateway enforcement, then reviewcrates/cli/src/configuration/mod.rsfor resolution and validation.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
New Features
Bug Fixes