Skip to content

[WIP] Rewrite changes and review tests from pull request 19591#19609

Merged
pelikhan merged 4 commits intomainfrom
copilot/rewrite-changes-and-review-tests
Mar 4, 2026
Merged

[WIP] Rewrite changes and review tests from pull request 19591#19609
pelikhan merged 4 commits intomainfrom
copilot/rewrite-changes-and-review-tests

Conversation

Copy link
Contributor

Copilot AI commented Mar 4, 2026

  • Understand PR fix: install correct version in action #19591 changes and review comments
  • Fix install.sh: replace gh extension install with version-aware branching (no eval, fix installed_version output bug)
  • Fix install_test.sh: correct test number labels, replace test_release_validation with test_version_pinning

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@pelikhan pelikhan marked this pull request as ready for review March 4, 2026 14:36
Copilot AI review requested due to automatic review settings March 4, 2026 14:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the setup-cli installer and its lightweight shell tests to support version-aware gh extension install and correct the reported installed version.

Changes:

  • Add conditional version pinning when installing via gh extension install (pin when VERSION != latest).
  • Fix installed_version output to reflect the detected installed extension version for the gh install path.
  • Update install_test.sh to correct test numbering and validate version pinning logic instead of release validation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
actions/setup-cli/install.sh Adds version-aware gh extension install branching and fixes the installed_version output for the gh install path.
actions/setup-cli/install_test.sh Renumbers tests and replaces the release-validation grep with a version-pinning grep.

💡 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.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@pelikhan
Copy link
Contributor

pelikhan commented Mar 4, 2026

@copilot review the changes from a security and portability point of view

Copy link
Contributor Author

Copilot AI commented Mar 4, 2026

@copilot review the changes from a security and portability point of view

Security and portability review of the changes:

Security ✅

  • No eval or string interpolation in shell invocations — $REPO, $VERSION are passed as distinct, quoted arguments to gh extension install, so no command injection is possible even if VERSION contains spaces or shell metacharacters.
  • $VERSION is validated to be non-empty and not the literal string "latest" before use with --pin, preventing the pin flag from being passed a blank or ambiguous value.
  • $INSTALLED_VERSION (the actual detected version string) is written to GITHUB_OUTPUT instead of the raw user input $VERSION, so the reported output always reflects reality.

Portability ✅

  • ${PIPESTATUS[0]} is bash-specific, but the script declares #!/bin/bash on line 1, making this fully appropriate. PIPESTATUS correctly captures the exit code of gh extension install rather than tee, so a failed install is reliably detected regardless of whether tee succeeds.
  • All arguments are double-quoted, which is correct for POSIX/bash — no word-splitting or glob-expansion issues.

No issues found — the implementation is both secure and appropriately portable for a bash script.

@pelikhan pelikhan merged commit 386fdff into main Mar 4, 2026
47 checks passed
@pelikhan pelikhan deleted the copilot/rewrite-changes-and-review-tests branch March 4, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants