Skip to content

Fix DefaultCopilotVersion toolcache bypass caused by compat.json drift - #48593

Open
pelikhan with Copilot wants to merge 16 commits into
mainfrom
copilot/fix-default-copilot-version-drifting
Open

Fix DefaultCopilotVersion toolcache bypass caused by compat.json drift#48593
pelikhan with Copilot wants to merge 16 commits into
mainfrom
copilot/fix-default-copilot-version-drifting

Conversation

Copilot AI commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

DefaultCopilotVersion (1.0.75) exceeded the compat.json max-agent (1.0.56), which caused install_copilot_cli.sh to skip compat resolution entirely when passed an explicit version—meaning the 1.0.56 runner toolcache entry was always rejected via exact-match, forcing a network download on every agentic job.

Root causes

  • compat.json max-agent was stale relative to DefaultCopilotVersion
  • The install script unconditionally skipped compat resolution when $VERSION was set, leaving COMPAT_MATCHED_MIN/MAX_AGENT empty
  • find_cached_copilot_bin used exact-match-only when a version was explicit (continue past the range check), so a compatible toolcache entry was always rejected
  • GH_AW_COMPILED_VERSION was never emitted in agent/detection job envs, so the install script had no compiled_version to resolve a compat window even if it wanted to

Changes

.github/aw/compat.json

  • max-agent: 1.0.561.0.75

actions/setup/sh/install_copilot_cli.sh

  • Accept MAJOR.MINOR.PATCH (with or without v) in GH_AW_COMPILED_VERSION validation — c.version in the compiler is "0.83.1" not "v0.83.1"
  • Run compat resolution even when $VERSION is explicit, populating COMPAT_MATCHED_MIN/MAX_AGENT for range-based toolcache matching
  • In find_cached_copilot_bin: when exact match fails but a compat range is available, fall through to the range check instead of skipping — lets cached 1.0.56 satisfy an explicit request for 1.0.75 when both are within the compat window
# Before: exact match only when VERSION is explicit
echo "  Skipping candidate (version mismatch: want ${requested_version_normalized}, got ${candidate_version_normalized})" >&2
continue   # range check below is dead code

# After: fall through to range check when compat window is available
if [ -z "$min_version" ] && [ -z "$max_version" ]; then
  echo "  Skipping candidate (version mismatch ...)" >&2
  continue
fi
echo "  No exact match; checking compat range ${min_version}..${max_version}" >&2
# fall through

pkg/workflow/compiler_main_job_helpers.go / threat_detection_job.go

  • Emit GH_AW_COMPILED_VERSION: <version> in job-level env for released builds, enabling the install script to perform compat resolution and populate the range for toolcache matching

pkg/constants/version_constants_test.go

  • TestDefaultCopilotVersionWithinCompatWindow: reads .github/aw/compat.json, finds the open compat row, and asserts min-agent ≤ DefaultCopilotVersion ≤ max-agent — CI gate to prevent this class of drift from recurring

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 12.3 AIC · ⌖ 5.94 AIC · ⊞ 7.2K ·
Comment /souschef to run again


Run: https://github.com/github/gh-aw/actions/runs/30381484988

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 11.6 AIC · ⌖ 6.08 AIC · ⊞ 7.2K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · gpt54 · 7.87 AIC · ⌖ 5.82 AIC · ⊞ 7.2K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · gpt54 · 15.8 AIC · ⌖ 7.75 AIC · ⊞ 7.2K ·
Comment /souschef to run again


Run: https://github.com/github/gh-aw/actions/runs/30395881970

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 9.56 AIC · ⌖ 6.17 AIC · ⊞ 7.2K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · gpt54 · 8.29 AIC · ⌖ 7.59 AIC · ⊞ 7.2K ·
Comment /souschef to run again


