fix: gh aw upgrade wraps SHA-pinned uses: value in quotes, including inline comment#19679
fix: gh aw upgrade wraps SHA-pinned uses: value in quotes, including inline comment#19679
uses: value in quotes, including inline comment#19679Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
When upgrading copilot-setup-steps.yml to a SHA-pinned action reference (e.g. `org/repo@<sha> # vX.Y.Z`), the YAML marshaler quoted the entire string because '#' is a special YAML character. This caused GitHub Actions to treat the comment as part of the action ref, resulting in an unresolvable git ref error. Fix: add an `unquoteUsesValues` post-processing step after yaml.Marshal that strips double-quotes from `uses:` lines, restoring the '#...' suffix to a proper YAML comment. Regression test added: TestUpgradeCopilotSetupSteps_SHAPinnedNoQuotes Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
gh aw upgrade wraps uses value in quotesuses: value in quotes, including inline comment
|
@copilot ensure minimal formatting change is applied to the .yml. In particular do not marshal/unmarshall as it completely changes the structure of the file. Instead you need to rely on regular expression search and replace. |
Instead of parsing the workflow into a struct, marshaling back to YAML (which reformats the entire file), and post-processing quotes, directly operate on the raw file bytes using two targeted regex substitutions: 1. setupCliUsesPattern - replaces the uses: line (handles quoted/unquoted, SHA-pinned with comment, version-tag), compiled once at package level. 2. versionInWithPattern - replaces the version: value in the with: block immediately following, built at call time with the escaped new ref so only the correct step is touched (cannot be pre-compiled). Remove upgradeSetupCliVersion, unquoteUsesValues, usesQuotePattern and the now-unused errors/goccy-yaml imports. Update tests to exercise the new content-based helper. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 3dda0a5. The upgrade path now operates entirely on raw file bytes using two targeted regex substitutions — no 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:
|
|
@copilot add tests to ensure yaml is not shuffled or other comments are not lost |
Add TestUpgradeSetupCliVersionInContent_ExactPreservation which provides a rich workflow YAML containing: - top-level, job-level, and step-level comments - inline comments on individual fields (on:, uses:, run:, path filters) - multiple steps including one after the upgraded step - blank lines and extra with: parameters It constructs the expected output manually (identical to input except the two target lines) and compares with string equality, so any accidental shuffle, comment drop, or whitespace change causes a clear line-by-line diff in the failure output. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Added |
|
@copilot revert install.sh |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes gh aw upgrade rewriting uses: lines into quoted strings (including inline # comments) by switching from YAML re-marshal to targeted in-place edits, aiming to preserve workflow formatting and comments.
Changes:
- Replace struct-based YAML unmarshal/marshal upgrade with regex-based byte-preserving upgrade logic for
setup-clireferences. - Update/expand tests to validate SHA-pinned upgrades remain unquoted and that non-target YAML content is preserved exactly.
- Update
actions/setup-cli/install.shinstall path logic and bump versions / add job concurrency in a generated workflow lock file.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pkg/cli/copilot_setup.go |
Implements regex-based setup-cli uses: + with.version rewriting to avoid YAML reformatting/quote issues. |
pkg/cli/copilot_setup_test.go |
Reworks tests to validate unquoted SHA refs and exact byte preservation outside the two modified lines. |
actions/setup-cli/install.sh |
Changes gh extension install behavior/output handling during installation. |
.github/workflows/constraint-solving-potd.lock.yml |
Bumps embedded tool versions and adds job-level concurrency configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| // setupCliUsesPattern matches the uses: line for github/gh-aw/actions/setup-cli. | ||
| // It handles unquoted version-tag refs, unquoted SHA-pinned refs (with trailing comment), | ||
| // and quoted refs produced by some YAML marshalers (e.g. "...@sha # vX.Y.Z"). | ||
| var setupCliUsesPattern = regexp.MustCompile( | ||
| `(?m)^(\s+uses:[ \t]*)"?(github/gh-aw/actions/setup-cli@[^"\n]*)"?([ \t]*)$`) | ||
|
|
There was a problem hiding this comment.
setupCliUsesPattern won’t match a common quoting style where only the action ref is quoted and an inline comment follows outside the quotes, e.g. uses: "github/gh-aw/actions/setup-cli@<sha>" # vX.Y.Z. Because the pattern anchors to end-of-line right after the optional closing quote, this case will be skipped and the upgrade won’t run. Consider extending the regex to allow an optional #... comment after the closing quote (while still stripping the quotes around the ref).
| // Replace the version: value in the with: block immediately following the | ||
| // setup-cli uses: line. A combined multiline match is used so that only the | ||
| // version: parameter belonging to this specific step is updated. | ||
| // This pattern cannot be pre-compiled at package level because it embeds | ||
| // the runtime value newUses (which varies with version and resolver output). | ||
| escapedNewUses := regexp.QuoteMeta(newUses) | ||
| versionInWithPattern := regexp.MustCompile( | ||
| `(?s)(uses:[ \t]*` + escapedNewUses + `[^\n]*\n(?:[^\n]*\n)*?[ \t]+with:[ \t]*\n(?:[^\n]*\n)*?[ \t]+version:[ \t]*)(\S+)([ \t]*(?:\n|$))`) | ||
| updated = versionInWithPattern.ReplaceAll(updated, []byte("${1}"+version+"${3}")) | ||
|
|
There was a problem hiding this comment.
versionInWithPattern can span across subsequent steps because it allows arbitrary lines between the matched uses: line and the first later with:/version: block. If the setup-cli step lacks a with: block (or lacks version:), this regex can end up updating a different step’s with.version later in the job. Consider restricting the match to the current step boundary (e.g., stop before the next ^\s*-\s at the same indentation) or switch to a small line-based scan starting at the matched uses: line and only rewriting within that step’s indented block.
| // Replace the version: value in the with: block immediately following the | |
| // setup-cli uses: line. A combined multiline match is used so that only the | |
| // version: parameter belonging to this specific step is updated. | |
| // This pattern cannot be pre-compiled at package level because it embeds | |
| // the runtime value newUses (which varies with version and resolver output). | |
| escapedNewUses := regexp.QuoteMeta(newUses) | |
| versionInWithPattern := regexp.MustCompile( | |
| `(?s)(uses:[ \t]*` + escapedNewUses + `[^\n]*\n(?:[^\n]*\n)*?[ \t]+with:[ \t]*\n(?:[^\n]*\n)*?[ \t]+version:[ \t]*)(\S+)([ \t]*(?:\n|$))`) | |
| updated = versionInWithPattern.ReplaceAll(updated, []byte("${1}"+version+"${3}")) | |
| // Replace the version: value in the with: block that belongs to the same | |
| // step as the setup-cli uses: line. We perform a small line-based scan | |
| // starting at each matching uses: line and only rewrite within that step's | |
| // indented block, stopping before the next step at the same or lesser | |
| // indentation. | |
| updatedStr := string(updated) | |
| lines := strings.Split(updatedStr, "\n") | |
| for i := 0; i < len(lines); i++ { | |
| line := lines[i] | |
| if !strings.Contains(line, "uses:") || !strings.Contains(line, newUses) { | |
| continue | |
| } | |
| // Indentation (spaces/tabs) of the uses: line; used to detect step boundary. | |
| trimmed := strings.TrimLeft(line, " \t") | |
| usesIndent := len(line) - len(trimmed) | |
| // Scan forward within this step to find a with: block and its version: entry. | |
| withIndent := -1 | |
| for j := i + 1; j < len(lines); j++ { | |
| stepLine := lines[j] | |
| stepTrimmed := strings.TrimLeft(stepLine, " \t") | |
| stepIndent := len(stepLine) - len(stepTrimmed) | |
| // Stop when we reach the start of another step at the same or less indentation. | |
| if strings.HasPrefix(stepTrimmed, "- ") && stepIndent <= usesIndent { | |
| break | |
| } | |
| if withIndent == -1 { | |
| // Look for the with: line belonging to this step. | |
| if strings.HasPrefix(strings.TrimSpace(stepLine), "with:") { | |
| withIndent = stepIndent | |
| } | |
| continue | |
| } | |
| // Once inside the with: block, stop if indentation decreases to the with: level or less. | |
| if stepIndent <= withIndent { | |
| break | |
| } | |
| // Look for the version: line within the with: block. | |
| versionIdx := strings.Index(stepLine, "version:") | |
| if versionIdx == -1 { | |
| continue | |
| } | |
| prefix := stepLine[:versionIdx+len("version:")] | |
| rest := stepLine[versionIdx+len("version:"):] | |
| restTrim := strings.TrimLeft(rest, " \t") | |
| leadingWSLen := len(rest) - len(restTrim) | |
| leadingWS := rest[:leadingWSLen] | |
| // Identify the existing version token (first non-whitespace sequence) to replace. | |
| endOfToken := len(restTrim) | |
| for k, r := range restTrim { | |
| if r == ' ' || r == '\t' { | |
| endOfToken = k | |
| break | |
| } | |
| } | |
| tail := "" | |
| if endOfToken < len(restTrim) { | |
| tail = restTrim[endOfToken:] | |
| } | |
| lines[j] = prefix + leadingWS + version + tail | |
| // Only update the first version: for this uses: occurrence. | |
| break | |
| } | |
| } | |
| updated = []byte(strings.Join(lines, "\n")) |
yaml.Marshal/Unmarshalupgrade path with raw regex search-and-replaceupgradeSetupCliVersionInContent(regex-based, preserves file formatting)setupCliUsesPatternto package-level variable (pre-compiled)upgradeSetupCliVersion,unquoteUsesValues,usesQuotePatternTestUpgradeSetupCliVersionInContentTestUpgradeCopilotSetupSteps_SHAPinnedNoQuotesto verify content preservationTestUpgradeSetupCliVersionInContent_ExactPreservation— exact byte-level comparison verifying YAML comments, blank lines, field ordering, indentation, and unrelated steps are all unchangedactions/setup-cli/install.shto its original pre-PR stateOriginal prompt
gh aw upgradewrapsusesvalue in quotes, including the inline comment #19640🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.