Fix annoying Formatting fails on PR synchronization #98

schewskone · 2026-01-29T16:16:46Z

Trying out a Claude script that prevents failure of Formatting jobs on PR synchronization

gitnotebooks · 2026-01-29T16:16:50Z

Review these changes at https://app.gitnotebooks.com/sensorium-competition/experanto/pull/98

Copilot

Pull request overview

This PR modifies the GitHub Actions CI workflow to address formatting job failures during PR synchronization. The changes focus on the lint-and-format job by adjusting when it runs and simplifying its execution logic.

Changes:

Modified job execution conditions to exclude main branch pushes and only run for forked PRs
Added explicit branch checkout configuration with ref parameter
Simplified the bash script logic for formatting and checking

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-29T16:19:56Z

.github/workflows/test.yml

+      if: |
+        (github.event_name == 'push' && github.ref != 'refs/heads/main') ||
+        (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository)


The workflow is missing explicit permissions configuration for the GITHUB_TOKEN. When the job attempts to push commits (line 81), it requires write access to the repository contents. Without explicit permissions set, the job relies on the default token permissions, which may be read-only depending on the repository settings. Add a permissions block with contents: write to ensure the token has the necessary permissions to push formatting commits.

Copilot · 2026-01-29T16:19:56Z

.github/workflows/test.yml

+      if: |
+        (github.event_name == 'push' && github.ref != 'refs/heads/main') ||
+        (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository)


The condition logic is inconsistent with the test job above (lines 15-20). The test job runs on all push events and forked PRs, while this formatting job excludes pushes to the main branch. This creates an inconsistency where tests run on main branch pushes but formatting doesn't. If the goal is to avoid conflicts with branch protection on main, consider whether the same logic should apply to the test job, or if the formatting job should handle main branch pushes differently (e.g., check-only mode).

Copilot · 2026-01-29T16:19:57Z

.github/workflows/test.yml

+      # 2. Run on PRs only if they come from a FORK (where we can't push)
+      if: |
+        (github.event_name == 'push' && github.ref != 'refs/heads/main') ||
+        (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository)


The workflow will never run for PRs from the same repository because the condition excludes them. This means that when a developer pushes to a PR branch in the same repo to synchronize it, the formatting job won't run at all. This could be the intended behavior to avoid the "annoying failures" mentioned in the PR title, but it leaves same-repo PRs without any formatting checks. Consider whether same-repo PRs should at least run in check-only mode to catch formatting issues before merge.

Suggested change

# 2. Run on PRs only if they come from a FORK (where we can't push)

if: |

(github.event_name == 'push' && github.ref != 'refs/heads/main') ||

(github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository)

# 2. Run on ALL PRs (forks and same-repo branches) in check-only mode

if: |

(github.event_name == 'push' && github.ref != 'refs/heads/main') ||

(github.event_name == 'pull_request')

Copilot · 2026-01-29T16:19:57Z

.github/workflows/test.yml

+
+              if ! git diff --quiet; then
+                git commit -am "style: auto-format with black and isort"
+                git push


The simplified logic in the bash script no longer validates that push events are from the same repository. The old code checked github.repository == github.event.repository.full_name before attempting to push. While the YAML condition on line 47 prevents the job from running on main branch pushes, it doesn't prevent the job from attempting to push to protected branches or branches where the GITHUB_TOKEN might not have write access. Consider adding a check or handling potential push failures gracefully.

Suggested change

git push

# Extra safety: only push if the event repository matches this repository

if [[ "${GITHUB_REPOSITORY}" == "${{ github.event.repository.full_name }}" ]]; then

echo "Repository match confirmed (${GITHUB_REPOSITORY}). Attempting to push formatting changes..."

if ! git push; then

echo "git push failed (likely due to permissions or branch protection); skipping auto-push."

fi

else

echo "Repository mismatch (GITHUB_REPOSITORY=${GITHUB_REPOSITORY}, event.repository.full_name=${{ github.event.repository.full_name }})."

echo "Skipping git push to avoid pushing to an unexpected repository."

fi

Copilot · 2026-01-29T16:19:57Z

.github/workflows/test.yml

+              black --check . || { echo "Black failed"; exit 1; }
+              isort --check-only . || { echo "Isort failed"; exit 1; }


The error messages have been changed from more informative messages that explain how to fix the issue (e.g., "Black formatting issues found. Run 'black .' to fix.") to terse messages ("Black failed"). The more detailed error messages help developers understand what went wrong and how to resolve it. Consider keeping the more descriptive error messages from the original implementation.

Suggested change

black --check . || { echo "Black failed"; exit 1; }

isort --check-only . || { echo "Isort failed"; exit 1; }

black --check . || { echo "Black formatting issues found. Run 'black .' to fix them locally."; exit 1; }

isort --check-only . || { echo "Isort formatting issues found. Run 'isort .' to fix them locally."; exit 1; }

Copilot · 2026-01-29T16:19:58Z

.github/workflows/test.yml

+      name: Lint & Auto-format
+      runs-on: ubuntu-latest
+      # IMPROVED LOGIC: 
+      # 1. Run on pushes to any branch EXCEPT main (to avoid protection issues)
+      # 2. Run on PRs only if they come from a FORK (where we can't push)
+      if: |
+        (github.event_name == 'push' && github.ref != 'refs/heads/main') ||
+        (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository)

-    steps:
-      - name: Checkout repository
-        uses: actions/checkout@v4
+      steps:
+        - name: Checkout repository
+          uses: actions/checkout@v4
+          with:
+            # Use a PAT if you want the 'auto-format' commit to trigger other CI tests
+            # Use GITHUB_TOKEN if you want it to be "silent"
+            token: ${{ secrets.GITHUB_TOKEN }}
+            ref: ${{ github.event.pull_request.head.ref || github.ref_name }}

