fix: enforce three-level version priority for Copilot CLI install - #49116
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates Copilot CLI installation to defer unpinned version selection to runtime compatibility resolution, with a baked-in fallback.
Changes:
- Propagates the compiler version into Copilot installation steps.
- Adds fallback behavior and tests for unavailable compatibility data.
- Recompiles generated workflow locks without explicit default-version arguments.
Show a summary per file
| File | Description |
|---|---|
actions/setup/sh/install_copilot_cli.sh |
Adds fallback version resolution. |
pkg/cli/install_copilot_cli_test.go |
Tests fallback installation. |
pkg/workflow/copilot_engine.go |
Updates implementation reference. |
pkg/workflow/copilot_engine_installation.go |
Revises version-selection flow. |
pkg/workflow/copilot_installer.go |
Generates deferred installer steps. |
pkg/workflow/copilot_installer_test.go |
Tests version-priority behavior. |
pkg/workflow/compiler_yaml_ai_execution.go |
Propagates compiler version. |
pkg/workflow/workflow_data.go |
Stores compiler version. |
.github/skills/agentic-workflows/SKILL.md |
Removes unavailable references. |
.github/workflows/*.lock.yml (266 files) |
Regenerates Copilot install steps. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 192/192 changed files
- Comments generated: 5
- Review effort level: Medium
| type WorkflowData struct { | ||
| Name string | ||
| WorkflowID string // workflow identifier derived from markdown filename (basename without extension) | ||
| CompiledVersion string // gh-aw compiler version (e.g. "v0.72.5"); injected by the Compiler into generated install steps as GH_AW_COMPILED_VERSION so the install script can resolve a compat.json window at runtime |
There was a problem hiding this comment.
Fixed in b45099b: both buildThreatDetectionWorkflowData (threat_detection_external.go:98) and buildEvalsEngineSteps (evals_steps.go:217) now copy CompiledVersion: data.CompiledVersion into their synthetic WorkflowData, so their Copilot install steps correctly inject GH_AW_COMPILED_VERSION.
| // Normalize EngineConfig.Version to the compile-time default so downstream | ||
| // consumers (OTel, GH_AW_INFO_VERSION, copilotSupportsNoAskUser, …) have a | ||
| // consistent effective value. The install script may install a different | ||
| // version within the compat window, but that is intentional. | ||
| workflowData.EngineConfig.Version = string(constants.DefaultCopilotVersion) |
There was a problem hiding this comment.
Fixed in b45099b: the normalization mutation is removed. copilotVersion initialises to "" and the existing fallback helpers (getVersionForSetup, getInstallationVersion) handle defaults without mutating EngineConfig.Version, so downstream threat-detection/eval config clones see the unset state and enter compat resolution as intended.
| // compiledVersion should be the gh-aw compiler version string (e.g. "v0.72.5"). Pass "" when | ||
| // the compiler version is unavailable (the script falls back to priority 3 in that case). | ||
| func GenerateCopilotInstallerSteps(version, stepName string, rootless bool, compiledVersion string) []GitHubActionStep { |
There was a problem hiding this comment.
Fixed in b45099b: the expression branch in copilot_installer.go (lines 49–51) now appends GH_AW_COMPILED_VERSION: compiledVersion when compiledVersion != "", matching the guard already applied in the empty-version branch.
| # DEFAULT_COPILOT_VERSION is the baked-in fallback used when neither an explicit version | ||
| # argument nor a GH_AW_COMPILED_VERSION-backed compat.json lookup is available. | ||
| # It is the last resort (priority 3) after engine.version (priority 1) and | ||
| # compat.json toolcache lookup (priority 2). | ||
| DEFAULT_COPILOT_VERSION="1.0.75" |
There was a problem hiding this comment.
Fixed in b45099b: scripts/sync-compat.sh now reads DEFAULT_COPILOT_VERSION directly from install_copilot_cli.sh (line 52–54) and validates or updates it alongside DefaultCopilotVersion, eliminating independent drift.
| # Version resolution follows this priority order: | ||
| # 1. Explicit VERSION argument (from engine.version in the workflow) | ||
| # 2. Compat.json toolcache lookup (requires GH_AW_COMPILED_VERSION to select the right window) | ||
| # 3. DEFAULT_COPILOT_VERSION baked into this script |
There was a problem hiding this comment.
Fixed in b45099b: the script header at lines 510–521 now documents the three-level priority order (explicit arg → compat.json + toolcache → DEFAULT_COPILOT_VERSION) so direct callers see the accurate behaviour contract.
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. |
|
✅ Test Quality Sentinel completed test quality analysis. Warning threat detection engine error DetailsThe threat detection engine failed to produce results. Review the workflow run logs for details. |
|
✅ PR Code Quality Reviewer completed the code quality review. Warning threat detection engine error DetailsThe threat detection engine failed to produce results. Review the workflow run logs for details. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ Warning threat detection engine error DetailsThe threat detection engine failed to produce results. Review the workflow run logs for details. |
|
Warning threat detection engine error DetailsThe threat detection engine failed to produce results. Review the workflow run logs for details. Test Quality Sentinel ReportTest Quality Score: 80/100 — Excellent Analyzed 5 test(s): 5 design, 0 implementation, 0 violation(s). Both test files have correct build tags. No mock library violations.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.
Details
The threat detection engine failed to produce results.
Review the workflow run logs for details.
Review: fix: enforce three-level version priority for Copilot CLI install
The core approach is correct and well-structured. The three-level priority (explicit engine.version → compat.json window → baked-in default) is properly enforced, the exit 1 removal is safe because priority 3 covers the gap, and all 266 lock files are consistently regenerated.
One blocking issue found:
The expression passthrough branch in pkg/workflow/copilot_installer.go (line 47) does not inject GH_AW_COMPILED_VERSION even when compiledVersion is non-empty. When a workflow passes engine.version: ${{ inputs.engine-version }} and the expression evaluates to empty string at runtime, the install script cannot enter the compat.json lookup path (priority 2) because GH_AW_COMPILED_VERSION is absent — it falls straight to priority 3. The version == "" branch already applies the correct guard; the same pattern needs to be applied in the expression branch.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 51.8 AIC · ⊞ 5.3K
| @@ -44,6 +47,23 @@ func GenerateCopilotInstallerSteps(version, stepName string, rootless bool) []Gi | |||
| return []GitHubActionStep{GitHubActionStep(stepLines)} | |||
There was a problem hiding this comment.
Missing GH_AW_COMPILED_VERSION injection in the expression branch
When version is a GitHub Actions expression (e.g. ${{ inputs.engine-version }}), the generated step does not inject GH_AW_COMPILED_VERSION even when compiledVersion is available. If the expression evaluates to an empty string at runtime, the install script immediately falls through to the baked-in default (priority 3), bypassing the compat.json window lookup (priority 2) entirely.
The version == "" branch (priority-2/3 path) correctly guards with:
if compiledVersion != "" {
stepLines = append(stepLines, " GH_AW_COMPILED_VERSION: "+compiledVersion)
}The same guard should be added to the expression branch so compat lookup still fires when ENGINE_VERSION resolves empty at runtime:
if compiledVersion != "" {
stepLines = append(stepLines, " GH_AW_COMPILED_VERSION: "+compiledVersion)
}
return []GitHubActionStep{GitHubActionStep(stepLines)}@copilot please address this.
There was a problem hiding this comment.
Fixed in b45099b: copilot_installer.go lines 49–51 add the same compiledVersion != "" guard to the expression branch, injecting GH_AW_COMPILED_VERSION alongside ENGINE_VERSION so compat.json resolution fires when the expression evaluates to empty at runtime.
…iority Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (207 new lines in business-logic directories, threshold: 100) but does not have a linked Architecture Decision Record (ADR). Draft ADR committed:
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
|
There was a problem hiding this comment.
Warning
threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.
Details
The threat detection engine failed to produce results.
Review the workflow run logs for details.
Skills-Based Review 🧠
Applied /diagnosing-bugs — requesting changes on propagation gaps and a second independent constant that will cause silent regressions.
📋 Key Themes & Highlights
Key Themes
-
CompiledVersionnot propagated to syntheticWorkflowDatainstances — bothbuildThreatDetectionWorkflowData(threat_detection_external.go) andbuildEvalsEngineSteps(evals_steps.go) create freshWorkflowDatastructs without copyingCompiledVersion. Those jobs will always hit compat-resolution without the compiled version context, defeating priority 2. -
Expression-version branch silently drops
compiledVersion— whenengine.versionis a GitHub Actions expression (${{ inputs.engine-version }}), the generated step never injectsGH_AW_COMPILED_VERSION. If the expression resolves to empty at runtime, compat lookup runs blind. -
Two independent
DEFAULT_COPILOT_VERSIONconstants —pkg/constants.DefaultCopilotVersionis tracked bysync-compat.sh; the newDEFAULT_COPILOT_VERSIONbaked intoinstall_copilot_cli.shis not. These will drift silently unless a sync mechanism is added. -
Unnecessary
EngineConfig.Versionmutation — settingworkflowData.EngineConfig.Version = DefaultCopilotVersionwhen no explicit version is supplied is observed by later clones (detection, evals), pushing a stale default into jobs that should also use compat resolution.
Positive Highlights
- ✅ Three-level priority is clearly documented in both Go and shell
- ✅ New regression test (
TestInstallCopilotCLIScriptFallsBackToBakedInDefaultWhenCompatUnavailable) exercises the exact fallback path that previouslyexit 1-ed - ✅
GH_AW_COMPILED_VERSIONinjection logic is correct for the common (non-expression, no-version) branch - ✅ The
sync-compat.shreference is a good model for the new constant sync requirement
The five existing inline comments already identify these issues precisely. Requesting changes until propagation to detection/evals data and the expression-branch injection are addressed, and the DEFAULT_COPILOT_VERSION sync gap is resolved.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 63.2 AIC · ⊞ 7K
Comment /matt to run again
There was a problem hiding this comment.
Warning
threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.
Details
The threat detection engine failed to produce results.
Review the workflow run logs for details.
Request changes: critical version-resolution bug remains unaddressed
All issues independently found in this pass (by both my own read and a second-pass sub-agent) are exact duplicates of the 5 existing unresolved review comments already on this PR from Copilot. No new comments are being added to avoid duplication, but none of these appear fixed in the current diff — they should block merge.
💡 Outstanding blocking issues (already commented, still present in code)
copilot_engine_installation.goline 165 (critical) — Whenengine.versionis unset, the code normalizesworkflowData.EngineConfig.Version = string(constants.DefaultCopilotVersion)on the shared mainWorkflowData. SincebuildJobsbuilds the main job first, this mutation is visible to the laterbuildEvalsEngineSteps/threat-detection config clones (a shallow copy ofdata.EngineConfig). Those clones then see a non-emptyVersionand pass it as an explicit positional arg toGenerateCopilotInstallerSteps, completely bypassing compat.json resolution and the new fallback logic for the evals/threat-detection jobs — defeating the entire point of this PR for those jobs.workflow_data.go/threat_detection_external.go(high) —CompiledVersionis never copied into the syntheticWorkflowDatabuilt bybuildThreatDetectionWorkflowDataor theevalsDataliteral inbuildEvalsEngineSteps. Combined with #1, both install-step generators for non-main jobs are broken with respect to the new priority scheme.copilot_installer.goline 21 (medium) — The expression-pattern branch ignores the newcompiledVersionparameter; if the expression resolves to empty at runtime, the script losesGH_AW_COMPILED_VERSIONand silently falls to the baked-in default with no signal at compile time.install_copilot_cli.shline 34 (medium) —DEFAULT_COPILOT_VERSION="1.0.75"is a second, independently-maintained copy ofpkg/constants.DefaultCopilotVersion.scripts/sync-compat.shonly validates the Go constant againstcompat.json; it has no check against this shell copy, so the two can silently drift.install_copilot_cli.shline 514 (low) — Usage header still documents the omitted-VERSION default as "latest release"; actual behavior is now compat-resolution then baked-in fallback.
Recommend fixing #1 and #2 together (they compound into the same defect) before merge, since they silently negate the compat-resolution guarantee for evals and threat-detection jobs — the exact regression this PR claims to fix for the main job.
🔎 Code quality review by PR Code Quality Reviewer · auto · 112.4 AIC · ⊞ 7.8K
Comment /review to run again
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
🚀 Smoke Gemini MISSION COMPLETE! Gemini has spoken. ✨ |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
📰 BREAKING: Smoke Copilot - AOAI (apikey) is now investigating this pull request. Sources say the story is developing... |
|
✅ Smoke Copilot Small completed successfully! |
|
✅ Smoke OTEL completed successfully! |
|
📰 BREAKING: Smoke Copilot - AOAI (Entra) is now investigating this pull request. Sources say the story is developing... |
|
✅ Smoke Copilot MAI completed successfully! |
Agent Container Tool Check
Result: 12/12 tools available ✅ Overall status: PASS
|
|
Version hidden, layers three, Caption: Inspired by the PR's three-tier fallback logic for resolving the Copilot CLI version.
|
Smoke Test Results
Overall Status: FAIL
|
|
Smoke test summary: PASS
|
|
Smoke test PRs: #49150, #49149 Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
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. Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
💥 Smoke Test Results — Run 30576615184Core tests #1–12: ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ Overall: PARTIAL — All non-skipped tests passed ✅ Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
Smoke Test: Copilot - Run 30577084701PR: "fix: enforce three-level version priority for Copilot CLI install" ✅ github/gh-proxy · ❌ mcpscripts-gh · ❌ serena find_symbol · ✅ playwright · ✅ web-fetch · ✅ file+bash · ✅ discussion comment · ✅ build · ✅ artifact upload · ✅ create discussion · ✅ dispatch workflow · ✅ PR review tools · ✅ comment memory · ✅ sub-agent · ✅ check run · ✅ LSP function count Overall status: FAIL (2/16 failed) Author: Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
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. Warning Firewall blocked 6 domainsThe following domains were blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
|
There was a problem hiding this comment.
Smoke test review. Me poke code. All good.
Warning
Firewall blocked 6 domains
The following domains were blocked by the firewall during workflow execution:
accounts.google.comandroid.clients.google.comclients2.google.comcontentautofill.googleapis.comsafebrowsingohttpgateway.googleapis.comwww.google.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "accounts.google.com"
- "android.clients.google.com"
- "clients2.google.com"
- "contentautofill.googleapis.com"
- "safebrowsingohttpgateway.googleapis.com"
- "www.google.com"See Network Configuration for more information.
📰 BREAKING: Report filed by Smoke Copilot · auto · 55.3 AIC · ⌖ 4.83 AIC · ⊞ 8.6K
Comment /smoke-copilot to run again
Add label smoke to run again
|
🎉 This pull request is included in a new release. Release: |
The Go compiler was always passing
DefaultCopilotVersionas an explicit positional arg toinstall_copilot_cli.sh, even when noengine.versionwas set — completely bypassingcompat.jsonresolution and the toolcache lookup.Priority order enforced:
engine.versionset → positional arg → script uses directlyengine.version→ no arg →GH_AW_COMPILED_VERSIONenv →compat.jsonwindow → toolcacheDEFAULT_COPILOT_VERSIONbaked into script (replacesexit 1)pkg/workflow/copilot_installer.goGenerateCopilotInstallerSteps(version, stepName string, rootless bool, compiledVersion string)GH_AW_COMPILED_VERSION) / explicit version argpkg/workflow/copilot_engine_installation.gocopilotVersioninitializes to""instead ofDefaultCopilotVersionworkflowData.CompiledVersionforGH_AW_COMPILED_VERSIONinjectionpkg/workflow/compiler_yaml_ai_execution.godata.CompiledVersion = c.versionbeforeengine.GetInstallationSteps(data)pkg/workflow/workflow_data.goCompiledVersion stringfield toWorkflowDataactions/setup/sh/install_copilot_cli.shDEFAULT_COPILOT_VERSION="1.0.75"constantexit 1in compat fallback with graceful fallback toDEFAULT_COPILOT_VERSIONAll 266
.lock.ymlfiles recompiled; install steps no longer contain a hardcoded version whenengine.versionis unset.Warning
threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.
Details
The threat detection engine failed to produce results.
Review the workflow run logs for details.
Warning
threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.
Details
The threat detection engine failed to produce results.
Review the workflow run logs for details.
Warning
threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.
Details
The threat detection engine failed to produce results.
Review the workflow run logs for details.
Warning
threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.
Details
The threat detection engine failed to produce results.
Review the workflow run logs for details.
run: https://github.com/github/gh-aw/actions/runs/30563574734