Repository navigation
Support engine.model-routing for Claude on native Anthropic and Codex on native OpenAI - #73052
SivaKesava1 with Copilot wants to merge 6 commits into
Conversation
Co-authored-by: SivaKesava1 <11771739+SivaKesava1@users.noreply.github.com>
Co-authored-by: SivaKesava1 <11771739+SivaKesava1@users.noreply.github.com>
…ream corroboration Co-authored-by: SivaKesava1 <11771739+SivaKesava1@users.noreply.github.com>
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality SentinelOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
✅ Ponytail Reviewer completed successfully! Ponytail ReviewerOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
✅ Design Decision Gate — ADR VerifiedADR reviewed: ADR-73052: Native Anthropic and OpenAI Model Routing for Claude and Codex (link unavailable: invalid HTTPS URL). The implementation matches the stated decision. 🏗️ 📋 Verification SummaryThe ADR includes all four required sections: Context, Decision, Alternatives Considered and Consequences. Its status is
The code adds no architectural changes that the ADR does not cover.
|
Comment MemoryPeek at saved memory (pr-code-quality-reviewer)Note This comment is managed by comment memory.Expand the saved memory block to view or edit the persistent context for this thread.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review summary
COMMENT — one medium correctness concern was found in the post-run native-provider validation.
Details
parse_token_usage.cjs:145: an unrecognized nativerouting.providerfalls back to the GitHub policy instead of failing closed, which can produce a false selected outcome. An inline comment requests explicit provider-mismatch rejection.
The specialist pass completed; its other suggestions were either low-confidence/non-actionable or not supported by the inspected source. Static review only; tests were not run.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · ~7 AIC · ⌖ ~1 AIC · ⊞ 1.6K
Comment /review to run again
| }; | ||
| const policy = getAWFModelRoutingPolicy(String(env.GH_AW_ENGINE_ID || "")); | ||
| // Native Claude (Anthropic) and Codex (OpenAI) selections are checked against their own provider's endpoint. | ||
| const routingProvider = (routing.provider && resolveAWFRoutingProvider(String(env.GH_AW_ENGINE_ID || ""), routing.provider)) || "github"; |
There was a problem hiding this comment.
An unrecognized native provider is silently coerced to the GitHub policy here, so a malformed or mismatched routing.provider can be validated as Copilot instead of being rejected; that defeats the provider-scoped fail-closed check and can produce a false selected outcome.
💡 Reject provider mismatches explicitly
If routing.provider is present but resolveAWFRoutingProvider returns an empty value, set the outcome to rejected with a provider-mismatch failure rather than defaulting to github. Only use the GitHub default when the proxy selection omits its provider.
There was a problem hiding this comment.
Fixed in 7fef268. GitHub is now the default only when the selection leaves out its provider. If the provider is unrecognized, isn't one the engine can route on, or differs from the compiled aw_info.model_config.routing.provider, the outcome is rejected with provider_mismatch. Added tests for a malformed provider, for Claude selecting openai, for Codex selecting anthropic, and for a Copilot selection when native routing is compiled.
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd (test-driven development review) and /codebase-design (deep-module, interface-seam review) - found no actionable issues. Approving.
📋 Review notes
Scope reviewed
Focused on the high-impact files identified by pr-triage: model_routing_targets.go (new), model_routing_candidates.go, model_routing.go, awf_model_routing.cjs, claude_harness.cjs, codex_harness.cjs, the ADR, and model-routing.md. The diff patch itself truncated at 3000 lines mid-way through model_routing_targets.go, so I read the remainder of that file and the awf_config*.go/aw_info_model_config.go changes directly from disk to confirm full coverage; no gaps found.
/tdd assessment
model_routing_native_test.go(319 new lines) is a thorough, specification-style test suite: table-driven cases for provider-target validation, allowed-models/alias resolution, and policy narrowing, each with descriptive names that read as behavior specs (e.g. "mismatched Copilot qualification is rejected", "exact native id wins over alias of the same name").claude_harness.test.cjsandcodex_harness.test.cjsadd symmetric native-routing describe blocks covering the fail-closed paths: wrong provider, absent/unconfigured native endpoint, unavailable model, incomplete metadata, incompatible endpoint, unsupported effort, and cross-engine provider rejection. This is exactly the edge-case coverage/tddasks for.go test ./pkg/workflow/... -run ModelRoutingpasses cleanly in this sandbox. JS tests could not be executed here (network/npm registry blocked by sandbox TLS policy, no vitest installed locally), so I verified the JS logic by direct reading instead; the new tests look consistent with the implementation (e.g.routingEndpointPathsprotocol-to-path mapping,resolveAWFRoutingProvidergating).
/codebase-design assessment
- The new
model_routing_targets.gomodule is appropriately deep: a small public surface (validateModelRoutingProviderTarget) hides provider/target/credential/custom-gateway checks behind one call, consistent with the existingmodel_routing.go/model_routing_candidates.goseam. - Namespace handling (
modelRoutingNamespace,qualifiedModelID,stripModelRoutingNamespacePrefix) cleanly generalizes the existinggithub-copilotnamespace concept toanthropic/openaiwithout introducing a parallel code path; aliases, policy matching, and candidate resolution all route through the same namespace-aware helpers. - Fail-closed semantics in
awf_model_routing.cjs'sresolveNativeAWFModelRoutingSelectionmirror the existing Copilot selection validator's structure, keeping the two code paths easy to compare side by side. - The ADR clearly documents three alternatives considered and why native-target validation (not just provider-label validation) was required; good use of domain vocabulary ("effective routing provider", "native HTTPS endpoint") that matches the code's naming.
Minor observations (non-blocking)
- The PR body itself discloses that the two new smoke workflows haven't been run against real Anthropic/OpenAI keys yet, and that CodeQL timed out for this PR. Neither blocks this review, but worth confirming before merge that the smokes pass at least once with real credentials.
- ADR status is "Proposed" rather than "Accepted"; the author may want to update it once this PR is approved/merged, per the team's ADR lifecycle convention (not verified here).
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · ~118 AIC · ⌖ ~18 AIC · ⊞ 10.2K
Comment /matt to run again
There was a problem hiding this comment.
🟡 Changes recommended
Provider-prefix handling can unintentionally authorize models on Copilot endpoints through the legacy allowlist and cause valid provider-qualified expression candidates to fail compilation.
2 open issues
What changed in this PR
This PR connects gh-aw to AWF’s existing provider-scoped routing, allowing Claude and Codex workflows to retain native Anthropic/OpenAI API keys and billing.
Changes:
- Adds native-provider validation, candidate expansion, and configuration while retaining the Copilot routing path.
- Makes harness validation, usage processing, and analytics provider-aware.
- Adds native smoke workflows, regression coverage, and updated schemas and documentation.
| File | Description |
|---|---|
schemas/logs-jsonl.schema.json |
Adds model-catalog evidence to log records. |
schemas/audit.schema.json |
Adds model-catalog evidence to audit output. |
pkg/workflow/schemas/awf-config.schema.json |
Defines supported routing providers. |
pkg/workflow/model_routing.go |
Adds native-provider routing and policy handling. |
pkg/workflow/model_routing_test.go |
Updates provider compatibility tests. |
pkg/workflow/model_routing_targets.go |
Validates native credentials and inference targets. |
pkg/workflow/model_routing_native_test.go |
Covers native compilation and Copilot compatibility. |
pkg/workflow/model_routing_candidates.go |
Resolves candidates and policies by provider. |
pkg/workflow/model_routing_aliases_test.go |
Shares compilation helpers with native tests. |
pkg/workflow/awf_config.go |
Adds the routing provider field. |
pkg/workflow/awf_config_build.go |
Emits native routing providers. |
pkg/workflow/aw_info_model_config.go |
Records the configured native provider. |
pkg/parser/schemas/main_workflow_schema.json |
Updates routing option descriptions. |
docs/src/content/docs/specs/unified-agent-session-specification.md |
Documents provider-aware routing metadata. |
docs/src/content/docs/reference/model-routing.md |
Explains native routing, policies, and limitations. |
docs/src/content/docs/reference/frontmatter-full.md |
Updates routing configuration comments. |
docs/public/schemas/unified-session.schema.json |
Defines native routing metadata. |
docs/adr/73052-native-anthropic-openai-model-routing-for-claude-and-codex.md |
Records the proposed integration design. |
actions/setup/js/types/unified_session.d.ts |
Adds native provider typing. |
actions/setup/js/smoke_model_routing_assertions.test.cjs |
Tests native routing evidence checks. |
actions/setup/js/smoke_model_routing_assertions.cjs |
Validates native selection, traffic, and inventory. |
actions/setup/js/routing_analytics.test.cjs |
Tests provider metadata projection. |
actions/setup/js/routing_analytics.cjs |
Preserves supported native provider values. |
actions/setup/js/parse_token_usage.test.cjs |
Tests native post-run validation. |
actions/setup/js/parse_token_usage.cjs |
Checks provider-specific selections and endpoint evidence. |
actions/setup/js/codex_harness.test.cjs |
Covers native OpenAI routing behavior. |
actions/setup/js/codex_harness.cjs |
Resolves routing against Codex’s provider. |
actions/setup/js/claude_harness.test.cjs |
Covers native Anthropic routing behavior. |
actions/setup/js/claude_harness.cjs |
Requires matching provider selection and endpoint. |
actions/setup/js/awf_reflect.cjs |
Corrects the listener-availability comment. |
actions/setup/js/awf_model_routing.test.cjs |
Tests provider resolution and protocol mapping. |
actions/setup/js/awf_model_routing.cjs |
Adds shared native-selection validation. |
.github/workflows/smoke-codex-routed-openai.md |
Adds a native OpenAI routing smoke. |
.github/workflows/smoke-claude-routed-anthropic.md |
Adds a native Anthropic routing smoke. |
.github/workflows/agentic_commands.yml |
Registers native smokes for label triggers. |
Review effort: Max (auto)
| candidates = modelRoutingDefaultCandidates(engineName) | ||
| candidates = modelRoutingDefaultCandidates(engineName, namespace) | ||
| } | ||
| if namespace != copilotRoutingNamespace { |
There was a problem hiding this comment.
Fixed in 7fef268. I kept the provider-qualified IDs on the legacy path instead of failing compilation, so workflows with a digest-only image or an older AWF still compile. Native prefixes are now stripped only for routing.candidateModels. Without that support, apiProxy.allowedModels gets anthropic/… / openai/…, so a bare ID can't also authorize a Copilot model. Added TestModelRoutingNativeLegacyAllowlistKeepsProviderPrefix, which covers Claude and Codex with AWF v0.28.32 and with a digest-only api-proxy image.
| if containsExpression(model) { | ||
| return model, false, nil | ||
| } | ||
| provider, name, qualified := strings.Cut(model, "/") | ||
| if !qualified { | ||
| return model, false, nil | ||
| } | ||
| if strings.EqualFold(provider, namespace) { | ||
| return name, true, nil | ||
| } |
There was a problem hiding this comment.
Fixed in 7fef268. stripModelRoutingNamespacePrefix now ignores slashes inside expressions when it finds the provider. That way a literal anthropic/ or openai/ in front of ${{ … }} gets the same normalization as other entries, and a mismatched prefix is still rejected. Added test cases for both.
…e-model-routing Co-authored-by: SivaKesava1 <11771739+SivaKesava1@users.noreply.github.com>
…ched routing providers, and normalize native prefixes before expressions Co-authored-by: SivaKesava1 <11771739+SivaKesava1@users.noreply.github.com>
Today
engine.model-routingonly works when Claude or Codex send inference through GitHub Copilot. The workaround ismodel-provider: github, which switches the workflow to Copilot credentials and billing. AWF has supported provider-scoped routing (apiProxy.routing.provider: anthropic|openai) since v0.28.28. This PR connects gh-aw to that, so workflows can keep their own provider API keys. Copilot routing is unchanged, and all existing locks are byte-identical.compiles to
apiProxy.routing: { provider: "anthropic", candidateModels: ["claude-sonnet-5"], ... }.Validation (
model_routing.go, newmodel_routing_targets.go)githuboranthropic(including omitted), and Codex withgithuboropenai(including omitted). Copilot and pi still accept onlygithub.engine.api-target;*_BASE_URLinengine.env;sandbox.agent.targets.<provider>;engine.authconfigured for a different provider;model-routing. They never tell a supported native-key workflow to switch to Copilot.githubpath is still not checked for custom targets, so existing Copilot and GitHub Enterprise Serverapi-targetbehaviour stays the same.Config and candidates (
awf_config*.go,model_routing_candidates.go)routing.provideris written only for native routes. Copilot routes leave it out.claude-*andgpt-*.claude-sonnet-5-5work.sonnetbecomesclaude-*sonnet*.anthropic/andopenai/patterns apply to native routes, andgithub-copilot/claude-*no longer authorizes Anthropic.Harnesses (
awf_model_routing.cjs,claude_harness.cjs,codex_harness.cjs,parse_token_usage.cjs)/reflectendpoint for that provider.messages→/v1/messages,responses→/responses,chat_completions→/chat/completions./v1/responsesnow counts as evidence for a/responsesselection. Copilot comparisons stay exact.Smokes
New smokes:
smoke-claude-routed-anthropic(shortlistclaude-sonnet-5) andsmoke-codex-routed-openai(shortlistgpt-5.4). Both use native-key credentials and do not usecopilotInventoryorexpectSelectedEffort: "max".New checks N1–N3 in
smoke_model_routing_assertions.cjscover:Copilot checks are unchanged.
Docs
reference/model-routing.md: getting-started authentication, the engine/provider table, a new "Native Anthropic and OpenAI routing" section, alias and policy semantics, the renamed "Supported providers and engines" limitation section, and troubleshooting rows.frontmatter-full.md.docs/adr/73052-...(Status: Proposed).Review notes