-      - name: Set up Python
-        uses: actions/setup-python@v5
-        with:
-          python-version: '3.12.8'
+        - name: Set up Python
+          uses: actions/setup-python@v5
+          with:
+            python-version: '3.12.8'

-      - name: Install tools
-        run: |
-          python -m pip install --upgrade pip
-          pip install "black[jupyter]==25.1.0" isort==6.0.1
+        - name: Install tools
+          run: |
+            python -m pip install --upgrade pip
+            pip install "black[jupyter]==25.1.0" isort==6.0.1

-      - name: Run Black and Isort
-        run: |
-          if [[ "${{ github.event_name }}" == "push" && "${{ github.repository }}" == "${{ github.event.repository.full_name }}" ]]; then
-            echo "Push event in same repo: Running black and isort with auto-format and commit"
-            black .
-            isort .
-            git config user.name "github-actions"
-            git config user.email "github-actions@github.com"
-            if ! git diff --quiet; then
-              git commit -am "style: auto-format with black and isort"
-              git push
-            else
-              echo "No formatting changes to commit."
-            fi
+        - name: Run Black and Isort
+          run: |
+            if [[ "${{ github.event_name }}" == "push" ]]; then
+              echo "Standard Branch Push: Formatting and pushing back to ${{ github.ref_name }}"
+              black .
+              isort .
+
+              git config user.name "github-actions"
+              git config user.email "github-actions@github.com"
+
+              if ! git diff --quiet; then
+                git commit -am "style: auto-format with black and isort"
+                git push
+              else
+                echo "No changes."
+              fi

-          elif [[ "${{ github.event_name }}" == "pull_request" ]]; then
-            echo "PR from fork: Running black and isort in check mode"
-            black --check . || { echo "Black formatting issues found. Run 'black .' to fix."; exit 1; }
-            isort --check-only . || { echo "isort import order issues found. Run 'isort .' to fix."; exit 1; }
-
-          else
-            echo "Not a forked PR and not a push. Skipping format."
-          fi
+            else
+              echo "Forked PR: Check-only mode (No push)"
+              black --check . || { echo "Black failed"; exit 1; }
+              isort --check-only . || { echo "Isort failed"; exit 1; }
+            fi


The entire lint-and-format job definition has incorrect indentation. Comparing with the test job above (lines 12-38), job properties like name, runs-on, if, and steps should be indented with 4 spaces from the job key. Currently, these properties are indented with 6 spaces. This inconsistency may cause YAML parsing issues or workflow failures.

Copilot · 2026-01-29T16:19:58Z

.github/workflows/test.yml

+            # Use a PAT if you want the 'auto-format' commit to trigger other CI tests
+            # Use GITHUB_TOKEN if you want it to be "silent"
+            token: ${{ secrets.GITHUB_TOKEN }}
+            ref: ${{ github.event.pull_request.head.ref || github.ref_name }}


The checkout step for PR events from forks may not work correctly. The ref parameter uses github.event.pull_request.head.ref, which provides only the branch name (e.g., "feature-branch"), not a full Git reference. For forks, this will attempt to check out a branch that doesn't exist in the base repository. A proper approach would be to checkout the head SHA or use github.event.pull_request.head.sha to ensure the correct code is checked out from the fork.

Suggested change

ref: ${{ github.event.pull_request.head.ref || github.ref_name }}

trying fix suggested by claude

9875146

Copilot AI review requested due to automatic review settings January 29, 2026 16:16

Copilot started reviewing on behalf of schewskone January 29, 2026 16:17 View session

schewskone closed this Jan 29, 2026

Copilot AI reviewed Jan 29, 2026

View reviewed changes

schewskone deleted the fix_automated_formatting branch January 29, 2026 16:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix annoying Formatting fails on PR synchronization #98

Fix annoying Formatting fails on PR synchronization #98

Uh oh!

schewskone commented Jan 29, 2026

Uh oh!

gitnotebooks bot commented Jan 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 29, 2026

Uh oh!

Copilot AI Jan 29, 2026

Uh oh!

Copilot AI Jan 29, 2026

Uh oh!

Copilot AI Jan 29, 2026

Uh oh!

Copilot AI Jan 29, 2026

Uh oh!

Copilot AI Jan 29, 2026

Uh oh!

Copilot AI Jan 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

-                git push
+                # Extra safety: only push if the event repository matches this repository
+                if [[ "${GITHUB_REPOSITORY}" == "${{ github.event.repository.full_name }}" ]]; then
+                  echo "Repository match confirmed (${GITHUB_REPOSITORY}). Attempting to push formatting changes..."
+                  if ! git push; then
+                    echo "git push failed (likely due to permissions or branch protection); skipping auto-push."
+                  fi
+                else
+                  echo "Repository mismatch (GITHUB_REPOSITORY=${GITHUB_REPOSITORY}, event.repository.full_name=${{ github.event.repository.full_name }})."
+                  echo "Skipping git push to avoid pushing to an unexpected repository."
+                fi

		black --check . \|\| { echo "Black failed"; exit 1; }
		isort --check-only . \|\| { echo "Isort failed"; exit 1; }

Fix annoying Formatting fails on PR synchronization #98

Fix annoying Formatting fails on PR synchronization #98

Uh oh!

Conversation

schewskone commented Jan 29, 2026

Uh oh!

gitnotebooks bot commented Jan 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant