Split engine provider selection from harness execution and route Claude/Codex via reflected CAPI endpoints#45490
Conversation
…t routing Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…ne.provider override Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot add smoke-github-Claude that uses github to run Claude code |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Implemented in |
🤖 PR Triage
Splits engine provider selection from harness execution and routes Claude/Codex via reflected CAPI endpoints. High-impact architectural change: new smoke workflow, new Go/JS files. Needs careful review.
|
There was a problem hiding this comment.
Pull request overview
Separates provider selection from harness execution and introduces reflected endpoint routing for Claude and Codex.
Changes:
- Adds
engine.providerparsing and provider aliases. - Adds runtime endpoint discovery and harness configuration.
- Adds tests and a GitHub-provider Claude smoke workflow.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/llm_provider.go |
Normalizes provider aliases. |
pkg/workflow/engine.go |
Parses provider overrides. |
pkg/workflow/engine_config_test.go |
Tests override precedence. |
pkg/workflow/codex_engine.go |
Updates provider documentation. |
pkg/workflow/claude_engine.go |
Updates provider documentation. |
pkg/workflow/agentic_engine.go |
Renames provider and harness interfaces. |
actions/setup/js/codex_harness.test.cjs |
Tests Codex endpoint configuration. |
actions/setup/js/codex_harness.cjs |
Reconfigures Codex from reflect data. |
actions/setup/js/claude_harness.cjs |
Configures Claude from reflect data. |
actions/setup/js/awf_reflect.test.cjs |
Tests endpoint resolution. |
actions/setup/js/awf_reflect.cjs |
Adds shared provider endpoint resolution. |
.github/workflows/smoke-github-claude.md |
Adds Claude GitHub-provider smoke test. |
.github/workflows/smoke-github-claude.lock.yml |
Compiles the smoke workflow. |
.github/workflows/agentic_commands.yml |
Registers its slash command. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 13/14 changed files
- Comments generated: 6
- Review effort level: Medium
| if providerValue, hasProvider := engineObj["provider"]; hasProvider && !config.IsInlineDefinition { | ||
| if providerStr, ok := providerValue.(string); ok { | ||
| config.LLMProvider = strings.ToLower(strings.TrimSpace(providerStr)) |
| const providerConfig = configureCodexProviderFromReflect({ | ||
| codexConfigPath: codexHome ? `${codexHome}/config.toml` : "", | ||
| reflectPath: AWF_REFLECT_OUTPUT_PATH, | ||
| provider: process.env.GH_AW_LLM_PROVIDER || "openai", |
| if (providerConfig.configured) { | ||
| codexEnv = { ...codexEnv, ...providerConfig.env }; | ||
| } |
| return normalized === provider; | ||
| }; | ||
|
|
||
| const matched = endpoints.find(ep => endpointProviderMatches(ep?.provider)) || endpoints[0]; |
| engine: | ||
| id: claude | ||
| model-provider: github | ||
| model: claude-haiku-4.5 |
| async function buildClaudeChildEnv() { | ||
| const childEnv = { ...process.env }; | ||
| const provider = normalizeReflectProviderName(process.env.GH_AW_LLM_PROVIDER, "anthropic"); | ||
| try { | ||
| const raw = fs.readFileSync(AWF_REFLECT_OUTPUT_PATH, "utf8"); |
|
@copilot please run the
|
All review comments have been addressed in local commits
All |
🤖 PR Triage — Run §29379921158
Score breakdown: impact 40 + urgency 20 + quality 18 Rationale: Large feature (+2131 lines) splitting engine provider selection from harness execution and routing Claude/Codex via reflected CAPI endpoints. Critical-path architectural change enabling new model routing. Not draft, CI reviewer commented. Needs expedited human review.
|
|
Smoke Test: Codex - 29386867095\n- [#45578] [docs] Self-healing documentation fixes from issue analysis - 2026-07-15\n- [#45575] feat: add gh-aw-detection: true to 4 flagged audit/analysis workflows\n- Overall: FAIL
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
|
Caution agentic threat detected DetailsThe threat detection engine failed to produce results. Review the workflow run logs for details. Pull request created: #45608
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Two new blocking issues found in the changed lines.
🔍 Findings summary
TOML corruption bug (high)
configureCodexProviderFromReflect in codex_harness.cjs uses a non-lookahead regex terminal group (?:\n\[|$) that consumes the opening [ of the next TOML section. The trimEnd()-based append branch then places that stolen [ immediately before the injected base_url line, producing invalid TOML. Triggered whenever the openai-proxy section lacks a pre-existing base_url key. The test suite misses this path entirely.
normalizeLLMProvider silent behavior change (medium)
Previously any unrecognized provider string fell back to LLMProviderAnthropic. After refactoring to a lookup-table pass-through, unknown strings are returned verbatim. Any workflow with a typo or previously-tolerated unknown value will now resolve differently with no error. Needs an explicit test and documentation.
Existing review comments (from prior passes) cover: schema additionalProperties blocking engine.provider, GH_AW_LLM_PROVIDER not reaching Codex, OpenAI validation conflict after proxy rewrite, endpoints[0] silent wrong-provider fallback, duplicate smoke workflow, and missing Claude harness test coverage.
🔎 Code quality review by PR Code Quality Reviewer · 130.3 AIC · ⌖ 4.69 AIC · ⊞ 5.6K
Comment /review to run again
Comments that could not be inline-anchored
actions/setup/js/codex_harness.cjs:388
TOML corruption when base_url key is absent from the openai-proxy section: the regex consumes the \n[ boundary that starts the next TOML section header; rewriting the config then injects an orphan [ into the output, corrupting the file.
<details>
<summary>💡 Details and fix</summary>
The regex:
const providerSectionPattern = /\[model_providers\.openai-proxy\][\s\S]*?(?:\n\[|$)/;is not a lookahead — it consumes \n[. When base_url is absent the else branch runs:…
pkg/workflow/llm_provider.go:438
normalizeLLMProvider no longer falls back to Anthropic for unknown providers: the previous implementation defaulted any unrecognized string to LLMProviderAnthropic; after this change unknown strings pass through as-is, silently changing behavior for any existing workflow with an unrecognized provider value.
<details>
<summary>💡 Details</summary>
Before:
switch strings.ToLower(strings.TrimSpace(provider)) {
case LLMProviderGitHub:
return LLMProviderGitHub
case LLMProviderOpe…
</details>
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
💥 [THE END] — Illustrated by Smoke Claude · 80.8 AIC · ⌖ 14.8 AIC · ⊞ 8.4K
Comment /smoke-claude to run again
| LLMProviderOpenAI = "openai" | ||
| ) | ||
|
|
||
| var llmProviderAliases = map[string]string{ |
There was a problem hiding this comment.
Nice extraction of the alias map — consider adding a brief comment documenting that keys must be lowercase since lookups normalize input.
|
|
||
| type llmProviderProfile struct { | ||
| id string | ||
| gatewayPort int |
There was a problem hiding this comment.
Returning the raw normalized value for unknown providers is a reasonable fallback — worth a test to confirm unexpected strings pass through unchanged.
|
Split engine provider selection from harness execution and route Claude/Codex via reflected CAPI endpoints
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
There was a problem hiding this comment.
Smoke validation review recorded.
📰 BREAKING: Report filed by Smoke Copilot · 100.4 AIC · ⌖ 4.62 AIC · ⊞ 19K
Comment /smoke-copilot to run again
Add label smoke to run again
|
Smoke tests: 1:✅,2:✅,3:❌,4:❌,5:❌,6:❌,7:❌,8:❌,9:❌,10:❌,11:❌,12:❌,13:❌,14:❌,15:❌. Overall: FAIL
|
|
@copilot resolve the merge conflicts on this branch. |
🔍 Sighthound Security Scan — PR #45490 (run 29387934559)102 actionable findings (outside test/testdata paths):
Top Findings by Severity🔴 Critical — Command Injection (11 findings, Go)Multiple
Remediation: Validate and sanitize all inputs used in command arguments. Prefer allowlists or structured APIs over shell construction. Ensure 🔴 Critical — Code Injection (17 findings, JS)Multiple Remediation: Audit 🟠 High — Path Traversal (7 findings)
Remediation: Resolve paths with 🟡 Medium — Unsafe Deserialization (55 findings, Go)
Remediation: Unmarshal into strict, well-defined structs. Avoid
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.
|
…-engine # Conflicts: # .github/workflows/agentic_commands.yml Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Resolved in |
This change separates inference-provider concerns from harness-runner concerns in the agentic engine flow, enabling Claude Code and Codex to run against GitHub Copilot CAPI-backed endpoints discovered at runtime. It also introduces explicit provider override semantics so workflows can select provider behavior without coupling to a specific harness implementation.
Provider override semantics (
engine.provider)engine.providerparsing as a first-class provider override for non-inline engines, while preserving existingengine.model-providercompatibility.engine.runtime+engine.providerobject behavior intact (id/auth/requestpath).Engine abstraction split (provider vs harness)
Known provider normalization
github,anthropic,openai) and mapped GitHub aliases consistently (includinggithub_models).Claude/Codex runtime endpoint routing from
/reflect/reflectpayloads.openai-proxybase_urland exporting corresponding env overrides.Targeted test coverage updates
engine.provideroverride behavior✨ PR Review Safe Output Test - Run 29386867182