-
Notifications
You must be signed in to change notification settings - Fork 0
chore: Rewriting the workflow to use push instead of PR #4
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
Because of Github actions security policies, a PR comming from a fork cannot access repository or organization secrets making it impossible to run the sync when external contributors submit their own pull requests. With this change the workflow is triggered by push events and uses commit messages and PR APIs to fetch more details making it run exclusivily on the repo
Caution Review failedThe pull request is closed. WalkthroughA new GitHub Actions workflow, "Reverse Sync on Push," has been introduced to automate the synchronization of documentation changes from the current repository to a target repository upon pushes to specific branches. The workflow includes steps for commit filtering, PR metadata extraction, patch creation, fallback copying, and pull request creation in the target repository. Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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 rewrites the reverse sync GitHub Actions workflow to trigger on push events instead of pull requests to comply with repository security policies. Key changes include:
- Adjusting the trigger event from PR to push.
- Modifying steps to determine commit origin and obtain PR details.
- Implementing a reverse sync process that creates a new branch and PR in the target repository.
- name: Checkout devops-pipelines-docs repository | ||
uses: actions/checkout@v4 | ||
with: | ||
token: ${{ secrets.GH_TOKEN }} |
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.
The workflow uses GH_TOKEN here while later steps use GITHUB_TOKEN for API calls. Consider using consistent secret names for clarity and maintainability.
token: ${{ secrets.GH_TOKEN }} | |
token: ${{ secrets.GITHUB_TOKEN }} |
Copilot uses AI. Check for mistakes.
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.
this is as expected
echo "⚠️ Patch application failed, trying manual copy..." | ||
|
||
# Fallback: manual copy of changed files | ||
while IFS= read -r file; do | ||
if [ -f "../$file" ]; then | ||
mkdir -p "$(dirname "$file")" | ||
cp "../$file" "$file" | ||
echo "✅ Copied: $file" |
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.
The fallback for a failed patch application relies on manual copy of files. Consider adding more detailed logging or error handling here to facilitate troubleshooting if the patch fails.
echo "⚠️ Patch application failed, trying manual copy..." | |
# Fallback: manual copy of changed files | |
while IFS= read -r file; do | |
if [ -f "../$file" ]; then | |
mkdir -p "$(dirname "$file")" | |
cp "../$file" "$file" | |
echo "✅ Copied: $file" | |
echo "⚠️ Patch application failed. Capturing error details..." | |
git apply ../changes.patch 2> ../patch_error.log | |
echo "❌ Error details logged to patch_error.log:" | |
cat ../patch_error.log | |
echo "⚠️ Attempting manual copy of changed files as fallback..." | |
# Validate changed_files.txt | |
if [ ! -f ../changed_files.txt ]; then | |
echo "❌ Error: changed_files.txt not found. Cannot proceed with manual copy." | |
exit 1 | |
fi | |
# Fallback: manual copy of changed files | |
while IFS= read -r file; do | |
if [ -f "../$file" ]; then | |
mkdir -p "$(dirname "$file")" || { | |
echo "❌ Error: Failed to create directory for $file" | |
continue | |
} | |
cp "../$file" "$file" || { | |
echo "❌ Error: Failed to copy $file" | |
continue | |
} | |
echo "✅ Copied: $file" | |
else | |
echo "⚠️ Warning: File $file not found in source. Skipping." |
Copilot uses AI. Check for mistakes.
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.
not necessary
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.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.github/workflows/reverse-sync-push.yml (1)
214-226
: Strip unintended leading spaces in the generated commit messageBecause the
cat << EOF
block is indented inside the step, each line in the commit message starts with 10 spaces, polluting history.- cat > commit_message.txt << EOF + # Using `<<-EOF` trims *tabs* from the beginning of each line. + cat > commit_message.txt <<-EOFIndent the message body with tab characters (not spaces) or move the heredoc to the left margin to keep the final commit clean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/reverse-sync-push.yml
(1 hunks)
🔇 Additional comments (1)
.github/workflows/reverse-sync-push.yml (1)
60-67
: No change needed—bot email check is safeThe workflow explicitly configures the sync bot’s email to
noreply@github.com
, while GitHub’s privacy-protected user addresses take the form<username>@users.noreply.github.com
. As a result:
- The condition
[[ "$commit_author_email" == "noreply@github.com" ]] && [[ "$commit_message" == *"[reverse-sync]"* ]]
only matches the bot, not real contributors.- Legitimate user commits (even with privacy enabled) will not be skipped.
You can safely retain the existing filter.
Likely an incorrect or invalid review comment.
uses: actions/checkout@v4 | ||
with: | ||
token: ${{ secrets.GH_TOKEN }} | ||
fetch-depth: 0 | ||
|
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.
🛠️ Refactor suggestion
Use a single built-in token instead of a custom GH_TOKEN
secret
actions/checkout@v4
and the GitHub REST API work seamlessly with the auto-generated GITHUB_TOKEN
, which is already scoped by the permissions:
block.
Relying on an extra secret increases maintenance, requires additional repository settings, and exposes one more credential.
- with:
- token: ${{ secrets.GH_TOKEN }}
+ with:
+ token: ${{ secrets.GITHUB_TOKEN }}
Make the same replacement in all other occurrences (checkout of target-docs
, curl … /pulls
, and the git push
remote URL).
After the change you can remove the GH_TOKEN
secret from the repo settings.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
uses: actions/checkout@v4 | |
with: | |
token: ${{ secrets.GH_TOKEN }} | |
fetch-depth: 0 | |
uses: actions/checkout@v4 | |
with: | |
token: ${{ secrets.GITHUB_TOKEN }} | |
fetch-depth: 0 |
🤖 Prompt for AI Agents
In .github/workflows/reverse-sync-push.yml around lines 22 to 26, replace the
custom secret token `${{ secrets.GH_TOKEN }}` used in the `actions/checkout@v4`
step with the built-in `${{ secrets.GITHUB_TOKEN }}`. Also update all other
occurrences where `GH_TOKEN` is used, including checkout steps for
`target-docs`, any `curl` commands accessing pull requests, and the `git push`
remote URL, to use `GITHUB_TOKEN` instead. After these changes, remove the
`GH_TOKEN` secret from the repository settings to reduce maintenance and
exposure of credentials.
# Look for PR number in commit message (GitHub automatically adds this) | ||
if [[ "$commit_message" =~ \(#([0-9]+)\)$ ]]; then | ||
pr_number="${BASH_REMATCH[1]}" | ||
echo "pr_number=$pr_number" >> $GITHUB_OUTPUT | ||
echo "is_pr_commit=true" >> $GITHUB_OUTPUT | ||
echo "✅ Detected commit from PR #$pr_number" | ||
else | ||
echo "is_pr_commit=false" >> $GITHUB_OUTPUT | ||
echo "ℹ️ Not a PR commit - skipping reverse sync" | ||
fi |
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.
🛠️ Refactor suggestion
PR-number detection misses regular merge commits
The current regex only matches squash-merge titles ending in “(#123)
”.
Regular merge commits created by the GitHub UI have the form “Merge pull request #123 …
” and will bypass the reverse-sync logic.
- if [[ "$commit_message" =~ \(#([0-9]+)\)$ ]]; then
+ if [[ "$commit_message" =~ \(#([0-9]+)\)$ ]] || [[ "$commit_message" =~ [Pp]ull[[:space:]]request[[:space:]]#([0-9]+) ]]; then
Consider extracting the regex into a variable for readability and unit-testing the pattern with typical commit messages to avoid future regressions.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
# Look for PR number in commit message (GitHub automatically adds this) | |
if [[ "$commit_message" =~ \(#([0-9]+)\)$ ]]; then | |
pr_number="${BASH_REMATCH[1]}" | |
echo "pr_number=$pr_number" >> $GITHUB_OUTPUT | |
echo "is_pr_commit=true" >> $GITHUB_OUTPUT | |
echo "✅ Detected commit from PR #$pr_number" | |
else | |
echo "is_pr_commit=false" >> $GITHUB_OUTPUT | |
echo "ℹ️ Not a PR commit - skipping reverse sync" | |
fi | |
# Look for PR number in commit message (GitHub automatically adds this) | |
- if [[ "$commit_message" =~ \(#([0-9]+)\)$ ]]; then | |
+ if [[ "$commit_message" =~ \(#([0-9]+)\)$ ]] || [[ "$commit_message" =~ [Pp]ull[[:space:]]request[[:space:]]#([0-9]+) ]]; then | |
pr_number="${BASH_REMATCH[1]}" | |
echo "pr_number=$pr_number" >> $GITHUB_OUTPUT | |
echo "is_pr_commit=true" >> $GITHUB_OUTPUT | |
echo "✅ Detected commit from PR #$pr_number" | |
else | |
echo "is_pr_commit=false" >> $GITHUB_OUTPUT | |
echo "ℹ️ Not a PR commit - skipping reverse sync" | |
fi |
🤖 Prompt for AI Agents
In .github/workflows/reverse-sync-push.yml around lines 49 to 58, the regex only
matches squash-merge commit messages ending with (#123), missing regular merge
commits like "Merge pull request #123 ...". Update the regex to also detect
these regular merge commit formats. Extract the regex pattern into a variable
for clarity and maintainability, and consider adding unit tests for the regex
against typical commit message formats to prevent future detection issues.
echo "📑 Will only sync these paths: $SYNCED_PATHS" | ||
git format-patch $parent_commit..$commit_sha --stdout -- $SYNCED_PATHS > changes.patch | ||
|
||
cd target-docs | ||
|
||
# Apply the patch | ||
if [ -s ../changes.patch ]; then | ||
echo "📦 Applying changes from devops-pipelines-docs..." | ||
git apply ../changes.patch || { | ||
echo "⚠️ Patch application failed, trying manual copy..." |
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.
🛠️ Refactor suggestion
Patch application nearly always fails because of directory mismatch
git format-patch … > changes.patch
is generated from the repo root, but git apply
is executed inside target-docs/
.
Unless both repos share an identical directory structure, paths inside the patch won’t resolve and the script falls back to the slower per-file copy.
- git format-patch $parent_commit..$commit_sha --stdout -- $SYNCED_PATHS > changes.patch
+ git -C $GITHUB_WORKSPACE \
+ format-patch $parent_commit..$commit_sha --stdout -- $SYNCED_PATHS > changes.patch
...
- git apply ../changes.patch || {
+ git apply --directory=$GITHUB_WORKSPACE/../target-docs ../changes.patch || {
Alternatively, switch to git diff -p1
and patch -p1
executed from the same directory to avoid the indirection.
Fixing this will save unnecessary IO and runtime.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .github/workflows/reverse-sync-push.yml around lines 160 to 169, the patch
generated by git format-patch is created from the repo root but applied inside
the target-docs directory, causing path mismatches and patch failures. To fix
this, replace git format-patch with git diff -p1 to generate the patch and use
patch -p1 to apply it, ensuring both commands run from the same directory
(target-docs) so paths align correctly and avoid fallback to manual copying.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Because of Github actions security policies, a PR comming from
a fork cannot access repository or organization secrets making
it impossible to run the sync when external contributors submit
their own pull requests.
With this change the workflow is triggered by push events and uses
commit messages and PR APIs to fetch more details making it
run exclusivily on the repo
Summary by CodeRabbit