feat: add OpenRouter app attribution headers - #264
Conversation
Add extra_headers field to ProviderConfig and populate it for OpenRouter with HTTP-Referer, X-OpenRouter-Title, and X-OpenRouter-Categories headers per OpenRouter's app attribution spec. Headers are applied in call_openai() for every request.
- Make OpenRouter header assertions order-independent (M-3) - Verify non-OpenRouter providers have empty extra_headers (M-4) - Test explicit TOML provider config path injects headers (I-1)
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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)
src/config.rs (1)
3993-4010:⚠️ Potential issue | 🟠 MajorNormalize provider ID before checking for OpenRouter attribution headers.
At Line 4005,
provider_idis compared before normalization, but the key is normalized at Line 3998. Mixed-case keys like[llm.provider.OpenRouter]will be stored asopenrouterwithout attribution headers.🔧 Proposed fix
.map(|(provider_id, config)| { let api_key = resolve_env_value(&config.api_key).ok_or_else(|| { anyhow::anyhow!("failed to resolve API key for provider '{}'", provider_id) })?; + let normalized_provider_id = provider_id.to_lowercase(); + let extra_headers = if normalized_provider_id == "openrouter" { + openrouter_extra_headers() + } else { + vec![] + }; Ok(( - provider_id.to_lowercase(), + normalized_provider_id, ProviderConfig { api_type: config.api_type, base_url: config.base_url, api_key, name: config.name, use_bearer_auth: false, - extra_headers: if provider_id == "openrouter" { - openrouter_extra_headers() - } else { - vec![] - }, + extra_headers, }, )) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.rs` around lines 3993 - 4010, The code normalizes provider_id when inserting the ProviderConfig but checks the original provider_id for OpenRouter (so mixed-case keys like "OpenRouter" miss extra headers); update the closure so the check uses the normalized id (e.g., compute let normalized_id = provider_id.to_lowercase() once and use normalized_id both as the map key and in the conditional that decides extra_headers), ensuring openrouter_extra_headers() is applied when normalized_id == "openrouter" in the ProviderConfig construction.
🧹 Nitpick comments (3)
src/llm/model.rs (1)
126-134: Consider propagatingextra_headersto other OpenAI-compatible call paths.The
extra_headersfromprovider_configis only applied incall_openai()(line 590). Other OpenAI-compatible paths likeOpenAiChatCompletions(here) andcall_openai_compatible()don't useprovider_config.extra_headers.Currently this works for OpenRouter since it uses
ApiType::OpenAiCompletions, but if a provider usingOpenAiChatCompletionsneeded custom headers, they wouldn't be applied.♻️ Proposed fix to propagate extra_headers
ApiType::OpenAiChatCompletions => { let endpoint = format!( "{}/chat/completions", provider_config.base_url.trim_end_matches('/') ); let display_name = provider_config .name .as_deref() .unwrap_or("OpenAI-compatible provider"); self.call_openai_compatible_with_optional_auth( request, display_name, &endpoint, Some(provider_config.api_key.clone()), - &[], + &provider_config.extra_headers.iter().map(|(k, v)| (k.as_str(), v.as_str())).collect::<Vec<_>>(), ) .await }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/llm/model.rs` around lines 126 - 134, The OpenAI-compatible call paths aren't propagating provider_config.extra_headers; update the call sites for OpenAiChatCompletions (where call_openai_compatible_with_optional_auth is invoked) and any call_openai_compatible invocations to pass provider_config.extra_headers (not just the api_key), and modify the signatures of call_openai_compatible_with_optional_auth and call_openai_compatible to accept an Option<&[Header]> (or similar type already used for extra_headers) and forward those headers into the underlying HTTP/request builder logic (the same way call_openai() does), so custom headers from provider_config are applied for all OpenAI-compatible providers.src/config.rs (2)
6517-6523: Use descriptive names instead ofk/vin test header lookup closures.At Line 6521-Line 6522 and Line 6589-Line 6590, abbreviated variable names reduce readability and violate the repository naming rule.
As per coding guidelines, "Don't abbreviate variable names. Use
queuenotq,messagenotmsg,channelnotch. Common abbreviations likeconfigare fine."Also applies to: 6585-6591
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.rs` around lines 6517 - 6523, The closure find_header uses abbreviated closure bindings `k` and `v`, which reduces readability; update the closure in the find_header closure that iterates openrouter_provider.extra_headers to use descriptive names (e.g., header_name/header_value or key/value) instead of `k`/`v`, and apply the same renaming to the other test header lookup closure referenced in the diff so both .find(|(key, _)| key == name) and .map(|(_, value)| value.as_str()) read clearly.
3381-3721: Consolidate duplicated provider bootstrap logic inload_from_env().The provider registration pass is duplicated in this block. It works because of
or_insert_with, but this duplication is a drift trap (new fields likeextra_headersmust be updated in multiple places).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.rs` around lines 3381 - 3721, The review points out duplicated provider-registration logic inside load_from_env(): remove the repeated blocks and consolidate provider bootstrap into a single pass by centralizing creation logic (use the existing add_shorthand_provider helper and a small helper for explicit entries) so each provider is registered once into llm.providers via or_insert_with with ProviderConfig; update load_from_env() to call add_shorthand_provider for shorthand keys (kilo, zhipu, zai-coding-plan, opencode-*, etc.) and replace duplicated if let Some(...) { llm.providers.entry(...).or_insert_with(|| ProviderConfig { ... }) } blocks (e.g., for openai, openrouter, anthropic/minimax/minimax-cn, opencode-zen/opencode-go, moonshot, nvidia, fireworks, deepseek, gemini, groq, together, xai, mistral, ollama) with either add_shorthand_provider or a single helper function so extra_headers and use_bearer_auth are defined in only one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/config.rs`:
- Around line 3993-4010: The code normalizes provider_id when inserting the
ProviderConfig but checks the original provider_id for OpenRouter (so mixed-case
keys like "OpenRouter" miss extra headers); update the closure so the check uses
the normalized id (e.g., compute let normalized_id = provider_id.to_lowercase()
once and use normalized_id both as the map key and in the conditional that
decides extra_headers), ensuring openrouter_extra_headers() is applied when
normalized_id == "openrouter" in the ProviderConfig construction.
---
Nitpick comments:
In `@src/config.rs`:
- Around line 6517-6523: The closure find_header uses abbreviated closure
bindings `k` and `v`, which reduces readability; update the closure in the
find_header closure that iterates openrouter_provider.extra_headers to use
descriptive names (e.g., header_name/header_value or key/value) instead of
`k`/`v`, and apply the same renaming to the other test header lookup closure
referenced in the diff so both .find(|(key, _)| key == name) and .map(|(_,
value)| value.as_str()) read clearly.
- Around line 3381-3721: The review points out duplicated provider-registration
logic inside load_from_env(): remove the repeated blocks and consolidate
provider bootstrap into a single pass by centralizing creation logic (use the
existing add_shorthand_provider helper and a small helper for explicit entries)
so each provider is registered once into llm.providers via or_insert_with with
ProviderConfig; update load_from_env() to call add_shorthand_provider for
shorthand keys (kilo, zhipu, zai-coding-plan, opencode-*, etc.) and replace
duplicated if let Some(...) { llm.providers.entry(...).or_insert_with(||
ProviderConfig { ... }) } blocks (e.g., for openai, openrouter,
anthropic/minimax/minimax-cn, opencode-zen/opencode-go, moonshot, nvidia,
fireworks, deepseek, gemini, groq, together, xai, mistral, ollama) with either
add_shorthand_provider or a single helper function so extra_headers and
use_bearer_auth are defined in only one place.
In `@src/llm/model.rs`:
- Around line 126-134: The OpenAI-compatible call paths aren't propagating
provider_config.extra_headers; update the call sites for OpenAiChatCompletions
(where call_openai_compatible_with_optional_auth is invoked) and any
call_openai_compatible invocations to pass provider_config.extra_headers (not
just the api_key), and modify the signatures of
call_openai_compatible_with_optional_auth and call_openai_compatible to accept
an Option<&[Header]> (or similar type already used for extra_headers) and
forward those headers into the underlying HTTP/request builder logic (the same
way call_openai() does), so custom headers from provider_config are applied for
all OpenAI-compatible providers.
- Normalize provider ID before OpenRouter header check (case-insensitive) - Redact extra_headers values in Debug impl (log names only) - Propagate extra_headers to OpenAiChatCompletions call path - Use descriptive variable names in test closures
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/config.rs (1)
3404-3444: Consolidate duplicated provider bootstrap logic inload_from_env.The OpenRouter/shorthand insertion path is duplicated, which increases drift risk (future metadata/header updates can be applied in one block but missed in the other).
♻️ Refactor sketch
- if let Some(openrouter_key) = llm.openrouter_key.clone() { - llm.providers - .entry("openrouter".to_string()) - .or_insert_with(|| ProviderConfig { - api_type: ApiType::OpenAiCompletions, - base_url: OPENROUTER_PROVIDER_BASE_URL.to_string(), - api_key: openrouter_key, - name: None, - use_bearer_auth: false, - extra_headers: openrouter_extra_headers(), - }); - } - - add_shorthand_provider(/* kilo */); - add_shorthand_provider(/* zhipu */); - add_shorthand_provider(/* zai-coding-plan */); + // Keep a single provider-bootstrap pass for each provider. + if let Some(openrouter_key) = llm.openrouter_key.clone() { + llm.providers + .entry("openrouter".to_string()) + .or_insert_with(|| ProviderConfig { + api_type: ApiType::OpenAiCompletions, + base_url: OPENROUTER_PROVIDER_BASE_URL.to_string(), + api_key: openrouter_key, + name: None, + use_bearer_auth: false, + extra_headers: openrouter_extra_headers(), + }); + } + add_shorthand_provider(/* kilo */); + add_shorthand_provider(/* zhipu */); + add_shorthand_provider(/* zai-coding-plan */);Also applies to: 3504-3543
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.rs` around lines 3404 - 3444, The OpenRouter bootstrap block duplicates the shorthand provider insertion logic; consolidate by routing OpenRouter creation through the existing helper used for other providers (e.g., call add_shorthand_provider or create a small helper like add_provider_from_key) instead of manually constructing a ProviderConfig in the if let block; specifically update the code that references llm.openrouter_key and llm.providers to use the same path as add_shorthand_provider (or extract shared ProviderConfig defaults used by add_shorthand_provider), ensure OPENROUTER_PROVIDER_BASE_URL, ApiType::OpenAiCompletions, openrouter_extra_headers(), and use_bearer_auth=false are applied, and remove the duplicated manual ProviderConfig construction so metadata/headers are centralized and consistent with the other shorthand providers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/config.rs`:
- Around line 3404-3444: The OpenRouter bootstrap block duplicates the shorthand
provider insertion logic; consolidate by routing OpenRouter creation through the
existing helper used for other providers (e.g., call add_shorthand_provider or
create a small helper like add_provider_from_key) instead of manually
constructing a ProviderConfig in the if let block; specifically update the code
that references llm.openrouter_key and llm.providers to use the same path as
add_shorthand_provider (or extract shared ProviderConfig defaults used by
add_shorthand_provider), ensure OPENROUTER_PROVIDER_BASE_URL,
ApiType::OpenAiCompletions, openrouter_extra_headers(), and
use_bearer_auth=false are applied, and remove the duplicated manual
ProviderConfig construction so metadata/headers are centralized and consistent
with the other shorthand providers.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/config.rssrc/llm/model.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/llm/model.rs
pre-existing issue, not in scope for this PR |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/config.rs (2)
6585-6591: Optional: extract repeated header lookup closure in tests.
find_headeris duplicated across two tests; a small shared helper would trim repetition and keep future header assertion updates in one place.Also applies to: 6653-6659
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.rs` around lines 6585 - 6591, The tests duplicate the closure find_header; extract it into a shared helper function (e.g., fn find_header<'a>(headers: &'a [(String,String)], name: &str) -> Option<&'a str>) placed once in the tests module or a test helper mod and replace both inline closures with calls to that function; update references in the two test locations (the closures at find_header usages around lines 6585-6591 and 6653-6659) so future header-lookup changes are centralized.
3453-3644: Consolidate duplicated env-provider bootstrap blocks to reduce drift risk.The block starting at Line 3553 re-adds providers already inserted starting at Line 3453 (OpenRouter/Kilo/Zhipu/ZAI/OpenCode/MiniMax). It works today because of
or_insert_with, but this duplication is easy to desync when metadata changes again.♻️ Refactor sketch
- if let Some(openrouter_key) = llm.openrouter_key.clone() { - llm.providers - .entry("openrouter".to_string()) - .or_insert_with(|| ProviderConfig { - api_type: ApiType::OpenAiCompletions, - base_url: OPENROUTER_PROVIDER_BASE_URL.to_string(), - api_key: openrouter_key, - name: None, - use_bearer_auth: false, - extra_headers: openrouter_extra_headers(), - }); - } - - add_shorthand_provider(... "kilo" ...); - add_shorthand_provider(... "zhipu" ...); - add_shorthand_provider(... "zai-coding-plan" ...); - - if let Some(opencode_zen_key) = llm.opencode_zen_key.clone() { ... } - if let Some(opencode_go_key) = llm.opencode_go_key.clone() { ... } - if let Some(minimax_key) = llm.minimax_key.clone() { ... } - if let Some(minimax_cn_key) = llm.minimax_cn_key.clone() { ... } + // Keep a single provider-bootstrap pass in `load_from_env` + // and avoid re-inserting the same providers a second time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.rs` around lines 3453 - 3644, The code duplicates provider bootstrap logic — remove the repeated block (the second set of entries for "openrouter", "kilo", "zhipu", "zai-coding-plan", "opencode-zen", "opencode-go", "minimax", and "minimax-cn") and consolidate into a single initialization sequence that uses add_shorthand_provider and the existing entry(...) / or_insert_with(...) calls; update only the repeated section so llm.providers is populated once, keeping ProviderConfig construction (ApiType::..., base_url constants like OPENROUTER_PROVIDER_BASE_URL, MINIMAX_PROVIDER_BASE_URL, etc.), openrouter_extra_headers(), and use_bearer_auth flags intact and referencing the same symbols (add_shorthand_provider, ProviderConfig, openrouter_extra_headers, ApiType) to avoid drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/config.rs`:
- Around line 6585-6591: The tests duplicate the closure find_header; extract it
into a shared helper function (e.g., fn find_header<'a>(headers: &'a
[(String,String)], name: &str) -> Option<&'a str>) placed once in the tests
module or a test helper mod and replace both inline closures with calls to that
function; update references in the two test locations (the closures at
find_header usages around lines 6585-6591 and 6653-6659) so future header-lookup
changes are centralized.
- Around line 3453-3644: The code duplicates provider bootstrap logic — remove
the repeated block (the second set of entries for "openrouter", "kilo", "zhipu",
"zai-coding-plan", "opencode-zen", "opencode-go", "minimax", and "minimax-cn")
and consolidate into a single initialization sequence that uses
add_shorthand_provider and the existing entry(...) / or_insert_with(...) calls;
update only the repeated section so llm.providers is populated once, keeping
ProviderConfig construction (ApiType::..., base_url constants like
OPENROUTER_PROVIDER_BASE_URL, MINIMAX_PROVIDER_BASE_URL, etc.),
openrouter_extra_headers(), and use_bearer_auth flags intact and referencing the
same symbols (add_shorthand_provider, ProviderConfig, openrouter_extra_headers,
ApiType) to avoid drift.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/config.rs (1)
6629-6667: Consider adding one env-path assertion for OpenRouter headers.This test covers explicit TOML OpenRouter config well. Adding a companion assertion for
OPENROUTER_API_KEYinload_from_env()would lock coverage for the env entry path too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.rs` around lines 6629 - 6667, Update the test test_explicit_openrouter_provider_toml_injects_extra_headers to also exercise the env-path: set the OPENROUTER_API_KEY env var to a distinct value, call the environment-loading path (e.g. Config::load_from_env() or the existing load_from_env helper), locate the "openrouter" provider on that config, and assert the provider.api_key matches the env value and that the attribution headers (HTTP-Referer, X-OpenRouter-Title, X-OpenRouter-Categories) are present; reference load_from_env and the openrouter provider lookup used in this test to place the new assertions alongside the TOML assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/config.rs`:
- Around line 6629-6667: Update the test
test_explicit_openrouter_provider_toml_injects_extra_headers to also exercise
the env-path: set the OPENROUTER_API_KEY env var to a distinct value, call the
environment-loading path (e.g. Config::load_from_env() or the existing
load_from_env helper), locate the "openrouter" provider on that config, and
assert the provider.api_key matches the env value and that the attribution
headers (HTTP-Referer, X-OpenRouter-Title, X-OpenRouter-Categories) are present;
reference load_from_env and the openrouter provider lookup used in this test to
place the new assertions alongside the TOML assertions.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/config.rs (1)
6996-7034: Add oneload_from_envassertion for OpenRouter headers.Coverage now validates TOML and legacy migration paths well; adding an env-only test would close the last bootstrap entry point from the PR objective.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.rs` around lines 6996 - 7034, Add a new test (e.g., test_openrouter_provider_env_injects_extra_headers) that mirrors test_explicit_openrouter_provider_toml_injects_extra_headers but builds the Config via the environment-loading path (call the env loader you use in codebase such as Config::load_from_env or Config::from_env) instead of from TOML; locate the provider by the same lookup (config.llm.providers.get("openrouter")) and assert the same properties on openrouter_provider.api_type, base_url, api_key, name and that openrouter_provider.extra_headers contains the three attribution headers using the same find_header helper (assert HTTP-Referer == "https://spacebot.sh/", X-OpenRouter-Title == "Spacebot", X-OpenRouter-Categories == "cloud-agent,cli-agent").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/config.rs`:
- Around line 6996-7034: Add a new test (e.g.,
test_openrouter_provider_env_injects_extra_headers) that mirrors
test_explicit_openrouter_provider_toml_injects_extra_headers but builds the
Config via the environment-loading path (call the env loader you use in codebase
such as Config::load_from_env or Config::from_env) instead of from TOML; locate
the provider by the same lookup (config.llm.providers.get("openrouter")) and
assert the same properties on openrouter_provider.api_type, base_url, api_key,
name and that openrouter_provider.extra_headers contains the three attribution
headers using the same find_header helper (assert HTTP-Referer ==
"https://spacebot.sh/", X-OpenRouter-Title == "Spacebot",
X-OpenRouter-Categories == "cloud-agent,cli-agent").
feat: add OpenRouter app attribution headers
Issue: #263
Adds HTTP-Referer, X-OpenRouter-Title, and X-OpenRouter-Categories headers to all OpenRouter API requests per https://openrouter.ai/docs/app-attribution.
Changes
extra_headersfield toProviderConfigfor provider-specific HTTP headerscall_openai()for all OpenRouter requests[llm.provider.openrouter]Note
This PR introduces a new
extra_headersfield toProviderConfig, initially populated with OpenRouter app attribution headers per their documentation. The headers are automatically injected into the HTTP request builder in the OpenAI completion path and are consistently applied across all OpenRouter configuration entry points including environment variables, legacy key paths, and explicit TOML provider configuration. Tests verify the headers are present for OpenRouter configurations while remaining empty for all other providers.Written by Tembo for commit 9ae5ac0. This will update automatically on new commits.