Fix Claude engine routing for model-provider: github#46013
Conversation
…t.com, add domain, mark experimental Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #46013 does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100). |
|
✅ Test Quality Sentinel completed test quality analysis. |
|
/smoke-claude-on-copilot |
There was a problem hiding this comment.
Pull request overview
This PR updates gh-aw’s AWF (firewall) configuration generation so that when the Claude engine is configured with model-provider: github, the Anthropic proxy sidecar routes to GitHub Copilot’s inference endpoint (and the firewall allows the required outbound domain), preventing auth failures caused by sending Copilot tokens to api.anthropic.com.
Changes:
- Auto-configure the AWF API proxy “anthropic” target to
api.githubcopilot.comwithAuthorizationheader when Claude uses the GitHub provider (unlessANTHROPIC_BASE_URLis explicitly set). - Permit
api.githubcopilot.comfor Claude by adding it to default domain allowlists, and regenerate affected lock/golden artifacts. - Emit an experimental-feature warning when
engine.model-provideris overridden to non-Anthropic on the Claude engine.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/awf_config.go | Adds conditional AWF target override for Claude+GitHub provider routing. |
| pkg/workflow/awf_config_test.go | Adds tests covering the new Claude+GitHub provider AWF target behavior and precedence rules. |
| pkg/workflow/domains.go | Adds api.githubcopilot.com to Claude default firewall domains. |
| pkg/workflow/compiler_validators.go | Adds an experimental warning when Claude’s model-provider is set to a non-Anthropic value. |
| pkg/workflow/testdata/TestWasmGolden_AllEngines/claude.golden | Updates golden output to reflect the new allowed domain. |
| .github/workflows/typist.lock.yml | Regenerated lock to include api.githubcopilot.com in allowlists. |
| .github/workflows/skillet.lock.yml | Regenerated lock to include api.githubcopilot.com in allowlists (also updates header/manifest content). |
| .github/workflows/release.lock.yml | Regenerated lock to include api.githubcopilot.com in allowlists (also updates header/manifest content). |
| .github/workflows/github-mcp-tools-report.lock.yml | Regenerated lock to include api.githubcopilot.com in allowlists. |
| .github/workflows/example-workflow-analyzer.lock.yml | Regenerated lock to include api.githubcopilot.com in allowlists. |
| .github/workflows/detection-analysis-report.lock.yml | Regenerated lock to include api.githubcopilot.com in allowlists. |
| .github/workflows/daily-token-consumption-report.lock.yml | Regenerated lock to include api.githubcopilot.com in allowlists. |
| .github/workflows/daily-reliability-review.lock.yml | Regenerated lock to include api.githubcopilot.com in allowlists. |
| .github/workflows/daily-choice-test.lock.yml | Regenerated lock to include api.githubcopilot.com in allowlists. |
| .github/workflows/copilot-agent-analysis.lock.yml | Regenerated lock to include api.githubcopilot.com in allowlists. |
| .github/workflows/ci-doctor.lock.yml | Regenerated lock to include api.githubcopilot.com in allowlists. |
| .github/workflows/audit-workflows.lock.yml | Regenerated lock to include api.githubcopilot.com in allowlists. |
| .github/workflows/agentic-auto-upgrade.yml | Updates the scheduled cron time for the auto-upgrade workflow. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 39/69 changed files
- Comments generated: 3
- Review effort level: Low
|
❌ Smoke Claude on Copilot failed. Please review the logs for details. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
The fix is well-targeted and correctly handles the Claude + model-provider: github routing bug.
Summary
awf_config.go: When the Claude engine usesmodel-provider: githuband no explicitANTHROPIC_BASE_URLis set, auto-configures theanthropicsidecar to route toapi.githubcopilot.comwith anAuthorizationheader.compiler_validators.go: Emits an experimental-feature warning forclaude engine.model-providerwhen a non-Anthropic provider is used.domains.go: Addsapi.githubcopilot.comtoClaudeDefaultDomainsso the firewall allowlist includes the endpoint.- Tests: Both new test cases cover the happy path and the explicit-override precedence correctly.
- Lock/golden files: Regenerated consistently.
Minor observation
api.githubcopilot.com is added to ClaudeDefaultDomains, meaning it is allowed for all Claude workflows even when model-provider is anthropic. This is a safe over-allowance (GitHub-owned domain), but could be tightened in a follow-up by conditionally injecting the domain only when the GitHub provider is active.
No blocking issues found.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 42 AIC · ⌖ 6.79 AIC · ⊞ 5K
There was a problem hiding this comment.
Skills-Based Review 🧠\n\nApplied /diagnosing-bugs and /tdd — one minor gap noted, no blocking issues.\n\n\n📋 Summary
\n\n#### Positive Highlights\n\n- ✅ Root cause properly addressed: the else if guard correctly short-circuits when an explicit ANTHROPIC_BASE_URL is set, preserving user overrides.\n- ✅ Two targeted regression tests added (awf_config_test.go): one for the happy path and one for the explicit-override precedence case.\n- ✅ api.githubcopilot.com added to ClaudeDefaultDomains so the firewall allows the new sidecar route without extra user config.\n- ✅ Experimental-feature warning emitted for non-default providers on the Claude engine — good discoverability.\n- ✅ Comment in awf_config.go explains why Authorization replaces x-api-key; that context is easy to lose.\n\n#### One Gap\n\nThe new experimental-feature warning in compiler_validators.go:326 has no test in compiler_validators_test.go. Every other flag in that slice is covered; this one should be too. See inline comment.\n\n
📋 Summary
\n\n#### Positive Highlights\n\n- ✅ Root cause properly addressed: theelse if guard correctly short-circuits when an explicit ANTHROPIC_BASE_URL is set, preserving user overrides.\n- ✅ Two targeted regression tests added (awf_config_test.go): one for the happy path and one for the explicit-override precedence case.\n- ✅ api.githubcopilot.com added to ClaudeDefaultDomains so the firewall allows the new sidecar route without extra user config.\n- ✅ Experimental-feature warning emitted for non-default providers on the Claude engine — good discoverability.\n- ✅ Comment in awf_config.go explains why Authorization replaces x-api-key; that context is easy to lose.\n\n#### One Gap\n\nThe new experimental-feature warning in compiler_validators.go:326 has no test in compiler_validators_test.go. Every other flag in that slice is covered; this one should be too. See inline comment.\n\n🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 49.7 AIC · ⌖ 4.48 AIC · ⊞ 6.7K
Comment /matt to run again
🧪 Test Quality Sentinel Report✅ Test Quality Score: 75/100 — Acceptable
📊 Metrics (2 tests)
Analysis Summary✅ Strengths:
Verdict✅ Tests passed review. 0% implementation tests (threshold: 30%). No guideline violations. Design coverage is strong for this critical configuration change. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
There was a problem hiding this comment.
Routing logic is correct and tests cover the key scenarios well. One minor observation below.
🔎 Code quality review by PR Code Quality Reviewer · 81.9 AIC · ⌖ 4.25 AIC · ⊞ 5.6K
Comment /review to run again
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
… not anthropic When model-provider: github is set on the Claude engine, AWF should configure the copilot provider (port 10002) to forward to api.githubcopilot.com. The driver already sets ANTHROPIC_BASE_URL to http://host.docker.internal:10002 (copilot proxy port), and the harness resolves the endpoint from /reflect — which now correctly maps provider=github → copilot → http://api-proxy:10002 instead of falling back to the only-configured anthropic proxy. Previously the anthropic sidecar (port 10001) was configured to target api.githubcopilot.com, but the harness was sending all traffic there via the copilot proxy port anyway, making that configuration wrong. The fix ensures the copilot proxy is marked as configured in AWF so /reflect correctly routes the claude harness to the copilot endpoint. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot please run the Unresolved review threads:
Failed checks:
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…epo slug detection - compile_orchestrator.go: isLocalhostOrigin() skips GHES mode for localhost:PORT origins (CI agent proxies), preventing SkipHardcodedFallback from being set and restoring # v7.0.0 version annotations in lock files - git.go: fall back to GITHUB_REPOSITORY env var when git remote URL cannot be parsed as a GitHub host, fixing schedule scattering for agentic-auto-upgrade.yml in agent envs - compile_orchestrator_host_test.go: add test for localhost remote not treated as GHES Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
All three threads were already replied to in earlier commits. To summarize the current state:
CI re-trigger needed to verify the smoke test passes on HEAD. |
|
/smoke-claude-on-copilot |
|
❌ Smoke Claude on Copilot failed. Please review the logs for details. |
… remotes Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
All items addressed. Summary of what was done in this pass: Review threads (all have been replied to; reviewers can now resolve):
Additional fix in this pass:
Local validation: CI re-trigger needed from a maintainer to verify the smoke test on HEAD. |
|
@copilot review and figure out the failure in https://github.com/github/gh-aw/actions/runs/29526411109 |
…c proxy (port 10001) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
/smoke-claude-on-copilot |
|
@copilot please run the
|
|
❌ Smoke Claude on Copilot failed. Please review the logs for details. |
… used instead of PAT api.githubcopilot.com rejects Personal Access Tokens (COPILOT_GITHUB_TOKEN) with "400 Personal Access Tokens are not supported for this endpoint". Adding the copilot-requests: write permission causes llmProviderSecretExpression to return github.token instead, which is accepted by the Copilot API endpoint. Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Investigated the latest smoke failure (run 29536903527) and fixed it in the latest commit. Root cause: The smoke workflow lacked Fix: Added Local validation: Still needed: CI re-trigger by a maintainer to verify the smoke test passes on HEAD. All three unresolved review threads are outdated (addressed in earlier commits — |
|
@copilot please run the Unresolved review threads:
Failed checks:
The threads already have author follow-up replies; please verify the current HEAD, resolve any stale review state, and get the smoke run green.
|
All review threads are now resolved (verified via the GitHub API). Current state of the failing check: Run 29536903527 failed on SHA Local validation: Still needed: A maintainer needs to re-trigger |
🤖 PR Triage
Rationale: Claude engine routing fix for model-provider:github — prevents 401s caused by anthropic sidecar targeting api.anthropic.com with a Copilot token. CI copilot check passing. GHA approved multiple times. High impact correctness fix — expedite.
|
When
model-provider: githubis set on the Claude engine, the AWF anthropic proxy sidecar was still targetingapi.anthropic.com— causing 401s because it was receiving a Copilot token. The harness resolves its endpoint from/reflectdata and falls back to the only configured sidecar (anthropic, port 10001), bypassing the intended GitHub routing.Changes
awf_config.go— When Claude engine usesmodel-provider: githuband no explicitANTHROPIC_BASE_URLis set, auto-configure the anthropic proxy target:Switches from the default
x-api-keyheader toAuthorizationto match Copilot API auth.domains.go— Addapi.githubcopilot.comtoClaudeDefaultDomainsso the firewall permits the outbound connection.compiler_validators.go— Emit an experimental feature warning whenmodel-provideris overridden to a non-anthropic value on the Claude engine.