run: https://github.com/github/gh-aw/actions/runs/30401927723

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 10.6 AIC · ⌖ 5.55 AIC · ⊞ 7.2K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · gpt54 · 6.2 AIC · ⌖ 5.29 AIC · ⊞ 7.6K ·
Comment /souschef to run again


Run: https://github.com/github/gh-aw/actions/runs/30446544004

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 9.46 AIC · ⌖ 5.69 AIC · ⊞ 7.6K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · gpt54 · 4.58 AIC · ⌖ 6.46 AIC · ⊞ 7.8K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · gpt54 · 12.2 AIC · ⌖ 6.24 AIC · ⊞ 7.8K ·
Comment /souschef to run again

… range fallback, emit GH_AW_COMPILED_VERSION in job envs

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix DefaultCopilotVersion drift past compat.json max-agent Fix DefaultCopilotVersion toolcache bypass caused by compat.json drift Jul 28, 2026
Copilot AI requested a review from pelikhan July 28, 2026 12:37
@pelikhan
pelikhan marked this pull request as ready for review July 28, 2026 12:38
Copilot AI review requested due to automatic review settings July 28, 2026 12:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes Copilot CLI toolcache bypasses caused by compatibility-window drift and missing compiler-version context.

Changes:

  • Updates the supported Copilot CLI range.
  • Enables compatibility-range matching for cached installations.
  • Emits compiler versions and adds drift-prevention tests.
Show a summary per file
File Description
.github/aw/compat.json Extends the Copilot compatibility window.
actions/setup/sh/install_copilot_cli.sh Adds explicit-version range fallback.
pkg/constants/version_constants_test.go Guards against version drift.
pkg/workflow/compiler_main_job_helpers.go Exposes compiler version to agent jobs.
pkg/workflow/threat_detection_job.go Exposes compiler version to detection jobs.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 5/5 changed files
  • Comments generated: 3
  • Review effort level: Medium

Comment thread actions/setup/sh/install_copilot_cli.sh Outdated
Comment thread actions/setup/sh/install_copilot_cli.sh Outdated
Comment thread pkg/constants/version_constants_test.go
@github-actions github-actions Bot mentioned this pull request Jul 28, 2026
@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Design Decision Gate 🏗️ completed the design decision gate check.

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Test Quality Sentinel completed test quality analysis.

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

PR Code Quality Reviewer completed the code quality review.

@github-actions

Copy link
Copy Markdown
Contributor

Test Quality Sentinel Report - Score: 100/100 Excellent - 2 design tests, 0 violations. Both tests are behavioral_contract/high_value/design_test. Verdict: PASSED (0% implementation tests, threshold 30%)

🧪 Test quality analysis by Test Quality Sentinel · sonnet46 · 37 AIC · ⌖ 6.06 AIC · ⊞ 8.1K ·
Comment /review to run again

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Test Quality Sentinel: 100/100. 0% implementation tests (threshold: 30%). Both tests are strong behavioral contracts enforcing design invariants (compat.json window and Playwright cooldown). No violations.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: Fix DefaultCopilotVersion toolcache bypass

The fix correctly addresses the root cause (stale compat.json max-agent + exact-match-only toolcache lookup). Overall approach is sound. Three pre-existing Copilot review comments cover the notable non-blocking issues:

  1. Explicit version becomes a soft hint — any workflow pinning engine.version: X will now silently accept a compat-range match when GH_AW_COMPILED_VERSION is present.
  2. TTL exemption still applies to explicit requests — a stale toolcache entry won't be expired when requested_version != latest.
  3. open bool default in test — Go JSON unmarshaling maps an omitted open field to false; the test would miss any future open row relying on the schema default.

No blocking issues found. The CI gate test (TestDefaultCopilotVersionWithinCompatWindow) is a good addition to prevent drift from recurring.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 33.8 AIC · ⌖ 5.68 AIC · ⊞ 5K

Draft ADR capturing the decision to run compat-matrix resolution
unconditionally (even for explicit version requests) so the runner
toolcache can satisfy Copilot CLI installs without a network download.
@github-actions

