fix(api-proxy): generalize deprecated header stripping for any provider/header#3689
fix(api-proxy): generalize deprecated header stripping for any provider/header#3689lpcox wants to merge 9 commits into
Conversation
Clarify that GET /reflect is available on all provider ports (10000–10004), each returning reflection metadata for its own adapter. Note that port 10000 (OpenAI) is also the management port serving /health and /metrics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts: # docs/awf-config-spec.md
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Extends the API proxy’s “deprecated anthropic-beta header” retry/learning logic to also apply when requests are routed via the Copilot provider (port 10002), ensuring 400 rejections from the Copilot gateway→Anthropic path trigger stripping + a single retry and that learned values are proactively removed on subsequent Copilot requests.
Changes:
- Apply proactive cached stripping of learned deprecated
anthropic-betavalues forprovider === 'copilot'when the header is present. - Apply reactive “buffer 400 → detect deprecated value → strip → retry once” logic for
provider === 'copilot'. - Improve observability by logging the actual provider (
copilotvs alwaysanthropic) when stripping occurs, and add targeted regression tests for both reactive and proactive Copilot paths.
Show a summary per file
| File | Description |
|---|---|
| containers/api-proxy/proxy-request.js | Extends deprecated anthropic-beta stripping/learning logic to cover Copilot provider requests and fixes provider attribution in logs. |
| containers/api-proxy/server.proxy.test.js | Adds regression tests covering Copilot-provider retry-on-400 stripping and proactive cached stripping after learning. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 0
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
4d285a9 to
507e777
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…er/header Refactor the anthropic-beta-specific retry-on-400 logic into a general deprecated header value detection system that: 1. Parses the error pattern: Unexpected value(s) `<value>` for the `<header>` header 2. Works for ANY header name (not just anthropic-beta) 3. Applies to both anthropic and copilot providers 4. Learns rejected (header, value) pairs and proactively strips them from subsequent requests without needing a retry round-trip The previous implementation (PR #3657) only handled provider=anthropic with the anthropic-beta header hardcoded. This caused Copilot CLI v1.0.48+ workflows to fail when sending context-1m-2025-08-07 through the copilot provider (port 10002 → api.githubcopilot.com → Anthropic). Key changes: - deprecatedHeaderValues: Map<headerName, Set<rejectedValues>> replaces the single anthropicDeprecatedBetaValues Set - DEPRECATED_HEADER_PATTERN captures both value and header name from the error message - shouldBuffer400ForHeaderStrip triggers on any 400 from anthropic/copilot providers (not gated on anthropic-beta header presence) - Log event renamed: deprecated_header_stripped (includes header field) Closes #3656 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
507e777 to
15c02e3
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…ort, function or class' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
|
Smoke Test Results ✅ GitHub API (recent-prs.json): 2 PR entries confirmed Overall: PASS
|
Smoke Test Results✅ MCP: PR #3670 retrieved: [Test Coverage] Add comprehensive tests for config-assembly validator Status: FAIL (file verification failed)
|
Copilot BYOK Smoke Test ResultsPR: #3689 - fix(api-proxy): generalize deprecated header stripping (@lpcox) ✅ GitHub MCP connectivity (fetched PR #3689) Note: Running in BYOK offline mode ( Status: PARTIAL PASS (2/3 core tests passed, 1 infra test failed)
|
|
Smoke test FAIL: connectivity and MCP issues detected. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
|
Smoke Test Codex: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
API Proxy OTEL Tracing Smoke Test ResultsSummaryAll OTEL integration scenarios validated successfully. Results✅ Scenario 1: Module Loading
✅ Scenario 2: Test Suite
✅ Scenario 3: Env Var Forwarding
✅ Scenario 4: Token Tracker Integration
✅ Scenario 5: OTEL Diagnostics
Architecture Notes
ConclusionAll scenarios validated. OTEL integration is production-ready. Warning Firewall blocked 3 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "127.0.0.1"
- "api.example.com"
- "api.openai.com"See Network Configuration for more information.
|
Chroot Runtime Version Test Results
Overall Result: ❌ Tests failed - version mismatches detected The chroot environment does not have matching runtime versions for Python and Node.js. This could indicate:
|
Smoke Test: Services Connectivity — ❌ FAIL
Result: FAIL — AWF sandbox cannot reach GitHub Actions service containers via host.docker.internal
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS All build and test operations completed successfully across all 18 projects in 8 ecosystems. The firewall successfully allowed necessary package manager traffic (npm, cargo, Maven, go mod, bun, deno, dotnet, CMake) while maintaining network isolation.
|
Problem
PR #3657 added retry-on-400 logic for deprecated
anthropic-betaheaders, but it was too narrowly scoped:provider === 'anthropic'— missed the copilot provider pathanthropic-betaheader specifically — would not adapt to other headers being rejectedanthropic-betawas present in the requestThis caused Copilot CLI v1.0.48/v1.0.51 workflows to fail when sending
context-1m-2025-08-07through the copilot provider (run 26347219614).Solution
Generalized the retry-on-400 logic into a header-agnostic system:
How it works
Unexpected value(s) \` for the `` header`Map<headerName, Set<rejectedValues>>Key design decisions
DEPRECATED_HEADER_PATTERNcaptures both the rejected value AND the header name, so it works for any header Anthropic (or the Copilot gateway) might reject in the futuredeprecatedHeaderValuesis aMap<string, Set<string>>with a 200-value cap per headerTesting
5 test cases covering:
anthropic-beta(new)All 47 proxy tests pass.
Related
anthropic-betaheader rejection #3657