Comfy-Org / Comfy-Org/ComfyUI_frontend

check-ai-co-authors flags main's own commits on any rebased PR (two-dot range from a stale base.sha)

Open Beginner friendly
#17,359 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

## Problem / Goal

`.github/scripts/check-ai-co-authors.sh` walks a two-dot range:

```
git log --format=' %h: %(trailers:key=Co-authored-by,separator=%x09)' "${base_sha}..${head_sha}"
```

and `ci-check-ai-co-authors.yaml` passes `${{ github.event.pull_request.base.sha }}` as `base_sha`. For a `pull_request` event that value is a snapshot of the base branch from when the PR was opened, and it does not move as `main` advances. So once a PR is rebased onto a newer `main`, `base_sha..head_sha` contains every `main` commit merged in between, and the check reports `main`'s own history as violations.

Seen on #16584. The branch was 8 days behind and had to be rebased to clear a conflict. Afterwards the check failed with 28 commits listed, all of them on `main` and none of them on the PR branch:

```
7fa1b02646: Co-authored-by: Claude # feat: add comment-sicko PR review skill (#17170)
b97b6316d7: Co-authored-by: Claude Sonnet 4.6 ...
2b6bb4b25a: Co-authored-by: Cursor Agent
...
```

The base SHA it printed in the `git rebase -i` hint was `adb54e6243` (`[cjx-5] (#16496)`, Sep 2), the PR's original base.

Corroborating evidence from the same push: both a `push`-triggered and a `pull_request`-triggered run fired on head `9d44b419fe`. The `push` run (#7958) passed, the `pull_request` run (#7959) failed. Same commits, different base.

Effect: every rebased PR gets a red X on this check, and the suggested remedy (`git rebase -i ` and drop the trailers) would rewrite `main`'s history if anyone followed it. It also trains people to ignore the check.

## Proposed Solution

Use the merge base rather than the recorded base. One-liner in the script:

```diff
- "${base_sha}..${head_sha}"
+ "${base_sha}...${head_sha}"
```

or equivalently resolve `git merge-base "$base_sha" "$head_sha"` first and use that with two dots. Three-dot with `git log` gives commits reachable from head but not from base, which is what the check wants. `fetch-depth: 0` is already set, so the merge base is available.

## Acceptance Criteria

- [ ] A PR rebased onto a `main` that contains commits carrying agent `Co-authored-by` trailers passes the check.
- [ ] A PR whose own commits carry an agent trailer still fails the check.
- [ ] The `git rebase -i ` hint in the failure message names the merge base, not the PR's original base.

Contributor guide

Open the contributing guide

Research direction

Read .github/scripts/check-ai-co-authors.sh and ci-check-ai-co-authors.yaml, then inspect how the pull_request base and head SHAs reach the script. Run the existing check against a rebased branch whose base contains agent trailers and against a branch with an agent trailer of its own. Done means the former passes, the latter fails, and the failure hint uses the merge base.

Written by the indexing model from the issue text.

Assessment

Tech stack
bash, git, github-actions
Domain
ci-cd
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
85/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.