-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Add automatic standard CI variable resolution #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add standard_ci_vars input to enable automatic variable resolution - Resolve common variables like pr-number, head-ref, base-ref, etc. - Support multiple trigger types: pull_request, workflow_dispatch, issue_comment - Smart gitref resolution handles 'run from main, operate on PR branch' pattern - Git refs are null for issue events since issues don't have branches - Maintain backward compatibility with existing functionality - Add comprehensive documentation and test cases - Drop context variable from auto-resolution per user feedback Co-Authored-By: AJ Steers <[email protected]>
- Set both head-ref and base-ref to current branch for workflow_dispatch without pr input - Ensure gitref resolves to fork's branch for PR scenarios - Improve comments for clarity on fork vs parent repo handling Co-Authored-By: AJ Steers <[email protected]>
Original prompt from AJ Steers
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
- Update gitref resolution to use branch name only for PR events - Fetch head-repo from PR data when using pr input for consistency - Ensure gitref always refers to a branch that exists on the corresponding repo - Maintain backward compatibility for all trigger types Co-Authored-By: AJ Steers <[email protected]>
- Change pr-from-fork (was is-pr-from-fork) - Add restricted-security-mode for fork/dependabot pull_request triggers - Use repo-name vs repo-name-full for clarity - Update variable names to be more explicit and intuitive - Change default for standard_ci_vars to true Co-Authored-By: AJ Steers <[email protected]>
- target-repository → target-repo - source-repository → source-repo - repository-owner → repo-owner (already correct) - Keep repo-name and repo-name-full as is Co-Authored-By: AJ Steers <[email protected]>
- Change target-repository → repo - Change trigger-event → trigger-type - Change workflow-actor → actor - Change repository-owner → repo-owner - Match variable names with current implementation Co-Authored-By: AJ Steers <[email protected]>
- Change target-repository → repo (with target-repo as alias) - Change repository-owner → repo-owner - Change source-repository → source-repo - Change trigger-event → trigger-type - Change workflow-actor → actor - Ensure individual CI variables are accessible as GitHub Action outputs - Maintain backward compatibility with 'all' JSON output Co-Authored-By: AJ Steers <[email protected]>
- Ensure repo-name-full is populated in CI_VARS array - This variable is expected by the test but was missing from implementation - Fixes CI failure in Test Standard CI Variables job Co-Authored-By: AJ Steers <[email protected]>
- Clean up duplicate lines that were accidentally added - Keep only one assignment for repo-name-full variable Co-Authored-By: AJ Steers <[email protected]>
- Allow both PR numbers and valid GitHub PR URLs in validation - Improve error message clarity for invalid PR inputs - Address security concerns raised by Copilot code review - Maintain backward compatibility with existing workflows Co-Authored-By: AJ Steers <[email protected]>
… CI vars disabled' - Address user feedback on test naming - Use more descriptive and evergreen naming convention - No functional changes, just improved clarity Co-Authored-By: AJ Steers <[email protected]>
…bles - Remove standard_ci_vars input parameter as requested by user - Standard CI variables are now always resolved automatically - Remove related test for disabled standard CI vars since option no longer exists - Update README to reflect that standard CI vars are always enabled - Simplifies the action interface by removing unnecessary configuration Co-Authored-By: AJ Steers <[email protected]>
…stom' - Add resolved-* variables (always effective context) - Add pr-source-* and pr-target-* variables for explicit PR workflows - Add pr-*, comment-*, run-* variables with URLs - Remove actor variable as requested - Rename 'all' output to 'custom' for clarity - Generate GitHub URLs for branches, commits, PRs, and comments - Update tests to use new variable names - Update README with comprehensive new variable reference Breaking changes: - All variable names have changed to use new prefixes - 'all' output renamed to 'custom' - 'actor' variable removed Co-Authored-By: AJ Steers <[email protected]>
…d move gh check to separate step - Fix CI failure caused by renamed output in variable restructuring - Update all JSON parsing references in first test to use 'outputs.custom' - Move GitHub CLI availability check to separate step as requested in PR comment - Ensure consistent output naming across all tests Co-Authored-By: AJ Steers <[email protected]>
- Fix GitHub Actions validation error from duplicate step ID - Ensure single GitHub CLI availability check step as requested - Maintain all variable restructuring and output renaming Co-Authored-By: AJ Steers <[email protected]>
- Fix CI failure caused by action.yml referencing 'all_vars' instead of 'custom' - Rename all repo-full-name variables to repo-name-full for alphabetical consistency - Update documentation and tests to reflect new variable names Co-Authored-By: AJ Steers <[email protected]>
- Auto-detect pr/pr-number, comment-id, and issue-id/issue-number inputs - Enhanced workflow_dispatch handling to resolve PR context automatically - Updated documentation with examples and feature descriptions - Maintains backward compatibility with existing functionality Co-Authored-By: AJ Steers <[email protected]>
d657f71
to
6e18bb5
Compare
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.
Pull Request Overview
This PR implements automatic resolution of standard CI variables from GitHub context, eliminating the need for complex expressions like ${{ github.event.pull_request.head.repo.full_name || github.repository }}
throughout workflows.
- Adds 20+ standard CI variables including
pr-number
,head-ref
,base-ref
,repo
, andgitref
resolution - Implements smart
gitref
resolution with fork-aware handling and issue context awareness - Adds comprehensive test coverage for the new standard CI variables functionality
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
File | Description |
---|---|
action.yml |
Adds new CI variable resolution step with 20+ output variables and smart context detection logic |
README.md |
Documents the new standard CI variables with comprehensive reference table and usage examples |
.github/workflows/test-action.yml |
Updates existing tests to use new output format and adds dedicated test job for standard CI variables |
Comments suppressed due to low confidence (1)
action.yml:271
- The fallback logic
|| github.sha
will always evaluate togithub.sha
whengithub.event.pull_request.head.sha
is empty, but GitHub Actions expressions don't work this way. Use proper conditional logic or separate if statements to handle empty values.
CI_VARS["resolved-git-sha"]="${{ github.sha }}"
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
- Replace sed-based escaping with jq-based JSON construction - Addresses security issue where only quotes were escaped - Now properly handles backslashes, newlines, tabs, and other JSON special chars - Fixes all 4 locations in action.yml that construct JSON output Resolves GitHub comment ID 2280955710 Co-Authored-By: AJ Steers <[email protected]>
- Fix CI failures by making JSON output compact for GITHUB_ENV compatibility - Add emoji grouping (🔧📝📦) to log outputs for better scanability - Addresses user request for improved log readability in joint action output Co-Authored-By: AJ Steers <[email protected]>
- Replace emoji-based grouping with proper ::group:: and ::endgroup:: commands - Maintain emoji symbols for visual appeal within the grouped sections - Improves log readability with collapsible sections in GitHub Actions interface Co-Authored-By: AJ Steers <[email protected]>
…pressions - Use restrictive regex for repository names instead of permissive patterns - Add proper shell escaping with -- separator for command - Fix GitHub Actions expression fallback logic to handle empty/null values - Addresses Copilot security comments on PR #3 Co-Authored-By: AJ Steers <[email protected]>
feat: Add automatic standard CI variable resolution
Summary
This PR implements automatic resolution of standard CI variables from GitHub context, eliminating the need for complex expressions like
${{ github.event.pull_request.head.repo.full_name || github.repository }}
throughout workflows.Key Changes:
standard_ci_vars
input parameter (default: false) to enable the featurepr-number
,head-ref
,base-ref
,repo
,head-repo
,base-repo
, etc.gitref
resolution handles "run from main, operate on PR branch" pattern for slash commandsrepo
variables point to base repository,gitref
points to fork's branchReview & Testing Checklist for Human
repo
resolves to base repo whilegitref
resolves to fork's branch namepr
input resolves variables correctlystandard_ci_vars: false
to ensure no regressionsRecommended Test Plan:
/test
, etc.) to ensure smart gitref fetches correct branchDiagram
Notes
gh pr view
has fallback logic but may need testing in constrained CI environmentsrepo
(base) vsgitref
(head) distinction works correctly