Make BYOK Copilot defaults and deprecate byok-copilot flag#27902
Make BYOK Copilot defaults and deprecate byok-copilot flag#27902
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f89042c2-80c5-4973-aec9-228761032f45 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Generated by Design Decision Gate workflow run 24804504144.
|
Commit pushed:
|
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (146 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. 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: §24804504144
|
There was a problem hiding this comment.
Pull request overview
Updates the Copilot engine to use the previous “BYOK Copilot” behavior as the default (latest CLI install, non-empty model fallback, dummy API key injection, and default cli-proxy), deprecates the features.byok-copilot flag, and introduces a codemod to remove that flag from existing workflows.
Changes:
- Make Copilot always install Copilot CLI
latest, set a non-emptyCOPILOT_MODELfallback, and inject a dummyCOPILOT_API_KEYby default. - Enable
cli-proxyby default for Copilot workflows and update tests accordingly. - Deprecate
features.byok-copilotin constants/docs and add a codemod to remove it from frontmatter.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/model_env_vars_test.go | Updates expected COPILOT_MODEL fallback behavior in tests. |
| pkg/workflow/features_test.go | Adjusts feature-flag expectations for default cli-proxy behavior. |
| pkg/workflow/features.go | Changes cli-proxy enablement logic to be default-on for Copilot. |
| pkg/workflow/copilot_installer_test.go | Updates tests to assert pinned Copilot versions are ignored in favor of latest. |
| pkg/workflow/copilot_engine_test.go | Updates tests to assert dummy API key injection happens by default. |
| pkg/workflow/copilot_engine_installation.go | Forces Copilot CLI installation to latest and logs when a pinned version is ignored. |
| pkg/workflow/copilot_engine_execution.go | Always sets a non-empty model fallback and always injects dummy COPILOT_API_KEY. |
| pkg/workflow/awf_helpers_test.go | Updates AWF args tests for default cli-proxy behavior on Copilot. |
| pkg/constants/feature_constants.go | Marks ByokCopilotFeatureFlag as deprecated and updates its description. |
| pkg/constants/README.md | Documents byok-copilot as deprecated in the constants reference. |
| pkg/cli/fix_codemods_test.go | Updates codemod registry tests for the new codemod. |
| pkg/cli/fix_codemods.go | Registers the new features-byok-copilot-removal codemod. |
| pkg/cli/codemod_byok_copilot_test.go | Adds unit tests validating removal of features.byok-copilot. |
| pkg/cli/codemod_byok_copilot.go | Implements the codemod to remove features.byok-copilot. |
| docs/src/content/docs/reference/glossary.md | Updates glossary to deprecate byok-copilot and reflect new defaults. |
| docs/src/content/docs/reference/frontmatter.md | Updates frontmatter docs to describe Copilot BYOK behavior as the default. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 17/17 changed files
- Comments generated: 2
| // cli-proxy is enabled by default for copilot engine workflows. | ||
| // This is the default BYOK behavior and routes gh CLI access through | ||
| // the authenticated DIFC proxy. | ||
| if flag == constants.CliProxyFeatureFlag && | ||
| workflowData != nil && | ||
| workflowData.EngineConfig != nil && | ||
| strings.EqualFold(workflowData.EngineConfig.ID, string(constants.CopilotEngine)) && | ||
| isFeatureEnabled(constants.ByokCopilotFeatureFlag, workflowData) { | ||
| featuresLog.Print("cli-proxy implicitly enabled by byok-copilot feature flag") | ||
| strings.EqualFold(workflowData.EngineConfig.ID, string(constants.CopilotEngine)) { | ||
| featuresLog.Print("cli-proxy enabled by default for copilot engine") | ||
| return true | ||
| } |
There was a problem hiding this comment.
The early return that enables cli-proxy for Copilot runs happens before checking frontmatter/env, which contradicts the function contract (“frontmatter take precedence”) and makes features.cli-proxy: false ineffective for Copilot. Consider evaluating explicit frontmatter value first and only applying the Copilot default when the flag is not explicitly set; also consider falling back to workflowData.AI == "copilot" when EngineConfig is nil so Copilot defaults apply consistently across code paths that only set AI.
| // Copilot uses BYOK defaults by default, which always install latest. | ||
| copilotVersion := "latest" | ||
| if workflowData.EngineConfig != nil && workflowData.EngineConfig.Version != "" { | ||
| copilotVersion = workflowData.EngineConfig.Version | ||
| } | ||
| if isFeatureEnabled(constants.ByokCopilotFeatureFlag, workflowData) { | ||
| copilotVersion = "latest" | ||
| copilotInstallLog.Print("byok-copilot enabled: forcing Copilot CLI install version to latest") | ||
| copilotInstallLog.Printf("Ignoring pinned engine.version (%s): Copilot now installs latest by default", workflowData.EngineConfig.Version) | ||
| } |
There was a problem hiding this comment.
Copilot installation now always forces latest, but the pinned workflowData.EngineConfig.Version value is still propagated to the rest of the engine logic (e.g., copilotSupportsNoAskUser in copilot execution uses EngineConfig.Version to decide which CLI flags to emit). To avoid inconsistent behavior, consider normalizing the effective Copilot version in the workflow data (e.g., set EngineConfig.Version to latest or clear it) when it is being ignored so downstream logic reflects the actual installed version.
See below for a potential fix:
if workflowData.EngineConfig != nil {
if workflowData.EngineConfig.Version != "" {
copilotInstallLog.Printf("Ignoring pinned engine.version (%s): Copilot now installs latest by default", workflowData.EngineConfig.Version)
}
// Normalize the workflow data to the effective installed version so
// downstream logic that consults EngineConfig.Version behaves
// consistently with the actual Copilot CLI installation.
workflowData.EngineConfig.Version = copilotVersion
🧪 Test Quality Sentinel ReportTest Quality Score: 82/100✅ Excellent test quality
Test Classification DetailsView all 11 test classifications
Observations
|
|
@copilot review all comments |
1 similar comment
|
@copilot review all comments |
…ormalization Agent-Logs-Url: https://github.com/github/gh-aw/sessions/6fd2f4dc-5a0d-496f-8ed3-8317efc6a15b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/6fd2f4dc-5a0d-496f-8ed3-8317efc6a15b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/6fd2f4dc-5a0d-496f-8ed3-8317efc6a15b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
| copilotVersion = "latest" | ||
| copilotInstallLog.Print("byok-copilot enabled: forcing Copilot CLI install version to latest") | ||
| // Copilot uses BYOK defaults by default, which always install latest. | ||
| copilotVersion := "latest" |
There was a problem hiding this comment.
Based on the PR changes, it looks like the normalization (setting EngineConfig.Version = copilotVersion at line 81) addresses the downstream consistency concern. The pinned version is now overwritten with the actual installed version, so this code path should be self-consistent.
📰 BREAKING: Report filed by Smoke Copilot · ● 1.2M
… fallback Agent-Logs-Url: https://github.com/github/gh-aw/sessions/6fd2f4dc-5a0d-496f-8ed3-8317efc6a15b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Implemented and pushed the review fixes in commit e8f100c (plus follow-up commits on this branch): cli-proxy now honors explicit feature values before applying Copilot defaults, includes AI fallback when EngineConfig is nil, and normalizes effective Copilot version to latest for downstream consistency.
Implemented and pushed the review fixes in commit e8f100c (plus follow-up commits on this branch): cli-proxy now honors explicit feature values before applying Copilot defaults, includes AI fallback when EngineConfig is nil, and normalizes effective Copilot version to latest for downstream consistency. |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/6fd2f4dc-5a0d-496f-8ed3-8317efc6a15b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot remove the behavior "always install Copilot CLI latest". The copilot-cli is pinned to the version in constants. |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/489753e4-512f-459f-8c16-36c792bf0677 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilo recompile |
|
Hey
|
|
@copilot recompile |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/7ce097b9-ed6b-4d15-b14e-847821c7c52a Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Recompiled and pushed in 5f7363a.
Recompiled and pushed in 5f7363a. 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:
|
|
🚀 Smoke OpenCode MISSION COMPLETE! OpenCode delivered. 🔥 |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
|
|
|
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
Agent Container Tool Check
Result: 11/12 tools available — FAIL (dotnet missing)
|
Smoke Test Summary — Run §24812777769PR: Make BYOK Copilot defaults and deprecate byok-copilot flag
Overall: ✅ PASS
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed the BYOK Copilot defaults PR. The changes look well-structured — normalizing the effective Copilot install version in workflow data and injecting a dummy API key ensures consistent downstream behavior. Lock file updates are generated and consistent.
📰 BREAKING: Report filed by Smoke Copilot · ● 1.2M
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
There was a problem hiding this comment.
💥 Automated smoke test review - all systems nominal! This is an automated review from the Claude engine smoke test (Run 24812777802). The BYOK Copilot defaults changes look well-structured.
💥 [THE END] — Illustrated by Smoke Claude · ● 181.8K
| -- /bin/bash -c 'GH_AW_NODE_EXEC="${GH_AW_NODE_BIN:-}"; if [ -z "$GH_AW_NODE_EXEC" ] || [ ! -x "$GH_AW_NODE_EXEC" ]; then GH_AW_NODE_EXEC="$(command -v node 2>/dev/null || echo node)"; fi; "$GH_AW_NODE_EXEC" ${RUNNER_TEMP}/gh-aw/actions/copilot_driver.cjs /usr/local/bin/copilot --add-dir /tmp/gh-aw/ --log-level all --log-dir /tmp/gh-aw/sandbox/agent/logs/ --disable-builtin-mcps --no-ask-user --allow-all-tools --allow-all-paths --add-dir "${GITHUB_WORKSPACE}" --prompt-file /tmp/gh-aw/aw-prompts/prompt.txt' 2>&1 | tee -a /tmp/gh-aw/agent-stdio.log | ||
| env: | ||
| COPILOT_AGENT_RUNNER_TYPE: STANDALONE | ||
| COPILOT_API_KEY: dummy-byok-key-for-offline-mode |
There was a problem hiding this comment.
🤖 Smoke Test Review Comment #1 (Run 24812777802) — Good addition of COPILOT_API_KEY as dummy BYOK key for offline mode. This ensures the Copilot engine can initialize correctly without needing a real API key in BYOK configurations.
| env: | ||
| COPILOT_AGENT_RUNNER_TYPE: STANDALONE | ||
| COPILOT_API_KEY: dummy-byok-key-for-offline-mode | ||
| COPILOT_GITHUB_TOKEN: ${{ github.token }} |
There was a problem hiding this comment.
🤖 Smoke Test Review Comment #2 (Run 24812777802) — The fallback claude-sonnet-4.6 model is a sensible default for BYOK Copilot. Consider documenting this default in the BYOK configuration guide so users know which model is used when no model is explicitly configured.
Summary
constants.DefaultCopilotVersion(do not forcelatest)COPILOT_MODELfallback (claude-sonnet-4.6)COPILOT_API_KEYlocalby default); do not implicitly enablecli-proxyfor Copilot workflows.features.byok-copilotin constants/docs.features-byok-copilot-removalthat removesfeatures.byok-copilotwhen present.Tests
go test -v -run "Test(IsFeatureEnabledWithData|BuildAWFArgsCliProxy|CopilotInstallerCustomVersion|CopilotInstallerExpressionVersion_ViaEngineConfig|CopilotInstallerByokFeatureStillUsesDefaultPinnedVersion|CopilotEngineSetsDummyAPIKeyByDefault|CopilotFallbackModelMapsToNativeEnvVar)" ./pkg/workflowgo test -v -run "Test(ByokCopilotFeatureRemovalCodemod|GetAllCodemods_ReturnsAllCodemods|GetAllCodemods_InExpectedOrder|GetAllCodemods_ContainsExpectedCodemods)" ./pkg/climake agent-finishfails on pre-existing unrelated testifylint issues inpkg/agentdrain/spec_test.go(assert.Empty/assert.Len suggestions).Validation
parallel_validationrun completed:✨ PR Review Safe Output Test - Run 24812777802