ci: fix ci run on push and pull request#507
Conversation
📝 WalkthroughWalkthroughGitHub Actions workflow configuration updated to trigger on pull requests and releases in addition to pushes. Concurrency grouping refined to separate PR contexts. Conditional logic added to skip certain jobs for specific PR prefixes. Checkout and token handling adjusted to use fallback mechanisms. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Important Merge conflicts detected (Beta)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 5-7: The CI workflow uses pull_request and release triggers which
create non-pushable refs and therefore auto-format pushes never reach PR
branches; update the workflow to run the formatting/push job on
pull_request_target (so actions run in the target repo with a writable
GITHUB_TOKEN) or add a separate pushable trigger (e.g., push to branches) and
ensure the checkout step uses actions/checkout@v3 with persist-credentials: true
so git receives the token; remove any "git push || true" masking and ensure the
push step sets GITHUB_TOKEN from secrets (e.g., env: GITHUB_TOKEN: ${{
secrets.GITHUB_TOKEN }}) so pushes from the workflow actually fail when
formatting cannot be applied.
| pull_request: | ||
| branches: | ||
| - '**' | ||
| release: | ||
| types: [published] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/ci.yml | head -100Repository: EngineSquared/EngineSquared
Length of output: 4199
🌐 Web query:
GitHub Actions pull_request event default ref vs release event ref
💡 Result:
In GitHub Actions, the default values of GITHUB_REF and GITHUB_SHA differ between pull_request and release events. For the pull_request event (non pull_request_target, non-merged): - GITHUB_REF: refs/pull/<pr_number>/merge (the merge ref simulating the merge commit of head into base) - GITHUB_SHA: SHA of that merge commit For the release event: - GITHUB_REF: refs/tags/<tag_name> (the tag ref of the release) - GITHUB_SHA: Last commit in the tagged release (SHA pointed to by the tag) These are documented in the official GitHub Actions docs in the events table and contexts/variables references.
Citations:
- 1: https://docs.github.com/actions/using-workflows/events-that-trigger-workflows
- 2: https://stackoverflow.com/questions/63568834/github-actions-which-branch-used-on-pr
- 3: https://docs.github.com/en/actions/reference/workflows-and-actions/contexts
- 4: https://docs.github.com/en/actions/reference/contexts-reference
- 5: https://docs.github.com/en/actions/reference/workflows-and-actions/variables
- 6: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows
🌐 Web query:
actions/checkout token parameter credential handling git push
💡 Result:
In GitHub Actions, the actions/checkout action handles the 'token' input parameter by using it (default ${{ github.token }}) to authenticate and fetch the repository. When persist-credentials is true (default), it configures the token in the local git config (in recent versions v6+, stored securely in
Citations:
- 1: https://github.com/actions/checkout
- 2: https://github.com/marketplace/actions/checkout
- 3: https://github.com/actions/checkout/blob/master/action.yml
🌐 Web query:
GitHub Actions fork pull request secrets github.token read-only
💡 Result:
In GitHub Actions, for pull requests from forked repositories (fork PRs), the GITHUB_TOKEN is provided with read-only permissions by default, and repository secrets are not available. This is a security measure to prevent malicious actors from accessing secrets or making unauthorized changes via PR workflows. Key behaviors: - GITHUB_TOKEN has read-only access (e.g., contents: read) unless the repository setting "Send write tokens to workflows from pull requests" is enabled (available for private repos/orgs). - No access to repository, organization, or environment secrets. - Workflows may require manual approval if configured. To enable workflows on fork PRs: - Go to repo Settings > Actions > General > Fork pull request workflows. - Enable "Run workflows from fork pull requests" (uses read-only GITHUB_TOKEN, no secrets). For write permissions: - Enable "Send write tokens to workflows from pull requests" (org/repo level, private repos only). - Combine with "Require approval for fork pull request workflows" for security. Alternatives for secrets/write access: - Use pull_request_target trigger: Runs on base branch (access to secrets, full GITHUB_TOKEN), but checkout PR head.sha explicitly. High risk if PR modifies workflow—use cautiously with approvals. - OIDC for cloud providers (e.g., AWS), avoiding secrets. - Manual approval workflows or environments (limited for forks). This behavior remains consistent as of 2026 per official docs.
Citations:
- 1: https://docs.github.com/en/actions/concepts/security/github_token
- 2: https://docs.github.com/en/actions/security-guides/automatic-token-authentication
- 3: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository
- 4: https://docs.github.com/en/actions/reference/github_token-reference
- 5: https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/
Auto-format pushes won't reach PR branches with this setup.
pull_request runs default to the PR merge ref (refs/pull/<number>/merge) and release runs use a tag ref, neither of which is a pushable branch. actions/checkout configures git credentials during the checkout step; setting GITHUB_TOKEN in the push step environment has no effect on subsequent git push commands. On fork PRs, the built-in token is read-only regardless, so git push || true masks the failure instead of enforcing formatting compliance.
💡 Suggested fix
- - uses: actions/checkout@v6
- if: steps.check_bot.outputs.skip != 'true'
+ - uses: actions/checkout@v6
+ if: steps.check_bot.outputs.skip != 'true'
+ with:
+ ref: ${{ github.head_ref || github.ref_name }}
+ token: ${{ secrets.PAT_CI_TEST || github.token }}
- - name: Push changes
- if: steps.check_bot.outputs.skip != 'true'
- env:
- GITHUB_TOKEN: ${{ secrets.PAT_CI_TEST || github.token }}
+ - name: Push changes
+ if: >
+ steps.check_bot.outputs.skip != 'true' &&
+ github.ref_type == 'branch' &&
+ (github.event_name != 'pull_request' ||
+ github.event.pull_request.head.repo.full_name == github.repository)
run: |
git add .
git commit -m "style: apply linter" || true
- git push || true
+ git push
+
+ - name: Fail if formatting changed on an unpushable PR
+ if: >
+ steps.check_bot.outputs.skip != 'true' &&
+ github.event_name == 'pull_request' &&
+ github.event.pull_request.head.repo.full_name != github.repository
+ run: git diff --quiet --exit-codeApplies to: 56–57, 88–95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 5 - 7, The CI workflow uses
pull_request and release triggers which create non-pushable refs and therefore
auto-format pushes never reach PR branches; update the workflow to run the
formatting/push job on pull_request_target (so actions run in the target repo
with a writable GITHUB_TOKEN) or add a separate pushable trigger (e.g., push to
branches) and ensure the checkout step uses actions/checkout@v3 with
persist-credentials: true so git receives the token; remove any "git push ||
true" masking and ensure the push step sets GITHUB_TOKEN from secrets (e.g.,
env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}) so pushes from the workflow
actually fail when formatting cannot be applied.
# Pull Request ## Description A clear and concise description of what this PR does and why it's needed. ## Related Issues Fixes #(issue) Relates to #(issue) ## Type of Change Please delete options that are not relevant. - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring - [ ] Build/CI configuration change ## Changes Made List the main changes in this PR: - Change 1 - Change 2 - Change 3 ## Testing Describe the tests you ran to verify your changes: - [ ] Unit tests pass (`xmake test`) - [ ] Code follows the project's style guidelines (`clang-format`) - [ ] Manual testing performed (describe what you tested) ### Test Environment - OS: [e.g. Windows 10, Ubuntu 22.04, macOS 13] - Compiler: [e.g. MSVC 2022, GCC 11.2, Clang 15] - Vulkan SDK Version: [e.g. 1.3.268.0] ## Screenshots/Videos (if applicable) Add screenshots or videos to help explain your changes. ## Documentation - [ ] I have updated the relevant documentation - [ ] I have added/updated comments in the code - [ ] No documentation changes are required ## Checklist - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published ## Breaking Changes If this PR introduces breaking changes, describe them here and provide migration instructions. ## Additional Notes Any additional information that reviewers should know. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated CI/CD workflow configuration to improve trigger conditions for pull requests and releases, refine concurrency handling, and adjust checkout and authentication behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
# Pull Request ## Description A clear and concise description of what this PR does and why it's needed. ## Related Issues Fixes #(issue) Relates to #(issue) ## Type of Change Please delete options that are not relevant. - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring - [ ] Build/CI configuration change ## Changes Made List the main changes in this PR: - Change 1 - Change 2 - Change 3 ## Testing Describe the tests you ran to verify your changes: - [ ] Unit tests pass (`xmake test`) - [ ] Code follows the project's style guidelines (`clang-format`) - [ ] Manual testing performed (describe what you tested) ### Test Environment - OS: [e.g. Windows 10, Ubuntu 22.04, macOS 13] - Compiler: [e.g. MSVC 2022, GCC 11.2, Clang 15] - Vulkan SDK Version: [e.g. 1.3.268.0] ## Screenshots/Videos (if applicable) Add screenshots or videos to help explain your changes. ## Documentation - [ ] I have updated the relevant documentation - [ ] I have added/updated comments in the code - [ ] No documentation changes are required ## Checklist - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published ## Breaking Changes If this PR introduces breaking changes, describe them here and provide migration instructions. ## Additional Notes Any additional information that reviewers should know. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated CI/CD workflow configuration to improve trigger conditions for pull requests and releases, refine concurrency handling, and adjust checkout and authentication behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->



Pull Request
Description
A clear and concise description of what this PR does and why it's needed.
Related Issues
Fixes #(issue)
Relates to #(issue)
Type of Change
Please delete options that are not relevant.
Changes Made
List the main changes in this PR:
Testing
Describe the tests you ran to verify your changes:
xmake test)clang-format)Test Environment
Screenshots/Videos (if applicable)
Add screenshots or videos to help explain your changes.
Documentation
Checklist
Breaking Changes
If this PR introduces breaking changes, describe them here and provide migration instructions.
Additional Notes
Any additional information that reviewers should know.
Summary by CodeRabbit