Skip to content

fix: preserve provider-required User-Agent on outbound requests, refs #2303 - #2501

Merged
looplj merged 1 commit into
looplj:unstablefrom
remote-controlled-man:codex/axonhub-2303-copilot-transform
Sep 21, 2026
Merged

looplj merged 1 commit into
looplj:unstablefrom
remote-controlled-man:codex/axonhub-2303-copilot-transform

Conversation

@remote-controlled-man

@remote-controlled-man remote-controlled-man commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Refs #2303.

Problem

For channels whose outbound transformer sets a provider-required User-Agent — e.g. GitHub Copilot channels set GitHubCopilotChat/0.26.7 via SetCopilotHeaders — the header never reached the upstream on real requests. Two layers erased it:

  1. The pipeline merges inbound client headers into the outbound request (MergeInboundRequest → MergeHTTPHeaders), and User-Agent was not blocked, so the client's UA (e.g. codex_cli_rs/...) overwrote the transformer-set one.
  2. The user-agent-pass-through middleware then forced User-Agent: axonhub/1.0 whenever UA pass-through was disabled (the default), discarding any UA the transformer had set.

The quota/health checker builds its request directly with SetCopilotHeaders and bypasses this pipeline (github_copilot_checker.go), which is why checking the channel showed the correct GitHubCopilotChat UA while real traffic went out as axonhub/1.0. The UI channel-test path does go through the pipeline and was equally affected before this fix.

Fix

  • llm/httpclient: treat User-Agent as a blocked header in MergeHTTPHeaders, so the inbound client UA is never auto-merged into the outbound request (same treatment as the transformer-owned Accept). Client-UA forwarding remains owned by the explicit pass-through middleware.
  • internal/server/orchestrator: when pass-through is disabled, the middleware applies the axonhub/1.0 default only when the outbound transformer set no User-Agent; a transformer-set (provider-required) UA is preserved.

Unchanged: pass-through enabled still forwards the client UA; channel header overrides still run after the middleware and can force any UA; transformers that set no UA still get the axonhub/1.0 default.

Scope note

Issue #2303 also reports Responses→Chat tool loss for Copilot. On current unstable, tool declarations already survive that path, and the remaining tool-lifecycle gap is being addressed by open PR #2220 (Responses-to-Chat tool compatibility), so this PR intentionally stays on the UA half to avoid duplicating that work.

Tests

  • New TestMergeHTTPHeaders_UserAgentNotMerged (llm/httpclient): client UA must not overwrite or enter outbound headers.
  • New TestApplyUserAgentPassThrough cases: transformer-set UA (GitHubCopilotChat/0.26.7) is preserved when pass-through is disabled; client UA still wins when enabled.
  • cd llm && go test ./... passes (one pre-existing websocket_executor failure also fails on a clean checkout of the base on Windows); go test ./internal/server/orchestrator/ passes; gofmt/go vet clean on changed files.

Summary by CodeRabbit

  • Bug Fixes
    • Preserved provider-required User-Agent values on outbound requests when User-Agent pass-through is disabled.
    • Prevented inbound client User-Agent values from overriding outbound request headers.
    • Applied the default User-Agent only when no outbound value is already present.
    • Continued supporting explicit User-Agent pass-through and custom header forwarding.

…ooplj#2303

The outbound transformer for channels like GitHub Copilot sets a
provider-required User-Agent (GitHubCopilotChat/...), but it never
reached the upstream on real calls:

1. The pipeline merged the inbound client's User-Agent into the
   outbound request, overwriting the transformer-set value.
2. The user-agent-pass-through middleware then forced the default
   axonhub/1.0 whenever pass-through was disabled (the default).

Channel checks bypass that pipeline, so they showed the correct UA
while real traffic went out as axonhub.

- llm/httpclient: treat User-Agent as a blocked header in
  MergeHTTPHeaders so the client UA is never auto-merged upstream;
  the pass-through middleware remains the single owner of client-UA
  forwarding.
- orchestrator: when pass-through is disabled, apply the axonhub/1.0
  default only when the transformer set no User-Agent. Pass-through
  enabled still forwards the client UA, and channel override headers
  still run afterwards.
Copilot AI lite review requested due to automatic review settings September 19, 2026 09:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 328c275d-f623-4799-a7b7-2d393ecc6f69

📥 Commits

Reviewing files that changed from the base of the PR and between cb29b65 and 949bc29.

📒 Files selected for processing (5)
  • internal/server/orchestrator/orchestrator.go
  • internal/server/orchestrator/pass_through.go
  • internal/server/orchestrator/pass_through_test.go
  • llm/httpclient/utils.go
  • llm/httpclient/utils_test.go

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The change prevents client User-Agent headers from being merged automatically. It preserves User-Agent values set by outbound transformers when pass-through is disabled. Tests cover both disabled and enabled pass-through behavior.

Changes

User-Agent forwarding