Copy link
Copy Markdown
Contributor

🏗️ Design Decision Gate — ADR Required

This PR makes significant changes to core business logic (125 new lines in pkg/ directories) but does not have a linked Architecture Decision Record (ADR).

📄 Draft ADR committed: docs/adr/48593-enable-compat-range-toolcache-for-explicit-copilot-version.md — review and complete it before merging.

🔒 This PR cannot merge until an ADR is linked in the PR body.

📋 What to do next
  1. Review the draft ADR committed to your branch — it was generated from the PR diff
  2. Complete the missing sections — add context the AI could not infer, refine the decision rationale, and list real alternatives you considered
  3. Commit the finalized ADR to docs/adr/ on your branch
  4. Reference the ADR in this PR body by adding a line such as:

    ADR: ADR-48593: Enable Compat-Range Toolcache Matching for Explicit Copilot CLI Version Requests

Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision.

❓ Why ADRs Matter

"AI made me procrastinate on key design decisions. Because refactoring was cheap, I could always say 'I'll deal with this later.' Deferring decisions corroded my ability to think clearly."

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 Reference

An ADR must contain these four sections to be considered complete:

  • Context — What is the problem? What forces are at play?
  • Decision — What did you decide? Why?
  • Alternatives Considered — What else could have been done?
  • Consequences — What are the trade-offs (positive and negative)?

