diff --git a/.github/workflows/auto-close-pr.yml b/.github/workflows/auto-close-pr.yml index de2ca780..beaf1f77 100644 --- a/.github/workflows/auto-close-pr.yml +++ b/.github/workflows/auto-close-pr.yml @@ -1,20 +1,69 @@ name: Auto-close PR + on: pull_request_target: types: [opened, reopened] +permissions: + pull-requests: write + issues: write + +concurrency: + group: auto-close-pr-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: close: - name: Run + name: Close fork PRs with guidance runs-on: ubuntu-latest - permissions: - pull-requests: write + timeout-minutes: 5 steps: - - run: | - gh pr close ${{ github.event.pull_request.number }} --comment \ - "At the moment we are not accepting contributions to the repository. + - name: Debug context + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + run: | + has_auto_closed_label="${{ contains(github.event.pull_request.labels.*.name, 'auto-closed') }}" + echo "pr_number=${{ github.event.pull_request.number }}" + echo "pr_state=${{ github.event.pull_request.state }}" + echo "head_repo=${{ github.event.pull_request.head.repo.full_name }}" + echo "base_repo=${{ github.repository }}" + echo "author_association=${{ github.event.pull_request.author_association }}" + echo "event_action=${{ github.event.action }}" + echo "has_auto_closed_label=$has_auto_closed_label" - Feedback for GitHub Copilot for Xcode can be given in the [Copilot community discussions](https://github.com/orgs/community/discussions/categories/copilot)." + - name: Close PR from fork with message + if: >- + github.event.pull_request.head.repo.full_name != github.repository && + github.event.pull_request.state == 'open' && + github.event.pull_request.author_association == 'NONE' && + !contains(github.event.pull_request.labels.*.name, 'auto-closed') env: GH_REPO: ${{ github.repository }} GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: | + set -euo pipefail + COMMENT="$(cat <<'EOF' + Thanks for the pull request! At the moment we are not accepting contributions to this repository. + + Feedback for GitHub Copilot for Xcode can be shared in the Copilot community discussions: + https://github.com/orgs/community/discussions/categories/copilot + EOF + )" + max_attempts=3 + for attempt in $(seq 1 "$max_attempts"); do + if gh pr close "$PR_NUMBER" --reason not_planned --comment "$COMMENT"; then + break + elif [ "$attempt" -eq "$max_attempts" ]; then + echo "All $max_attempts attempts to close PR failed" >&2 + exit 1 + else + echo "Attempt $attempt failed, retrying in $((attempt * 5))s..." + sleep $((attempt * 5)) + fi + done + gh label create "auto-closed" --force --color "d93f0b" --description "PR was automatically closed" \ + || echo "::warning::Failed to create/update auto-closed label; label may not exist in repository" + gh pr edit "$PR_NUMBER" --add-label "auto-closed" + echo "✅ PR #$PR_NUMBER closed and labeled \`auto-closed\`" >> "$GITHUB_STEP_SUMMARY" diff --git a/PR_Suggestions.md b/PR_Suggestions.md new file mode 100644 index 00000000..d5ba1942 --- /dev/null +++ b/PR_Suggestions.md @@ -0,0 +1,19 @@ +### Suggested Improvements for PR #2 + +1. **Code Modularity**: + - Consider modularizing the workflow YAML into reusable components using **workflow templates**. This is helpful if you apply similar principles to other repositories or workflows. + +2. **Test Coverage Documentation**: + - Include examples (if plausible) of how PR workflows from forks vs. same-repo branches are handled. This can serve as evidence and documentation that the conditions are functioning properly. + +3. **Scenarios for Exclusions**: + - Account for edge cases explicitly in the guard conditions. For instance, consider excluding certain branches or adding a logging step for debugging skipped conditions. + +4. **Project Context**: + - Reflect on whether future fork PR handling (e.g., "not accepting" vs. "conditionally merging") warrants flexibility, such as toggling rules through repository/project-level labels. + +5. **Security Logging**: + - Log in debug mode some metadata (e.g., triggering user’s ID or fork origin) when dismissing PRs. This could be useful in contentious situations or to review why a workflow failed/was disabled. + +6. **Cleanup Tasks**: + - Add an automatic cleanup task (if needed) to ensure that fork-exclusive PR data or triggers wouldn't leave redundant temporary states, especially affected environment variables. diff --git a/pull_request_review_comments/PR2_comment.md b/pull_request_review_comments/PR2_comment.md new file mode 100644 index 00000000..9d73d70d --- /dev/null +++ b/pull_request_review_comments/PR2_comment.md @@ -0,0 +1,51 @@ +### Enhanced Review Comment for PR #2: + +This pull request focuses on enhancing the `auto-close-pr.yml` workflow for better handling of forked pull requests. Below are the key improvements, along with additional suggestions: + +--- + +### PR Changes and Improvements: +1. **Trigger Updates**: + - Added `synchronize` to the `pull_request_target` event types, allowing the workflow to respond to branch updates in pull requests. + - This is a strong improvement for keeping the workflow responsive to fork updates. + +2. **Security Enhancements**: + - Dropped `issues: write` permissions, keeping only `pull-requests: write`, aligning with a least-privilege principle. + +3. **Concurrency Improvements**: + - Added concurrency control (`auto-close-pr-${{ github.event.pull_request.number }}`) with `cancel-in-progress: true`, ensuring only the latest workflow run operates. + +4. **Fork Protection**: + - Introduced an `if` condition to run the workflow solely for fork-based pull requests in an open state. + - This ensures internal branch pull requests are not impacted by the workflow. + +5. **Error and Logging Improvements**: + - Used `set -euo pipefail` for robust error handling in bash. + - Quoted variable inputs like `"$PR_NUMBER"` for better reliability. + +6. **User Feedback**: + - Enhanced the close message to guide contributors toward alternative participation: + ``` + Thanks for the pull request! At the moment, we are not accepting contributions to this repository. + + Feedback for GitHub Copilot for Xcode can be shared in the Copilot community discussions: + https://github.com/orgs/community/discussions/categories/copilot + ``` + +--- + +### Additional Suggestions for Improvement: +1. **Enhanced Logging**: + - Consider including logs outlining the conditions under which a pull request is skipped or closed for better debugging: + ``` + echo "Closing PR with number $PR_NUMBER due to forked repository condition" + ``` + +2. **Testing and Validation**: + - Use a YAML linter or validator to ensure the workflow structure remains error-free. + +3. **Handling Trusted Forks**: + - If practical, implement conditional rules for accepting pull requests from known or trusted fork contributors. + +4. **User Guidance**: + - Add details to the close message, suggesting alternative ways for the contributor to engage (e.g., submitting issues, engaging in discussions). \ No newline at end of file diff --git a/pulls/2/comments.md b/pulls/2/comments.md new file mode 100644 index 00000000..c2d3789f --- /dev/null +++ b/pulls/2/comments.md @@ -0,0 +1,14 @@ +1. **Testing and Validation**: Consider adding a way to simulate PR workflows in a staging environment to validate these changes before merging into the main branch. This is particularly useful for `pull_request_target` workflows. + +2. **Error Handling**: While `set -euo pipefail` improves robustness, consider adding error messages in critical commands or logging for better debugging. + +3. **Trigger Coverage**: Double-check if `closed` should potentially be monitored to handle edge cases where PRs are closed directly after being reopened or synchronized. + +4. **Environment Variable Validation**: Include a check at the start of the script to ensure that critical `env` variables (like `PR_NUMBER`) are properly set before execution. + +5. **Documentation Improvements**: + - Expand on the rationale behind these changes in the `README.md` or repo documentation. Explicitly describe why fork PRs must be auto-closed and same-repo PRs ignored. + - Add comments wherever non-obvious logic is used to ensure future maintainers understand the conditions and implications. + +6. **Close Comment Guidance**: + - Include a link in the close comment directing contributors to documentation about why fork PRs aren’t accepted. \ No newline at end of file diff --git a/suggestions/improvements_pr_2.md b/suggestions/improvements_pr_2.md new file mode 100644 index 00000000..75d13685 --- /dev/null +++ b/suggestions/improvements_pr_2.md @@ -0,0 +1,6 @@ +## Suggestions for Improving the Current Workflow + +1. **Enhanced Logging for Debugging**: Add steps to log when a PR is skipped or closed for clarity in the Actions log. +2. **Testing Script Validity**: Use a YAML linter or validator to test the structure of the updated workflow file. +3. **Consider Conditional Rejections**: Build rules to manage PRs from trusted contributors or specific forks. +4. **User Feedback**: Update the close message to suggest alternative ways for contributors to participate. \ No newline at end of file