feat(providers): let a provider send configured extra headers - #1
Merged
Conversation
`OpenAiTransport::build_projected_request` writes exactly two headers — authorization and content type — and there is no way to add a third. Several gateways ask callers to identify themselves on the wire (an app name, a referer, a partner id) and cannot attribute traffic without it, so anything routed through this crate arrives anonymous no matter what the caller configured further up. Adds `extra_headers` to `TransportCompat`, applied by the OpenAI and Anthropic transports. Nothing here names a provider: which headers to send is a per-deployment question, so it belongs in configuration rather than in a match arm on a host name. Three details that are deliberate: - **Reserved headers win.** The map is written first and the protocol headers after it, so a config typo cannot unauthenticate a request, change the wire format, or downgrade `anthropic-version`. No blocklist to keep in sync — the ordering is the rule, and tests pin it for both transports. - **Merging is per key.** `TransportCompat::merge` extends rather than replaces, so setting one header in user config does not drop the preset's others the way `.or()` semantics would. - **Bedrock and Vertex are not covered.** Bedrock signs its headers with SigV4; a header added outside the signing step invalidates the signature instead of being attributed. Better to leave those transports alone than to ship something that fails confusingly. Backward compatible on disk: the field is `#[serde(default)]` with `skip_serializing_if`, so an empty map never appears in a rendered config and existing files parse unchanged. A test pins that too, since it is the part a release would break quietly. Verified: aion-config and aion-providers suites pass (240 in the providers lib alone). Reverting the two `insert_extra_headers` calls while keeping the tests fails exactly the three that assert the new behaviour, and leaves the reserved-headers-win test passing — which is what it should do, since that one holds either way.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a generic
extra_headersmap toTransportCompat, applied by the OpenAI and Anthropic transports.Why
OpenAiTransport::build_projected_requestwrites exactly two headers — authorization and content type — and there is no way to add a third. Several gateways ask callers to identify themselves on the wire (an app name, a referer, a partner id) and cannot attribute traffic without it, so anything routed through this crate arrives anonymous no matter what the caller configured further up.Nothing in this change names a provider. Which headers to send is a per-deployment question, so it belongs in configuration rather than in a match arm on a host name — there are no host-specific branches in this crate today and this does not add the first one.
Three deliberate details
anthropic-version. No blocklist to keep in sync — the ordering is the rule, pinned by tests for both transportsTransportCompat::mergeextends rather than replaces, so setting one header in user config does not drop the preset's others the way.or()semantics wouldBackward compatibility
#[serde(default)]withskip_serializing_if, so an empty map never appears in a rendered config and existing files parse unchanged. There is a test for that specifically, since it is the part a release would break quietly.Verification
aion-configandaion-providerssuites pass (240 in the providers lib alone),cargo check --workspaceclean,cargo fmtclean.Reverting the two
insert_extra_headerscalls while keeping the tests fails exactly the three that assert the new behaviour — and leaves the reserved-headers-win test passing, which is correct, since that one holds either way.