All ADRs are stored in docs/adr/ as Markdown files numbered by PR number (e.g., 48593-enable-compat-range-toolcache-for-explicit-copilot-version.md for PR #48593).

🏗️ ADR gate enforced by Design Decision Gate 🏗️ · sonnet46 · 59.2 AIC · ⌖ 13.1 AIC · ⊞ 8.5K ·
Comment /review to run again

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skills-Based Review 🧠

Applied /diagnosing-bugs and /tdd. The fix correctly addresses all four root causes identified in the PR description and adds a solid CI gate. A few follow-up items worth addressing before merge.

📋 Key Themes & Highlights

Key Themes

  • open bool decode (existing comment #3665567402): json.Unmarshal maps omitted openfalse; the test skips rows where open == false, so a schema-valid row without an explicit open: true is silently ignored — the CI gate would pass even if the version drifted outside that row's range
  • Soft-hint concern (existing comment #3665567324): the fallthrough from exact-match to range check means any workflow pinned to a specific explicit version can silently install a different (compatible) version; the behaviour change is intentional but deserves an integration test
  • TTL exemption gap (existing comment #3665567363): the TTL bypass for explicit versions is not updated to account for the new range-match path
  • Wildcard max-agent (new comment): semverCmp will panic-via-Fatal if max-agent is ever "*"; add a guard
  • IsRelease() nil check (new comment): if env == nil in buildMainJobEnv is dead code; worth a comment to explain the dev/release asymmetry

Positive Highlights

  • ✅ All four root causes are identified and addressed in a single coherent change
  • TestDefaultCopilotVersionWithinCompatWindow is an excellent CI gate — exactly the right kind of regression prevention
  • parseSemver / semverCmp are clean, self-contained, and easy to test in isolation
  • ✅ Shell changes preserve backward-compatibility: when no compat window is available, exact-match still applies

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 44.9 AIC · ⌖ 4.93 AIC · ⊞ 6.7K
Comment /matt to run again

Comment thread pkg/constants/version_constants_test.go Outdated
Comment thread pkg/workflow/compiler_main_job_helpers.go

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST_CHANGES

The fix correctly addresses the reported toolcache bypass, but the new explicit-version compat-range fallthrough introduces a regression that silently disables cache freshness checks.

💡 Themes and highlights

Blocking:

  • install_copilot_cli.sh: the fallthrough that lets an explicit version match a compat-range candidate bypasses the TTL-expiry check entirely, because that check is guarded by requested_version = "latest". This means an explicit-version install can now silently reuse an arbitrarily old cached binary within the compat window with no freshness check at all — the opposite of the intended fix's rigor.

Non-blocking / lower priority:

  • version_constants_test.go: parseSemver rejects any prerelease-suffixed version (e.g. 1.0.75-beta.1) despite the compat.json schema explicitly permitting one, which would turn a schema-valid config into a confusing test failure rather than a real compat violation.
  • A prior reviewer already flagged the Open bool JSON-decode default mismatch (schema says open defaults to true when omitted, but the Go struct defaults to false) — worth fixing alongside the above.
  • The compat.json bump and GH_AW_COMPILED_VERSION env-injection changes themselves look correct and are consistent between compiler_main_job_helpers.go and threat_detection_job.go.

Recommend extending the TTL check to also apply on the explicit-version + range-match path before merging.

🔎 Code quality review by PR Code Quality Reviewer · sonnet50 · 73.8 AIC · ⌖ 4.4 AIC · ⊞ 7.4K
Comment /review to run again

Comments that could not be inline-anchored

actions/setup/sh/install_copilot_cli.sh:431

This fallthrough lets an explicit-version request match a stale cached binary within the compat range, but the TTL-expiry check below only fires when requested_version = &quot;latest&quot; — so explicit-version installs now silently accept an expired cache entry with zero freshness check.

pkg/constants/version_constants_test.go:179

parseSemver hard-fails on any prerelease-suffixed version (e.g. 1.0.75-beta.1), even though compat.schema.json's semver definition explicitly permits a -prerelease suffix.

@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot run pr-finisher skill

…comment, default-pin-only range fallback

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@copilot review changes and generate a table of app compat scenarios and their outcome

Reviewed against the default-pin/exact-pin contract; no additional issues found.

Scenario Cache behavior Fallback outcome
Release build, omitted engine.version Compiler adds --compat-range; highest fresh cached version in 1.0.21..* is accepted Downloads pinned default 1.0.75 if no cache matches
Release build, explicit engine.version: 1.0.75 Exact 1.0.75 match only Downloads 1.0.75
Release build, other explicit version Exact requested version only; in-range mismatches are rejected Downloads requested version
Development build, omitted version Exact default-version match only Downloads pinned default
Fresh cache below min-agent Rejected Uses another match or downloads
Fresh cache within open range Accepted; highest matching version wins No download
Expired cache within range Rejected by 14-day TTL Uses another match or downloads
Wrong architecture or non-numeric cached version Rejected Uses another match or downloads
Compat lookup unavailable for compiler default Falls back to exact default-version matching Downloads pinned default if absent
Script invoked without a version and valid compiler context Resolves open max-agent: "*" to latest; range cache matching applies Downloads latest release if absent

Targeted shell and Go regression tests pass.

@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot remove the release behavior specific --compat-range stuff. The script should favor the use provided engine.version, then the compat range, then finally the defaultg engine version.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@copilot remove the release behavior specific --compat-range stuff. The script should favor the use provided engine.version, then the compat range, then finally the defaultg engine version.

Fixed in 4389225. Removed --compat-range; explicit engine.version remains an exact pin, otherwise the installer checks the compat window for a fresh cached CLI and falls back to DefaultCopilotVersion. Regressions and regenerated lock files are included.

@github-actions

Copy link
Copy Markdown
Contributor

PR Triage

  • Category: bug (toolcache/version drift fix)
  • Risk: medium
  • Priority score: 80/100 (impact 40, urgency 22, quality 18)
  • Recommendation: fast_track

Fixes a real toolcache-bypass bug (compat.json drift) forcing network downloads of Copilot CLI on every agentic job — meaningful cost/reliability impact. Mergeable, non-draft, Test Quality Sentinel 100/100 (2 design tests, 0 violations). Author has responded to review feedback (simplified installer precedence per maintainer request) and touches compiler/install-script/CI-gate paths together. CI: combined status is pending with several action_required checks — verify these clear (may just be non-required integrations) before merge. Recommend fast-track review given the CI-cost fix is broadly impactful.

Generated by 🔧 PR Triage Agent · aut00 · 47.6 AIC · ⌖ 5.15 AIC · ⊞ 7.6K ·

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, address unresolved review feedback, and rerun checks once the branch is up to date.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 10 AIC · ⌖ 6.62 AIC · ⊞ 7.8K ·
Comment /souschef to run again

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, address unresolved review feedback, and rerun checks once the branch is up to date.

...

Completed in 0757e87. All review threads are resolved, the branch is up to date with main, and local formatting, lint, full Go/integration tests, workflow recompilation, final repository validation, secret scanning, code review, and CodeQL checks pass. The remaining GitHub checks are approval-gated/stale and require a maintainer re-trigger after this agent push.

Copilot AI requested a review from gh-aw-bot July 29, 2026 16:06
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, confirm the PR is fully ready for maintainer review, and rerun any required checks if the branch update changed the head. The latest author follow-up says review threads are resolved and the branch is up to date; please verify there are no remaining unresolved review items or stale blockers before handing off.

Run: https://github.com/github/gh-aw/actions/runs/30471606180

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 4.58 AIC · ⌖ 6.46 AIC · ⊞ 7.8K ·
Comment /souschef to run again

Copilot AI and others added 2 commits July 29, 2026 17:11
…opilot-version-drifting

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, confirm the PR is fully ready for maintainer review, and rerun any required checks if the branch update changed the head. The latest author follow-up says ...

Completed in 04561cc. All review threads remain resolved, latest main is merged, the generated workflow lock is refreshed, and the branch is clean/pushed. make test and make recompile pass; targeted final review found no issues. CI must be re-triggered by a maintainer for the updated head. The only local gate issue is an unrelated lint finding already present on main in pkg/cli/forecast_compliance_fixtures_formal_test.go:443.

@github-actions

Copy link
Copy Markdown
Contributor

Triage: bug (toolcache/version drift fix)

Risk: high | Priority: high (score 71/100)

Breakdown: impact 35/50 (fixes toolcache bypass forcing network downloads on every agentic job across 201 files, mostly regenerated lockfiles) · urgency 24/30 (mergeable_state blocked, active discussion with 21 comments/12 review comments, real CI-cost impact) · quality 12/20 (clear root-cause writeup, but currently blocked and needs conflict/review resolution)

Recommended action: fast_track

High-value fix for a real CI cost/performance bug, but PR is currently blocked (mergeable_state) with substantial review activity — needs human attention to unblock and merge promptly.

Generated by 🔧 PR Triage Agent · aut00 · 40.7 AIC · ⌖ 5.25 AIC · ⊞ 7.9K ·

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill and confirm the PR is ready for maintainer review. The latest author follow-up says all review threads are resolved and the branch is up to date, but the PR is still blocked. Please verify there are no remaining unresolved review items or stale blockers, and rerun any required checks if the head changes.

Run: https://github.com/github/gh-aw/actions/runs/30481139487

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 12.2 AIC · ⌖ 6.24 AIC · ⊞ 7.8K ·
Comment /souschef to run again

…opilot-version-drifting

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill and confirm the PR is ready for maintainer review. The latest author follow-up says all review threads are resolved and the branch is up to date, but the PR...

Completed in 2e9d4bc. All six review threads remain resolved, latest main is merged, and the branch is clean and pushed. make fmt, impacted unit tests, full make test, and make recompile pass. The only local gate issue is the unrelated lint finding already present on main at pkg/cli/forecast_compliance_fixtures_formal_test.go:443; approval-gated CI must be re-triggered for the new head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DefaultCopilotVersion drifts past compat.json max-agent, forcing a network install on every job and disabling the toolcache path

4 participants