Skip to content

ci: use pull_request_target for clang-format comment permissions#7262

Closed
UdjinM6 wants to merge 2 commits into
dashpay:developfrom
UdjinM6:fix_clang_format_ci
Closed

ci: use pull_request_target for clang-format comment permissions#7262
UdjinM6 wants to merge 2 commits into
dashpay:developfrom
UdjinM6:fix_clang_format_ci

Conversation

@UdjinM6
Copy link
Copy Markdown

@UdjinM6 UdjinM6 commented Mar 31, 2026

Issue being fixed or feature implemented

Looks like codex bot was right #7251 (comment) -
https://github.com/dashpay/dash/actions/runs/23818749792/job/69425275349?pr=7101

#7251 follow-up

What was done?

Switch from pull_request to pull_request_target so the workflow gets a write-capable GITHUB_TOKEN for posting PR comments, including on fork PRs. The base branch is checked out (keeping clang-format-diff.py trusted) and only the PR head SHA is fetched for diffing.

How Has This Been Tested?

Breaking Changes

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

Switch from pull_request to pull_request_target so the workflow gets
a write-capable GITHUB_TOKEN for posting PR comments, including on
fork PRs. The base branch is checked out (keeping clang-format-diff.py
trusted) and only the PR head SHA is fetched for diffing.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@UdjinM6 UdjinM6 added this to the 24 milestone Mar 31, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 31, 2026

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

@thepastaclaw
Copy link
Copy Markdown

thepastaclaw commented Mar 31, 2026

Review Gate

Commit: 86d94ee3

  • Debounce: 31m ago (need 30m)

  • CI checks: checks still running (9 pending)

  • CodeRabbit review: comment found

  • Run review now (check to override)

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 31, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 189aa4f8-bd46-4320-ab3d-ee2c1cc50b4c

📥 Commits

Reviewing files that changed from the base of the PR and between 5eb29ad and 86d94ee.

📒 Files selected for processing (1)
  • .github/workflows/clang-diff-format.yml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/clang-diff-format.yml

Walkthrough

This change updates the GitHub Actions workflow for clang-format checks. The trigger was changed from pull_request to pull_request_target, job permissions now include contents: read alongside pull-requests: write, and the checkout step was relabeled. Git handling was changed to a shallow checkout (fetch-depth: 50) and an explicit fetch of the PR head commit (${{ github.event.pull_request.head.sha }}), with diffs computed using HEAD...${{ github.event.pull_request.head.sha }} instead of origin/develop. Diff generation and has_diff detection remain functionally equivalent.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: switching from pull_request to pull_request_target trigger to enable write permissions for posting PR comments.
Description check ✅ Passed The description is directly related to the changeset, explaining the issue being addressed, what was done, and the rationale for the workflow trigger change.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

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/clang-diff-format.yml:
- Around line 10-11: The permissions block only grants pull-requests: write
which leaves contents at none and breaks actions/checkout@v6 and git fetch;
update the permissions stanza to explicitly include contents: read alongside
pull-requests: write so the workflow has repository contents read access (i.e.,
add contents: read to the existing permissions block that currently contains
pull-requests: write).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 4db61ee0-e472-4930-ab0c-842c1b48711b

📥 Commits

Reviewing files that changed from the base of the PR and between 2ca9da3 and 5eb29ad.

📒 Files selected for processing (1)
  • .github/workflows/clang-diff-format.yml

Comment thread .github/workflows/clang-diff-format.yml
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5eb29ad689

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread .github/workflows/clang-diff-format.yml Outdated
Comment thread .github/workflows/clang-diff-format.yml Outdated
Add contents: read permission so checkout and git fetch work when
job-level permissions are specified. Use fetch-depth: 50 consistent
with other workflows. Pass non-backported.txt patterns directly to
git diff so PR-added files matching existing globs are included.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 86d94ee3cb

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread .github/workflows/clang-diff-format.yml
@UdjinM6 UdjinM6 removed this from the 24 milestone Mar 31, 2026
@UdjinM6 UdjinM6 closed this Mar 31, 2026
PastaPastaPasta added a commit that referenced this pull request Apr 6, 2026
… check

1682e71 ci: fix grep pattern to match literal parentheses in BRE (UdjinM6)
eb7554a ci: replace PR comment with job summary for clang-format check (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  The PR comment approach #7251 fails on fork PRs because `pull_request` events get a read-only `GITHUB_TOKEN`. Switching to `pull_request_target` #7262 would fix permissions but introduces its own issues: the checkout is the base branch so `git ls-files` misses files added by the PR, and the three-dot merge-base diff requires deeper clone history.

  ## What was done?
  Keep `pull_request` (correct checkout, full file coverage), remove the comment machinery, and write a short summary to `GITHUB_STEP_SUMMARY`. The full diff remains visible in the job log.

  ## How Has This Been Tested?

  ## Breaking Changes

  ## Checklist:
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    utACK 1682e71

Tree-SHA512: e1b9a1e5a6af6e40492cd611a7af8bde0ee8c7681f76df742725d30fd78fdb100347ee5f9d8181d3021dccaf7242b06a53a21c187339cd8aa090518212490938
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.

2 participants