Update OpenCode/Crush universal LLM consumer backend handling#27708
Merged
Update OpenCode/Crush universal LLM consumer backend handling#27708
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c3384ed2-c196-4275-90f3-f4323d9a34ea Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c3384ed2-c196-4275-90f3-f4323d9a34ea Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
pelikhan
April 21, 2026 22:41
View session
pelikhan
approved these changes
Apr 21, 2026
Contributor
|
✅ Smoke CI completed successfully! |
Contributor
|
🤖 Smoke CI check completed for this pull request. Run URL: https://github.com/github/gh-aw/actions/runs/24751430059 |
Contributor
🧪 Test Quality Sentinel ReportTest Quality Score: 91/100✅ Excellent test quality
Test Classification DetailsView All Test Classifications (7 tests)
Flagged Tests — Requires ReviewNo tests flagged. All new and modified tests are clean. Build TagsAll four changed test files correctly carry Inflation Note
Language SupportTests analyzed:
Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators. References: §24751429891
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
✅ Test Quality Sentinel: 91/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All 7 new/modified tests verify behavioral contracts (correct secret routing per LLM provider, model validation errors, end-to-end compilation failures). No guideline violations detected.
Generated by Design Decision Gate workflow run 24751429896. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Commit pushed:
|
Contributor
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (370 new lines in AI has analyzed the PR diff and generated a draft ADR to help you get started: 📄 Draft ADR: What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. Draft ADR SummaryThe generated draft captures the following key points from the diff:
Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
References: §24751429896
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors OpenCode/Crush to share a “universal LLM consumer” backend implementation that derives the native backend (Copilot/Anthropic/Codex(OpenAI)) from a provider-qualified engine.model, and updates compilation + tests to enforce and exercise the new behavior.
Changes:
- Added a shared
UniversalLLMConsumerEnginethat resolvesengine.modelprovider prefixes and applies backend-specific env/secret behavior (including firewall base URL envs). - Updated OpenCode and Crush engines to embed the shared implementation, and updated tests/loader tests to require
engine.model(provider/model) for these engines. - Added compiler validation to enforce
engine.modelpresence/format for OpenCode/Crush; updated smoke lock output for Crush (Anthropic secrets/env).
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/universal_llm_consumer_engine.go | New shared backend/profile resolver + shared secret/env helpers for OpenCode/Crush. |
| pkg/workflow/opencode_engine.go | Switch OpenCode to shared universal consumer behavior for secrets/env. |
| pkg/workflow/crush_engine.go | Switch Crush to shared universal consumer behavior for secrets/env. |
| pkg/workflow/compiler_orchestrator_tools.go | Invokes new compiler validation for universal consumer model requirements. |
| pkg/workflow/agent_validation.go | Adds validateUniversalLLMConsumerModel to enforce provider/model and supported providers. |
| pkg/workflow/agent_validation_model_test.go | New unit tests for the universal consumer model validation behavior. |
| pkg/workflow/opencode_engine_test.go | Updates firewall assertions and adds provider-specific secret expectation tests. |
| pkg/workflow/crush_engine_test.go | Adds provider-specific secret expectation tests and updates firewall assertions. |
| pkg/workflow/engine_definition_loader_test.go | Updates string-form engine tests to expect compile errors for OpenCode/Crush without model. |
| .github/workflows/smoke-crush.lock.yml | Regenerated lock file reflecting Anthropic backend env/secrets for the smoke workflow. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 11/11 changed files
- Comments generated: 2
Comment on lines
+125
to
+129
| if hasGitHubTool(workflowData.ParsedTools) { | ||
| secrets = append(secrets, "GITHUB_MCP_SERVER_TOKEN") | ||
| } | ||
|
|
||
| headerSecrets := collectHTTPMCPHeaderSecrets(workflowData.Tools) |
There was a problem hiding this comment.
GetUniversalRequiredSecretNames will panic if it is ever called with workflowData == nil because it unconditionally dereferences workflowData.ParsedTools and workflowData.Tools. Either guard these accesses (if workflowData != nil && ...) or treat nil as empty Tools/ParsedTools to keep engine methods safe to call in minimal/diagnostic contexts.
Suggested change
| if hasGitHubTool(workflowData.ParsedTools) { | |
| secrets = append(secrets, "GITHUB_MCP_SERVER_TOKEN") | |
| } | |
| headerSecrets := collectHTTPMCPHeaderSecrets(workflowData.Tools) | |
| var parsedTools []ParsedWorkflowTool | |
| var tools []WorkflowTool | |
| if workflowData != nil { | |
| parsedTools = workflowData.ParsedTools | |
| tools = workflowData.Tools | |
| } | |
| if hasGitHubTool(parsedTools) { | |
| secrets = append(secrets, "GITHUB_MCP_SERVER_TOKEN") | |
| } | |
| headerSecrets := collectHTTPMCPHeaderSecrets(tools) |
Contributor
There was a problem hiding this comment.
Smoke test agent here — this is a valid concern. The nil guard would make GetUniversalRequiredSecretNames safer to call in minimal/diagnostic contexts. +1 on the suggestion!
📰 BREAKING: Report filed by Smoke Copilot · ● 1.2M
Comment on lines
14
to
15
| // Crush is a provider-agnostic, open-source AI coding agent that supports | ||
| // 75+ models via BYOK (Bring Your Own Key). |
There was a problem hiding this comment.
The header comment says Crush supports “75+ models via BYOK”, but this PR adds compiler validation that restricts engine.model to a small provider allowlist (copilot/anthropic/openai/codex). Please update the comment to reflect the supported providers in gh-aw (or clarify that Crush CLI supports more models but gh-aw currently supports a subset).
Suggested change
| // Crush is a provider-agnostic, open-source AI coding agent that supports | |
| // 75+ models via BYOK (Bring Your Own Key). | |
| // Crush CLI is a provider-agnostic, open-source AI coding agent with broader | |
| // BYOK (Bring Your Own Key) support, but gh-aw currently supports a subset of | |
| // providers for engine.model validation: copilot, anthropic, openai, and codex. |
Collaborator
|
@copilot Review all comments |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5c464fe3-b681-44c9-b763-5e07fd7d335b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5c464fe3-b681-44c9-b763-5e07fd7d335b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Reviewed and addressed the actionable review threads. I fixed nil-safety in universal secret collection (including nil Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Collaborator
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/6e5351b8-7539-4922-b782-fb4713e68d0e Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Fixed. The build failure was caused by Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Collaborator
|
@copilot use copilot/gpt-5.3-codex for smoke-opencode |
Contributor
|
|
Contributor
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
Contributor
|
🚀 Smoke OpenCode MISSION COMPLETE! OpenCode delivered. 🔥 |
Contributor
|
|
Contributor
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
Contributor
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
Copilot stopped work on behalf of
pelikhan due to an error
April 22, 2026 01:15
Contributor
Agent Container Tool Check
Result: 12/12 tools available ✅ Overall Status: PASS
|
Contributor
|
🤖 Smoke test §24754915298 for PR #27708 by
Overall: ✅ PASS —
|
Contributor
Comment MemoryTests run in silence Green checkmarks bloom like flowers Bugs fear the pipeline
|
Contributor
There was a problem hiding this comment.
Smoke test review — overall this PR looks clean! The UniversalLLMConsumerEngine refactor nicely consolidates provider routing logic for both OpenCode and Crush. Minor suggestion on trimming model part in validation.
📰 BREAKING: Report filed by Smoke Copilot · ● 1.2M
| // Crush is a provider-agnostic, open-source AI coding agent with broader BYOK | ||
| // (Bring Your Own Key) support, but gh-aw currently supports a subset of | ||
| // providers for engine.model validation: copilot, anthropic, openai, and codex. | ||
| type CrushEngine struct { |
Contributor
There was a problem hiding this comment.
The embedding of UniversalLLMConsumerEngine (replacing BaseEngine) is a clean refactor that avoids code duplication across OpenCode and Crush. LGTM!
| model = strings.TrimSpace(model) | ||
| if model == "" { | ||
| return "", errors.New("for universal consumer engines (OpenCode/Crush), engine.model is required and must use provider/model format (supported providers: copilot, anthropic, openai, codex)") | ||
| } |
Contributor
There was a problem hiding this comment.
Nice validation logic here! The resolveUniversalLLMBackendFromModel function cleanly parses and validates the provider/model format. Consider also trimming the model part (parts[1]) before validation for consistency with the provider trimming above.
Contributor
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
Contributor
|
Smoke Test 24754915256 — Claude Engine
Overall: PARTIAL — 1 failed (#10 MCP unavailable), 2 skipped
|
Contributor
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal!
💥 [THE END] — Illustrated by Smoke Claude · ● 222.8K
| return fmt.Errorf("invalid engine.model for engine '%s': %w", engine.GetID(), err) | ||
| } | ||
|
|
||
| return nil |
Contributor
There was a problem hiding this comment.
The validateUniversalLLMConsumerModel function is clean and well-structured. Consider adding a brief doc comment explaining that this validation runs at compile time, so users see clear errors before deploying workflows. This would help distinguish it from runtime errors.
| - Compiled workflow lock files (`.lock.yml`) are regenerated to reflect the new secret names and environment variables, which will require safe-update approval gates for secrets like `ANTHROPIC_API_KEY`. | ||
| - The `CrushLLMGatewayPort` constant is no longer referenced directly in the Crush engine; gateway port selection is now driven by the backend profile returned from `getUniversalLLMBackendProfile`. | ||
|
|
||
| --- |
Contributor
There was a problem hiding this comment.
The ADR clearly articulates the "Negative" consequences (structural coupling, engine.model becoming required). It may be worth noting a mitigation path for the breaking change — for example, adding a compiler warning (rather than error) for existing workflows that omit engine.model, with a grace period before making it a hard error.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
engine.modelprovider prefixes to native backends (copilot,anthropic,codex/openai)engine.modelfor OpenCode/Crush and validate provider/model format and supported providersValidation
make fmt✅go test -v -run "TestBuiltinEngineStringFormInjection|TestValidateUniversalLLMConsumerModel|TestOpenCodeEngine|TestCrushEngine" ./pkg/workflow/go test -v -run "TestValidateUniversalLLMConsumerModel|TestOpenCodeEngineProviderProfiles|TestCrushEngine" ./pkg/workflow/parallel_validationrun:make agent-finish❌ currently fails inmake recompiledue safe-update approval gate for a new restricted secret (ANTHROPIC_API_KEY) insmoke-crushcompilation outputSmoke CI run completed. Run URL: https://github.com/github/gh-aw/actions/runs/24751430059
Smoke CI run completed. Run URL: https://github.com/github/gh-aw/actions/runs/24753019093
✨ PR Review Safe Output Test - Run 24754915256