Add version tag comments to pinned GitHub Actions#3848
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Modified GetActionPin and GetActionPinWithData to append version comments - Added unquoteUsesWithComments function to handle YAML marshalling quotes - Updated tests to expect new format with version comments - All pinned actions now show format: uses: slug@sha # version Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot merge main and lint |
There was a problem hiding this comment.
Pull Request Overview
This PR adds version tag comments to pinned GitHub Actions to improve readability and maintainability. When actions are pinned to SHA commits for security, a comment is added showing the original version tag (e.g., uses: actions/checkout@sha # v5).
Key Changes
- Modified
GetActionPin()andGetActionPinWithData()to append version comments to SHA references - Added
unquoteUsesWithComments()function to handle YAML marshaller's quoting behavior - Updated tests to expect the new comment format
Reviewed Changes
Copilot reviewed 68 out of 80 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| pkg/workflow/action_pins.go | Modified pin functions to append # <version> comments to action SHAs |
| pkg/workflow/agentic_engine.go | Added post-processing to remove quotes from uses values with comments |
| pkg/workflow/action_pins_test.go | Updated test expectations to include version comments in assertions |
| .github/workflows/*.lock.yml | Regenerated workflow files with new pinned action format including version comments |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -935,11 +935,11 @@ jobs: | |||
| output_types: ${{ steps.collect_output.outputs.output_types }} | |||
| steps: | |||
| - name: Checkout repository | |||
| uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 | |||
| uses: "actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5" | |||
There was a problem hiding this comment.
This uses value is quoted, which is inconsistent with other action pins. The unquoting function should have removed these quotes to match the format of other pinned actions in the workflow files.
| parts := strings.SplitN(line, "\"", 3) | ||
| if len(parts) >= 3 { | ||
| // parts[0] = " uses: " OR " - uses: " | ||
| // parts[1] = "slug@sha # version" | ||
| // parts[2] = "" or something else | ||
| lines[i] = parts[0] + parts[1] + strings.TrimPrefix(parts[2], "\"") |
There was a problem hiding this comment.
The unquoteUsesWithComments function has a logic issue with how it handles quotes. When splitting by ", if there are multiple quoted sections on a line or if the comment itself contains quotes, this could produce incorrect output. The function splits by " expecting exactly 3 parts, but doesn't validate this assumption properly.
Additionally, the check if len(parts) >= 3 allows for cases with more than 3 parts (4+), which could happen if there are additional quotes in the line. This would incorrectly concatenate parts and potentially break the YAML syntax.
| // parts[0] = " uses: " OR " - uses: " | ||
| // parts[1] = "slug@sha # version" | ||
| // parts[2] = "" or something else | ||
| lines[i] = parts[0] + parts[1] + strings.TrimPrefix(parts[2], "\"") |
There was a problem hiding this comment.
The strings.TrimPrefix(parts[2], "\"") only removes a leading quote from parts[2]. However, if the original line was uses: "slug@sha # v1", after splitting by ", parts[2] would be an empty string (or subsequent content), not a string with a leading quote to trim. This logic doesn't correctly handle the closing quote of the quoted uses value.
For a line like uses: "actions/checkout@sha # v5", splitting by " gives:
- parts[0] = "uses: "
- parts[1] = "actions/checkout@sha # v5"
- parts[2] = "" (empty)
The TrimPrefix on an empty string is a no-op, which works by accident, but the logic is confusing and fragile.
| @@ -167,15 +167,15 @@ jobs: | |||
| run: "pip install --user numpy pandas matplotlib seaborn scipy\n\n# Verify installations\npython3 -c \"import numpy; print(f'NumPy {numpy.__version__} installed')\"\npython3 -c \"import pandas; print(f'Pandas {pandas.__version__} installed')\"\npython3 -c \"import matplotlib; print(f'Matplotlib {matplotlib.__version__} installed')\"\npython3 -c \"import seaborn; print(f'Seaborn {seaborn.__version__} installed')\"\npython3 -c \"import scipy; print(f'SciPy {scipy.__version__} installed')\"\n\necho \"All scientific libraries installed successfully\"\n" | |||
| - if: always() | |||
| name: Upload generated charts | |||
| uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 | |||
| uses: "actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5" | |||
There was a problem hiding this comment.
This uses value is quoted, which is inconsistent with other action pins in the same file. All other pinned actions in this workflow use unquoted format (e.g., line 66, 74), but this one is quoted. This suggests the unquoteUsesWithComments function may not be working correctly for all cases, or these specific instances weren't processed through the same code path.
| with: | ||
| if-no-files-found: warn | ||
| name: data-charts | ||
| path: /tmp/gh-aw/python/charts/*.png | ||
| retention-days: 30 | ||
| - if: always() | ||
| name: Upload source files and data | ||
| uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 | ||
| uses: "actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5" |
There was a problem hiding this comment.
This uses value is quoted, which is inconsistent with other action pins in the same file. Most other pinned actions use unquoted format, but this one is quoted. This inconsistency suggests the unquoteUsesWithComments function may not be processing all action references uniformly.
|
@copilot review the unquoting bugs and add tests |
…placement-comment
- Fixed function to properly process CustomSteps after YAML marshalling - Added unquoteUsesWithComments call for CustomSteps in all 3 code paths - Added unquoteUsesWithComments call for PostSteps - Improved function logic for better reliability - Merged main branch successfully - All tests passing - Zero quoted uses values remaining in compiled workflows Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
- Created unquote_uses_test.go with 3 test suites - TestUnquoteUsesWithComments: 12 basic test cases - TestUnquoteUsesWithCommentsEdgeCases: 6 edge case tests - TestUnquoteUsesWithCommentsRealWorldExamples: 2 real workflow tests - All 20 tests pass - Tests cover normal cases, edge cases, malformed input, and real workflows - Validates the function correctly handles quotes with version comments Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Added comprehensive tests in commit 785311b. Created 3 test suites with 20 tests total:
All tests pass. The unquoteUsesWithComments function is now fully tested and validates that it correctly handles:
|
Completed: Fix Action Pin Comments and Merge Main
Issue Analysis
The bot review identified that many
uses:values were still quoted despite the unquoteUsesWithComments function. Investigation revealed:ConvertStepToYAML- they're marshalled directly to YAML strings without post-processingunquoteUsesWithCommentsfunction wasn't being called after marshalling CustomSteps and PostStepsChanges Made
Modified Files:
pkg/workflow/compiler.go- Added 4 calls tounquoteUsesWithComments:pkg/workflow/agentic_engine.go- ImprovedunquoteUsesWithCommentsfunction for robustnesspkg/workflow/unquote_uses_test.go- Added comprehensive test coverage:Results
✅ All 78 workflows compiled successfully
✅ All unit tests passing (20 new tests added)
✅ Zero quoted uses values remain (previously 34)
✅ Version comments properly preserved
Before:
After:
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.