Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 108 additions & 11 deletions .github/workflows/rubocop_syntax_checker.yaml
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
name: Rubocop
on:
push:
branches:
- master
paths:
- 'scripts/**'
- 'type_data/migrations/**'
pull_request:
paths:
- 'scripts/**'
- 'type_data/migrations/**'

permissions:
contents: write

jobs:
rubocop:
runs-on: ubuntu-latest
Expand All @@ -23,6 +20,8 @@ jobs:
steps:
- uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.2
with:
ref: ${{ github.ref_name }}
token: ${{ secrets.GITHUB_TOKEN }}
fetch-depth: 0

- name: Get changed files
Expand All @@ -37,9 +36,107 @@ jobs:
with:
ruby-version: ${{ matrix.ruby }}
bundler-cache: true

- name: Rubocop

- name: Determine base reference and fetch
id: base_ref
run: |
# For push events, check if it's a new branch
if [ "${{ github.event.before }}" == "0000000000000000000000000000000000000000" ]; then
echo "New branch detected, comparing with default branch"
# Use git symbolic-ref which properly handles branch names with spaces
DEFAULT_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD | sed 's@^refs/remotes/origin/@@')
echo "base=$DEFAULT_BRANCH" >> $GITHUB_OUTPUT
echo "compare_ref=origin/$DEFAULT_BRANCH" >> $GITHUB_OUTPUT
git fetch origin "$DEFAULT_BRANCH"
else
echo "base=${{ github.event.before }}" >> $GITHUB_OUTPUT
echo "compare_ref=${{ github.event.before }}" >> $GITHUB_OUTPUT
fi

- name: Get changed files
id: changed_files
run: |
# Write NUL-separated output directly to file to preserve delimiters
git diff --name-only --diff-filter=ACM -z "${{ steps.base_ref.outputs.compare_ref }}" HEAD | \
grep -zE '\.(rb|rbw|lic)$' > /tmp/changed_files.txt || true
Comment on lines +60 to +61
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's understand the git diff filter flags
git diff --help | grep -A 20 "diff-filter" || echo "Checking git documentation..."

Repository: elanthia-online/scripts

Length of output: 211


🏁 Script executed:

# Let's also examine the full workflow file to understand the context
cat -n .github/workflows/rubocop_syntax_checker.yaml | head -80

Repository: elanthia-online/scripts

Length of output: 3484


🌐 Web query:

git diff --diff-filter flags documentation ACM ACMR renamed files

💡 Result:

git diff --diff-filter=<flags> lets you include or exclude paths by change type. The flag letters mean: Added, Copied, Deleted, Modified, Renamed, Type-changed, Unmerged, Xunknown, Broken pairing. You can combine them (e.g. ACMR). You can also lowercase a letter to exclude that type (e.g. --diff-filter=ad excludes added+deleted). An optional * means “all-or-none”. [1]

About renamed files (R)

  • R entries only appear if rename detection is enabled (e.g. -M / --find-renames, or your config enables it). If rename detection is off, you won’t get R even if a rename happened. [1]

Common examples

# Only renamed paths (plus show old/new names)
git diff -M --name-status --diff-filter=R <rev1> <rev2>

# Added, Copied, Modified, Renamed (exclude Deletes)
git diff -M --name-status --diff-filter=ACMR <rev1> <rev2>

# Everything except Added and Deleted
git diff --diff-filter=ad <rev1> <rev2>

Docs: diff-options (covers --diff-filter and -M/--find-renames). [1]

Sources
[1] Git documentation: diff options (--diff-filter, -M/--find-renames, exclude via lowercase, *). (git-scm.com)

Citations:


Include renamed files in the diff filter and enable rename detection.

--diff-filter=ACM excludes R (renamed files), and without the -M flag, git diff won't detect renames at all. Both changes are needed to ensure renamed .rb, .rbw, and .lic files are checked by RuboCop.

🛠️ Suggested change
-          git diff --name-only --diff-filter=ACM -z "${{ steps.base_ref.outputs.compare_ref }}" HEAD | \
+          git diff -M --name-only --diff-filter=ACMR -z "${{ steps.base_ref.outputs.compare_ref }}" HEAD | \
📝 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.

Suggested change
git diff --name-only --diff-filter=ACM -z "${{ steps.base_ref.outputs.compare_ref }}" HEAD | \
grep -zE '\.(rb|rbw|lic)$' > /tmp/changed_files.txt || true
git diff -M --name-only --diff-filter=ACMR -z "${{ steps.base_ref.outputs.compare_ref }}" HEAD | \
grep -zE '\.(rb|rbw|lic)$' > /tmp/changed_files.txt || true
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/rubocop_syntax_checker.yaml around lines 60 - 61, The git
diff command currently uses --diff-filter=ACM and lacks rename detection; update
the invocation used in the rubocop_syntax_checker workflow (the git diff
--name-only --diff-filter=ACM -z "${{ steps.base_ref.outputs.compare_ref }}"
HEAD | grep -zE ...) to include renamed files and detect renames by adding R to
the diff-filter and the -M flag (i.e., use --diff-filter=ACMR and include -M
immediately after git diff) so renamed .rb, .rbw, and .lic files are captured
for RuboCop checks.