Layer / File(s) Summary
Header merge policy
llm/httpclient/utils.go, llm/httpclient/utils_test.go
MergeHTTPHeaders no longer merges the client User-Agent. Tests verify preservation of transformer values, absence of unintended defaults, and continued merging of custom headers.
Middleware User-Agent preservation
internal/server/orchestrator/pass_through.go, internal/server/orchestrator/pass_through_test.go, internal/server/orchestrator/orchestrator.go
The middleware sets axonhub/1.0 only when the outbound request has no User-Agent. Tests verify transformer values remain unchanged without pass-through and that enabled pass-through uses the client value. The processing comment documents the behavior.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: looplj

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preserving provider-required User-Agent values on outbound requests. The refs #2303`` suffix is relevant issue-tracking context.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@looplj
looplj merged commit 86c51a8 into looplj:unstable Sep 21, 2026
6 checks passed
Edint386 pushed a commit to Edint386/axonhub that referenced this pull request Oct 9, 2026
…ooplj#2303 (looplj#2501)

The outbound transformer for channels like GitHub Copilot sets a
provider-required User-Agent (GitHubCopilotChat/...), but it never
reached the upstream on real calls:

1. The pipeline merged the inbound client's User-Agent into the
   outbound request, overwriting the transformer-set value.
2. The user-agent-pass-through middleware then forced the default
   axonhub/1.0 whenever pass-through was disabled (the default).

Channel checks bypass that pipeline, so they showed the correct UA
while real traffic went out as axonhub.

- llm/httpclient: treat User-Agent as a blocked header in
  MergeHTTPHeaders so the client UA is never auto-merged upstream;
  the pass-through middleware remains the single owner of client-UA
  forwarding.
- orchestrator: when pass-through is disabled, apply the axonhub/1.0
  default only when the transformer set no User-Agent. Pass-through
  enabled still forwards the client UA, and channel override headers
  still run afterwards.
looplj pushed a commit that referenced this pull request Oct 11, 2026
…g client headers (#2635) (#2638)

The channel-level and global "pass through User-Agent" switch was inert for
codex channels because of two stacked causes:

1. The codex outbound transformer unconditionally copied the inbound client
   User-Agent back onto the outbound request (both the Responses path and the
   mirrored alpha-search path), so the client fingerprint always won before any
   policy layer ran.
2. After #2501 the orchestrator middleware only applies the axonhub default
   when the outbound request has no User-Agent at all. Since the transformer
   always produced one, the disabled branch never applied.

Fix the transformer side only, keeping #2501's contract intact: the transformer
now owns the UA and presents a Codex-client-like default
(codexDefaultUserAgent = "codex_cli_rs/" + codexDefaultVersion), symmetric with
how Version already defaults to codexDefaultVersion (#2591). Client-UA
forwarding remains solely owned by applyUserAgentPassThrough, whose
disabled-branch semantics are unchanged, so a transformer-set provider UA
(e.g. GitHubCopilotChat on Copilot channels) is still preserved when the switch
is off and the client UA still wins when it is on.

Also stop leaking client attribution headers: block the non-standard
Http-Referer spelling (the canonical Referer was already blocked) and the
client-private X-Zcode-* prefix. X-Title / X-Platform / X-Os-* are deliberately
left unblocked because OpenRouter-style channels legitimately use them.

Tests:
- codex outbound/alpha-search: outbound UA is the codex default and never the
  inbound client UA, including through the generic inbound-header merge.
- orchestrator: switch enabled -> client UA wins over the codex default;
  switch disabled -> the codex default survives; Copilot-preservation case
  still passes.
- httpclient: Http-Referer and X-Zcode-* are not merged into outbound headers.
Edint386 pushed a commit to Edint386/axonhub that referenced this pull request Oct 12, 2026
…g client headers (looplj#2635) (looplj#2638)

The channel-level and global "pass through User-Agent" switch was inert for
codex channels because of two stacked causes:

1. The codex outbound transformer unconditionally copied the inbound client
   User-Agent back onto the outbound request (both the Responses path and the
   mirrored alpha-search path), so the client fingerprint always won before any
   policy layer ran.
2. After looplj#2501 the orchestrator middleware only applies the axonhub default
   when the outbound request has no User-Agent at all. Since the transformer
   always produced one, the disabled branch never applied.

Fix the transformer side only, keeping looplj#2501's contract intact: the transformer
now owns the UA and presents a Codex-client-like default
(codexDefaultUserAgent = "codex_cli_rs/" + codexDefaultVersion), symmetric with
how Version already defaults to codexDefaultVersion (looplj#2591). Client-UA
forwarding remains solely owned by applyUserAgentPassThrough, whose
disabled-branch semantics are unchanged, so a transformer-set provider UA
(e.g. GitHubCopilotChat on Copilot channels) is still preserved when the switch
is off and the client UA still wins when it is on.

Also stop leaking client attribution headers: block the non-standard
Http-Referer spelling (the canonical Referer was already blocked) and the
client-private X-Zcode-* prefix. X-Title / X-Platform / X-Os-* are deliberately
left unblocked because OpenRouter-style channels legitimately use them.

Tests:
- codex outbound/alpha-search: outbound UA is the codex default and never the
  inbound client UA, including through the generic inbound-header merge.
- orchestrator: switch enabled -> client UA wins over the codex default;
  switch disabled -> the codex default survives; Copilot-preservation case
  still passes.
- httpclient: Http-Referer and X-Zcode-* are not merged into outbound headers.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@remote-controlled-man
remote-controlled-man deleted the codex/axonhub-2303-copilot-transform branch October 13, 2026 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants