Skip to content

ci: auto lint/format fix in CI + test with bad formatting #172

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.

Sign up for GitHub

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

Open
snomiao wants to merge 3 commits into
base: main
Choose a base branch
from
Open

ci: auto lint/format fix in CI + test with bad formatting #172

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
47eea8c
ci: add auto lint/format fix and commit to test workflow
snomiao Feb 19, 2026
7a98c87
fix: checkout PR branch ref to allow git push from CI
snomiao Feb 19, 2026
ad4840d
Merge branch 'main' into sno-fix-msg-post-2026-02-18
snomiao Mar 2, 2026
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
17 changes: 16 additions & 1 deletion .github/workflows/test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@ on:
branches:
- main
jobs:
run_comfy_pr:
comfy_pr_test:
runs-on: ubuntu-latest
permissions:
contents: write
timeout-minutes: 10
steps:
- uses: actions/checkout@v4
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

The PR description mentions adding "intentionally bad formatting in lib/slack/index.ts to verify the auto-fix works", but this file is not included in the changes. To properly test the auto-fix functionality, you should include a file with formatting issues in the PR. Without this, the CI workflow changes cannot be verified to work as intended.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

The checkout action needs to be configured to checkout the PR branch with proper ref when running on pull_request events. By default, actions/checkout@v4 checks out a merge commit in pull_request context, which prevents pushing back to the PR branch. You should add ref: ${{ github.head_ref }} to the checkout step to ensure the actual PR branch is checked out, allowing the auto-commit to work properly.

Copilot uses AI. Check for mistakes.
with:
ref: ${{ github.head_ref }}
# setup comfy-cli
- uses: actions/setup-python@v5
with:
Expand All @@ -30,6 +34,17 @@ jobs:
# setup comfy-pr
# Run Comfy-PR Tests
- run: bun i
- run: bunx oxlint --fix
- run: bunx oxfmt --write
- name: Commit lint/format fixes
run: |
git config user.name "github-actions[bot]"
git config user.email "github-actions[bot]@users.noreply.github.com"
if ! git diff --quiet; then
git add -A
Comment on lines +37 to +44
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

The git push command will fail when trying to push to a PR from a forked repository because workflows triggered by pull_request events from forks have restricted permissions and cannot push to the fork. This is a GitHub security feature. Consider using a token-based push mechanism or only enabling this auto-fix feature for PRs from branches in the same repository. You should add a condition to check if the PR is from a fork before attempting the push.

Copilot uses AI. Check for mistakes.
git commit -m "style: auto-fix lint and formatting"
Comment on lines +37 to +45
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

When the auto-fix commits changes and pushes to the PR branch, this will trigger a new pull_request synchronize event, which could re-run this workflow. While the second run won't find any changes to commit (since the code is already fixed), this causes unnecessary CI runs. Consider adding a condition to skip the workflow when the latest commit author is 'github-actions[bot]', or use [skip ci] in the commit message to prevent re-triggering the workflow.

Copilot uses AI. Check for mistakes.
git push
Comment on lines +39 to +46
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

Running the auto-fix before tests creates a potential issue where tests will run against code that hasn't been fixed yet if there are no changes, or against fixed code if there are changes. This could lead to inconsistent test results. Consider either: 1) Running tests both before and after auto-fix to ensure they pass in both states, 2) Moving the auto-fix to a separate job that runs after tests pass, or 3) Failing the workflow if fixes are needed and requiring developers to run the linter locally before pushing.

Suggested change
- name: Commit lint/format fixes
run: |
git config user.name "github-actions[bot]"
git config user.email "github-actions[bot]@users.noreply.github.com"
if ! git diff --quiet; then
git add -A
git commit -m "style: auto-fix lint and formatting"
git push
- name: Check lint/format fixes
run: |
if ! git diff --quiet; then
echo "Lint/formatting issues detected."
echo "Please run 'bunx oxlint --fix' and 'bunx oxfmt --write' locally and commit the changes before pushing."
exit 1

Copilot uses AI. Check for mistakes.
fi
- run: bun test
timeout-minutes: 8
env:
Expand Down
Loading