-
Notifications
You must be signed in to change notification settings - Fork 467
feat: honor engine.version for copilot; add version tests for copilot and gemini #48519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
3fffeb9
plan: engine.version support for copilot/claude/codex/gemini
Copilot 42c5d3e
feat: honor engine.version for copilot; add version tests for gemini
Copilot 8b2a5fd
docs(adr): draft ADR-48519 for honoring engine.version in Copilot engine
github-actions[bot] f303c1b
plan: address review thread issues from pr-finisher run
Copilot 68f2556
fix: handle expression versions in copilotSupportsNoAskUser; add Gemi…
Copilot 2c35902
Merge branch 'main' into copilot/ensure-supported-engine-version
github-actions[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| # ADR-48519: Honor engine.version for Copilot CLI Installation | ||
|
|
||
| **Date**: 2026-07-28 | ||
| **Status**: Draft | ||
| **Deciders**: pelikhan, copilot-swe-agent | ||
|
|
||
| --- | ||
|
|
||
| ### Context | ||
|
|
||
| The `engine.version` field in workflow configuration allows users to pin the version of a coding engine (Copilot, Claude, codex, gemini) installed during workflow execution. Claude, codex, and gemini engines all honored this field correctly, using the user-specified value when present and falling back to a default pinned constant otherwise. However, the Copilot engine's `GetInstallationSteps` silently ignored `engine.version` and always installed `DefaultCopilotVersion`, logging only a message that it was ignoring the user-set value. This inconsistency made `engine.version` a no-op for Copilot, violating the principle of least surprise and breaking user expectations established by the other three engine implementations. | ||
|
|
||
| ### Decision | ||
|
|
||
| We will make the Copilot engine honor `engine.version` exactly as Claude, codex, and gemini do: when `EngineConfig.Version` is set, use it as the installation target; when it is absent, fall back to `DefaultCopilotVersion` and normalize `EngineConfig.Version` to that default so all downstream consumers in the same compile flow observe the effective installed value. The install script already skips `compat.json` resolution when an explicit version is provided, so no additional bypass logic is required. | ||
|
|
||
| ### Alternatives Considered | ||
|
|
||
| #### Alternative 1: Keep Copilot always pinned to DefaultCopilotVersion | ||
|
|
||
| The existing behavior guaranteed a stable, tested Copilot CLI version regardless of user configuration, which reduced risk of users accidentally installing untested or incompatible versions. This was rejected because it made `engine.version` silently ineffective for Copilot while working for all other engines, creating inconsistent and confusing behavior. The field's documented contract implies it controls the installed version across all engines. | ||
|
|
||
| #### Alternative 2: Introduce a Copilot-specific version override field | ||
|
|
||
| A dedicated field (e.g., `engine.copilot_version`) could allow Copilot-specific version control without changing the behavior of `engine.version`. This was rejected because it adds unnecessary API surface, conflicts with the established cross-engine convention of `engine.version`, and would require documentation and migration for any users who expect the unified field to work. | ||
|
|
||
| ### Consequences | ||
|
|
||
| #### Positive | ||
| - `engine.version` now behaves consistently across all four supported engines (Copilot, Claude, codex, gemini), fulfilling user expectations. | ||
| - Users can pin or override the Copilot CLI version in their workflow definitions, enabling version-locked reproducible builds or controlled rollouts of new CLI versions. | ||
| - Test coverage explicitly asserts the honored-version contract, preventing silent regression back to the ignore-version behavior. | ||
|
|
||
| #### Negative | ||
| - `compat.json` compatibility-matrix resolution is skipped when an explicit `engine.version` is provided, meaning users who specify an incompatible or unsupported version will receive no automated correction and may encounter runtime failures. | ||
| - Any workflow that previously set `engine.version` for Copilot expecting it to be ignored (e.g., to document an intent without changing behavior) will now have that version actually installed — a silent behavioral change at the call site. | ||
|
|
||
| #### Neutral | ||
| - `EngineConfig.Version` is still normalized to the effective installed version when no explicit version is set, preserving the existing contract for downstream consumers in the compile flow. | ||
| - The change is contained to `copilot_engine_installation.go`; the install script and `compat.json` integration path are unchanged. | ||
|
|
||
| --- | ||
|
|
||
| *ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* |
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in the latest commit.
copilotSupportsNoAskUsernow checkscontainsExpression(versionStr)before the semver gate and returnstruefor expression versions (e.g.${{ inputs.engine-version }}). The rationale mirrors how "latest" is handled: the value resolves at runtime, so we can't gate at compile time. The installer already handles expression versions correctly viaENGINE_VERSIONenv-var injection, ensuring the right binary is installed. Two new test cases were added toTestCopilotSupportsNoAskUserfor both input expression forms.