if [ -s /tmp/changed_files.txt ]; then
echo "has_changes=true" >> $GITHUB_OUTPUT
echo "Changed files:"
tr '\0' '\n' < /tmp/changed_files.txt
else
echo "has_changes=false" >> $GITHUB_OUTPUT
echo "No Ruby, .rbw, or .lic files changed"
fi

- name: Run Rubocop autocorrect on changed files
if: steps.changed_files.outputs.has_changes == 'true'
run: |
echo "Running rubocop -a on changed files..."
xargs -0 bundle exec rubocop -a < /tmp/changed_files.txt || {
echo "Warning: Rubocop autocorrect encountered issues but continuing..."
exit 0
}

- name: Check for changes and commit
if: steps.changed_files.outputs.has_changes == 'true'
run: |
set -o pipefail

# Skip auto-commit on default/protected branches
if [ "${{ github.ref_name }}" == "master" ] || [ "${{ github.ref_name }}" == "main" ]; then
echo "::notice::Skipping auto-commit on protected branch ${{ github.ref_name }}"
if ! git diff --quiet; then
echo "::error::Rubocop autocorrect would make changes to protected branch. Please run 'rubocop -a' locally."
git diff --stat
exit 1
fi
echo "No changes needed on protected branch"
exit 0
fi

git config --local user.email "github-actions[bot]@users.noreply.github.com"
git config --local user.name "github-actions[bot]"

if git diff --quiet; then
echo "No changes made by rubocop autocorrect"
else
if git add -u && git commit -m "Auto-fix: Apply rubocop autocorrections [skip ci]"; then
# Attempt to push and capture the exit code
if git push origin "HEAD:${{ github.ref_name }}" 2>&1 | tee /tmp/push_output.txt; then
echo "Successfully pushed autocorrect changes"
else
PUSH_EXIT_CODE=$?

# Check if push failed due to protected branch or permissions (exit code 1)
# GitHub's push rejection for protected branches typically uses exit code 1
if [ $PUSH_EXIT_CODE -eq 1 ]; then
# Additional check: look for common protected branch indicators in output
if grep -qiE "(protected branch|permission|prohibited|rejected)" /tmp/push_output.txt; then
echo "::warning::Cannot push to protected branch or insufficient permissions."
echo "::warning::Rubocop fixes were not committed. Please apply rubocop fixes manually or adjust branch protection rules."
exit 0
fi
fi

# For any other push failure, report as error
echo "::error::Failed to push changes (exit code: $PUSH_EXIT_CODE)"
cat /tmp/push_output.txt
exit 1
Comment on lines +104 to +125
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't let an unpushable autofix commit report success.

By this point the fixes are already committed locally. Returning 0 on a push rejection lets the later RuboCop step run against that local-only commit, so the job can go green while the branch still lacks the fixes. Since master and main are already gated above, this path should fail instead of warning-only.

🛠️ Suggested change
-                  if grep -qiE "(protected branch|permission|prohibited|rejected)" /tmp/push_output.txt; then
-                    echo "::warning::Cannot push to protected branch or insufficient permissions."
-                    echo "::warning::Rubocop fixes were not committed. Please apply rubocop fixes manually or adjust branch protection rules."
-                    exit 0
+                  if grep -qiE "(protected branch|permission|prohibited|rejected)" /tmp/push_output.txt; then
+                    echo "::error::Autocorrect changes were committed locally but could not be pushed back to ${{ github.ref_name }}."
+                    echo "::error::Please apply the fixes locally or update branch protection/permissions."
+                    exit 1
                   fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/rubocop_syntax_checker.yaml around lines 104 - 125, The
current push error handling in the git push block treats protected-branch
rejections as a warning and exits 0, which allows subsequent RuboCop steps to
run against unpushed local commits; update the handler in the block that
inspects PUSH_EXIT_CODE and /tmp/push_output.txt so that when grep detects
"protected branch|permission|prohibited|rejected" it emits the warning messages
but then exits non‑zero (exit 1) instead of exit 0. Keep the existing echo
"::warning::" lines for context, but change the final control flow so
protected/permission push failures fail the job (use exit 1) — this affects the
git push handling around the PUSH_EXIT_CODE logic and the grep check that reads
/tmp/push_output.txt.

fi
else
echo "::error::Failed to commit changes"
exit 1
fi
fi

- name: Run Rubocop check on changed files
if: steps.changed_files.outputs.has_changes == 'true'
run: |
echo "Running rubocop check on changed files..."
xargs -0 bundle exec rubocop < /tmp/changed_files.txt

- name: Summary
if: always() && steps.changed_files.outputs.has_changes == 'false'
run: |
for file in ${{ steps.changed-files.outputs.all_changed_files }}; do
bundle exec rubocop $file
done
echo "✅ No Ruby, .rbw, or .lic files were changed in this push"
Loading