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)
- 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
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