Restore compat-aware Copilot toolcache fallback for explicit pins #48507
There are no files selected for viewing
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 |
|---|---|---|
|
|
@@ -199,7 +199,7 @@ download_compat_json() { | |
| local source_file="$2" | ||
|
|
||
| echo "Attempting to download compatibility matrix from ${COMPAT_URL}..." >&2 | ||
| if curl -fsSL --retry 6 --retry-delay 5 --retry-all-errors -o "$compat_file" "$COMPAT_URL"; then | ||
| echo "$COMPAT_URL" > "$source_file" | ||
| echo "Successfully downloaded compatibility matrix from ${COMPAT_URL}" >&2 | ||
| return 0 | ||
|
|
@@ -368,6 +368,7 @@ find_cached_copilot_bin() { | |
| local candidate_version_normalized="" | ||
| local best_candidate="" | ||
| local best_version="" | ||
| local used_requested_fallback=false | ||
|
|
||
| echo "Searching toolcache for GitHub Copilot CLI (requested: ${requested_version}, arch: ${ARCH_NAME}, range: ${min_version:-none}..${max_version:-none})..." >&2 | ||
| if [ -n "$cache_ttl_days" ]; then | ||
|
|
@@ -422,8 +423,12 @@ find_cached_copilot_bin() { | |
| printf '%s\n' "$candidate" | ||
| return 0 | ||
| fi | ||
| if [ -z "$min_version" ] && [ -z "$max_version" ]; then | ||
| echo " Skipping candidate (version mismatch: want ${requested_version_normalized}, got ${candidate_version_normalized})" >&2 | ||
| continue | ||
| fi | ||
| echo " Exact version mismatch (want ${requested_version_normalized}, got ${candidate_version_normalized}); checking compat window fallback" >&2 | ||
| used_requested_fallback=true | ||
| fi | ||
|
|
||
| if [ -n "$min_version" ] && version_is_greater "$min_version" "$candidate_version_normalized"; then | ||
|
|
@@ -461,6 +466,9 @@ find_cached_copilot_bin() { | |
| done | ||
|
|
||
| if [ -n "$best_candidate" ]; then | ||
| if [ "$used_requested_fallback" = "true" ] && [ -n "$requested_version_normalized" ]; then | ||
| echo " Falling back to compat-range cached version: ${best_version} (requested ${requested_version_normalized})" >&2 | ||
| fi | ||
| echo " Selected best cached version: ${best_version} at ${best_candidate}" >&2 | ||
| printf '%s\n' "$best_candidate" | ||
| return 0 | ||
|
|
@@ -503,22 +511,36 @@ EOF | |
| TEMP_DIR=$(mktemp -d) | ||
| trap 'rm -rf "$TEMP_DIR"' EXIT | ||
|
|
||
| # Resolve compat metadata before toolcache lookup so explicit version requests can | ||
| # still prefer a cached CLI inside the published compatibility window. | ||
| if [ -n "$COMPILED_GH_AW_VERSION" ]; then | ||
|
Comment on lines
+514
to
+516
|
||
| if RESOLVED_COMPAT_INFO="$(resolve_version_from_compat "$COMPILED_GH_AW_VERSION" "${TEMP_DIR}/compat.json")"; then | ||
|
Comment on lines
+516
to
517
|
||
| IFS='|' read -r RESOLVED_COMPAT_VERSION COMPAT_MATCHED_MIN_AGENT COMPAT_MATCHED_MAX_AGENT COMPAT_CACHE_TTL_DAYS <<< "$RESOLVED_COMPAT_INFO" | ||
| if [ -z "$VERSION" ]; then | ||
| echo "No explicit Copilot CLI version requested. Attempting compat-driven version resolution..." | ||
| VERSION="$RESOLVED_COMPAT_VERSION" | ||
| REQUESTED_VERSION="latest" | ||
| echo "Using compat-resolved Copilot CLI window: ${COMPAT_MATCHED_MIN_AGENT}..${COMPAT_MATCHED_MAX_AGENT}" | ||
| echo "Will install compat max-agent ${VERSION} if no cached version satisfies the window." | ||
| else | ||
| echo "Explicit Copilot CLI version argument provided (${VERSION}); using compatibility window ${COMPAT_MATCHED_MIN_AGENT}..${COMPAT_MATCHED_MAX_AGENT} for toolcache lookup." | ||
| fi | ||
| elif [ -z "$VERSION" ]; then | ||
| echo "ERROR: Failed to resolve Copilot CLI version from compatibility matrix." >&2 | ||
| echo "ERROR: Cannot install without a compatible version." >&2 | ||
| echo "To fix: Pass an explicit version as an argument (e.g., 'install_copilot_cli.sh 1.0.56')" >&2 | ||
| echo " or ensure GH_AW_COMPILED_VERSION matches a row in .github/aw/compat.json" >&2 | ||
| exit 1 | ||
| else | ||
| echo "Explicit Copilot CLI version argument provided (${VERSION}); compatibility matrix unavailable; exact-match toolcache lookup only." | ||
| fi | ||
| elif [ -z "$VERSION" ]; then | ||
| echo "No explicit Copilot CLI version requested. Attempting compat-driven version resolution..." | ||
| echo "ERROR: Failed to resolve Copilot CLI version from compatibility matrix." >&2 | ||
| echo "ERROR: Cannot install without a compatible version." >&2 | ||
| echo "To fix: Pass an explicit version as an argument (e.g., 'install_copilot_cli.sh 1.0.56')" >&2 | ||
| echo " or ensure GH_AW_COMPILED_VERSION matches a row in .github/aw/compat.json" >&2 | ||
| exit 1 | ||
|
Contributor
There was a problem hiding this comment. [/diagnosing-bugs] The refactored 💡 SuggestionExtract the no-version error into a small helper function, e.g. die_no_version() {
echo 'ERROR: Failed to resolve Copilot CLI version from compatibility matrix.' >&2
echo 'ERROR: Cannot install without a compatible version.' >&2
echo "To fix: Pass an explicit version as an argument (e.g., 'install_copilot_cli.sh 1.0.56')" >&2
echo ' or ensure GH_AW_COMPILED_VERSION matches a row in .github/aw/compat.json' >&2
exit 1
}@copilot please address this. |
||
| else | ||
| echo "Explicit Copilot CLI version argument provided (${VERSION}); skipping compat matrix resolution." | ||
| fi | ||
|
|
@@ -558,11 +580,11 @@ CHECKSUMS_URL="${BASE_URL}/SHA256SUMS.txt" | |
|
|
||
| # Download checksums | ||
| echo "Downloading checksums from ${CHECKSUMS_URL}..." | ||
| curl -fsSL --retry 6 --retry-delay 5 --retry-all-errors -o "${TEMP_DIR}/SHA256SUMS.txt" "${CHECKSUMS_URL}" | ||
|
|
||
| # Download binary tarball | ||
| echo "Downloading binary from ${TARBALL_URL}..." | ||
| curl -fsSL --retry 6 --retry-delay 5 --retry-all-errors -o "${TEMP_DIR}/${TARBALL_NAME}" "${TARBALL_URL}" | ||
|
|
||
| # Verify checksum | ||
| echo "Verifying SHA256 checksum for ${TARBALL_NAME}..." | ||
|
|
||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -173,7 +173,6 @@ exit 97 | |
| assert.Contains(t, string(curlLogContent), "/compat.json", "compat matrix should be downloaded") | ||
| assert.NotContains(t, string(curlLogContent), "SHA256SUMS.txt", "release downloads should not run when toolcache is hit") | ||
|
|
||
| curlLines := strings.Split(strings.TrimSpace(string(curlLogContent)), "\n") | ||
| compatFetches := 0 | ||
| for _, line := range curlLines { | ||
|
|
@@ -184,6 +183,95 @@ exit 97 | |
| assert.Equal(t, 1, compatFetches, "compat.json should be fetched exactly once (no double fallback)") | ||
| } | ||
|
|
||
| func TestInstallCopilotCLIScriptExplicitVersionFallsBackToCompatToolcache(t *testing.T) { | ||
| const explicitVersion = "1.0.75" | ||
| const compatMinVersion = "1.0.21" | ||
| const compatMaxVersion = "1.0.56" | ||
|
|
||
| wd, err := os.Getwd() | ||
| require.NoError(t, err, "Failed to get working directory") | ||
|
|
||
| projectRoot := filepath.Join(wd, "..", "..") | ||
| installScript := filepath.Join(projectRoot, "actions", "setup", "sh", "install_copilot_cli.sh") | ||
|
|
||
| tempDir := t.TempDir() | ||
| toolcacheBin := filepath.Join(tempDir, "toolcache", "copilot-cli", compatMaxVersion, "x64", "bin") | ||
| require.NoError(t, os.MkdirAll(toolcacheBin, 0o755)) | ||
| require.NoError(t, os.WriteFile(filepath.Join(toolcacheBin, "copilot"), []byte("#!/usr/bin/env bash\necho 'copilot "+compatMaxVersion+"'\n"), 0o755)) | ||
|
|
||
| fakeBinDir := filepath.Join(tempDir, "fake-bin") | ||
| require.NoError(t, os.MkdirAll(fakeBinDir, 0o755)) | ||
|
|
||
| curlLog := filepath.Join(tempDir, "curl.log") | ||
| sudoScript := filepath.Join(fakeBinDir, "sudo") | ||
| curlScript := filepath.Join(fakeBinDir, "curl") | ||
|
|
||
| require.NoError(t, os.WriteFile(sudoScript, []byte(`#!/usr/bin/env bash | ||
| if [ "${1:-}" = "chown" ]; then | ||
| exit 0 | ||
| fi | ||
| exec "$@" | ||
| `), 0o755)) | ||
| require.NoError(t, os.WriteFile(curlScript, []byte(`#!/usr/bin/env bash | ||
| set -euo pipefail | ||
| output_file="" | ||
| url="" | ||
| while [ "$#" -gt 0 ]; do | ||
| case "$1" in | ||
| -o) | ||
| output_file="$2" | ||
| shift 2 | ||
| ;; | ||
| *) | ||
| url="$1" | ||
| shift | ||
| ;; | ||
| esac | ||
| done | ||
| echo "$url" >> "`+curlLog+`" | ||
| if [[ "$url" == *"/compat.json" ]]; then | ||
|
Contributor
There was a problem hiding this comment. [/tdd] The fake 💡 SuggestionAdd a check that the expected curl flags appear in the log, or make the fake curl echo its full echo "$@" >> "$(curlLog)"Then assert: assert.Contains(t, string(curlLogContent), "--retry-all-errors")@copilot please address this. |
||
| cat > "$output_file" <<'JSON' | ||
| { | ||
| "agent-compat-v1": { | ||
| "copilot": [ | ||
| { | ||
| "min-gh-aw": "0.72.0", | ||
| "max-gh-aw": "*", | ||
| "min-agent": "`+compatMinVersion+`", | ||
| "max-agent": "`+compatMaxVersion+`" | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| JSON | ||
| exit 0 | ||
| fi | ||
| echo "unexpected URL: $url" >&2 | ||
| exit 97 | ||
| `), 0o755)) | ||
|
|
||
| githubPath := filepath.Join(tempDir, "github-path") | ||
| cmd := exec.Command("bash", installScript, explicitVersion) | ||
| cmd.Env = append(os.Environ(), | ||
| "RUNNER_TOOL_CACHE="+filepath.Join(tempDir, "toolcache"), | ||
| "GITHUB_PATH="+githubPath, | ||
| "GH_AW_COMPILED_VERSION=v0.83.1", | ||
| "PATH="+fakeBinDir+":"+os.Getenv("PATH"), | ||
| ) | ||
|
|
||
| output, err := cmd.CombinedOutput() | ||
| require.NoError(t, err, "install_copilot_cli.sh should prefer compat toolcache for explicit versions: %s", output) | ||
|
|
||
| assert.Contains(t, string(output), "Explicit Copilot CLI version argument provided ("+explicitVersion+"); using compatibility window "+compatMinVersion+".."+compatMaxVersion+" for toolcache lookup.") | ||
| assert.Contains(t, string(output), "Falling back to compat-range cached version: "+compatMaxVersion+" (requested "+explicitVersion+")") | ||
| assert.Contains(t, string(output), "Using cached GitHub Copilot CLI") | ||
|
Contributor
There was a problem hiding this comment. [/tdd] The new test covers the happy path (explicit version → compat window hit → toolcache used). It doesn't cover the negative case: explicit version requested, compat window resolved, but no toolcache entry within that window. That path should fall through to a network download; without a test it's easy to accidentally break. 💡 SuggestionAdd a companion test @copilot please address this. |
||
|
|
||
| curlLogContent, err := os.ReadFile(curlLog) | ||
| require.NoError(t, err) | ||
| assert.Contains(t, string(curlLogContent), "/compat.json") | ||
| assert.NotContains(t, string(curlLogContent), "SHA256SUMS.txt", "release downloads should not run when compat fallback hits toolcache") | ||
| } | ||
|
|
||
| func TestInstallCopilotCLIScriptRootlessModeUsesRealScriptWithToolcacheAndNoSudo(t *testing.T) { | ||
| wd, err := os.Getwd() | ||
| require.NoError(t, err, "Failed to get working directory") | ||
|
|
||
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.
[/diagnosing-bugs]
used_requested_fallbackis set whenever any version-mismatched candidate is encountered, even if that candidate is later filtered out by the min/max window check. The log message at line 469 then fires for any cached miss, potentially misleading operators into thinking a fallback occurred when it did not.💡 Suggestion
Only log the fallback message if the selected
best_candidatewas genuinely reached via the compat-range path after an explicit-version mismatch. One approach: capture the version that triggered the flag and compare againstbest_versionbefore emitting the message.@copilot please address this.