Skip to content

fix(ci): compare the CLA ledger with the fork point, not main's tip - #190

Merged
shockalotti merged 1 commit into
mainfrom
fix/cla-guard-merge-base
Oct 29, 2026
Merged

shockalotti merged 1 commit into
mainfrom
fix/cla-guard-merge-base

Conversation

@shockalotti

Copy link
Copy Markdown
Contributor

The bug (mine)

The append-only guard I added in #161 compares the pull request's ledger against the tip of the base branch. Signatures are committed to main by the CLA bot, so a signature added after a PR was opened is absent from its head by construction. Comparing with today's main calls that "removed".

That is what hit four PRs at once:

#176, #179, #158, #159 → check=fail
##[error]Process completed with exit code 1     (CLA ledger is append-only)

MohammadHadi2031 signed, main grew from 11 to 12 ledger entries, and every PR forked before that point was reported as revoking his signature. Nothing was removed from any of them.

Fix

Compare against git merge-base HEAD origin/<base> instead, which only sees entries the PR's own branch actually had.

  • Checkout fetches full history for this job so the merge base is computable. The repository is 8MB.
  • A shallow clone that still cannot compute a merge base now warns and skips rather than failing closed on an infrastructure limitation.

Verified against the real case

base compared result
main tip (old) reports MohammadHadi2031 (id 17288524) removed, exit 1
merge-base (new) nothing removed, exit 0
merge-base, one signature genuinely deleted fails, naming tgrahammer (id 3223680)

So the guard still catches a real removal; it just no longer mistakes a later signature for one.

YAML parses and the shell block passes bash -n.

The append-only guard added in #161 compares the pull request's ledger with the
tip of the base branch. Signatures are committed to main by the CLA bot, so any
signature added after a PR was opened is absent from its head by construction.
Comparing with today's main calls that "removed" and fails the PR.

That is what happened to #176, #179, #158 and #159 at once: MohammadHadi2031
signed, the ledger on main grew from 11 to 12 entries, and every PR forked
before that point was reported as revoking his signature. Nothing was removed.

Compare against git merge-base HEAD origin/<base> instead, which only sees
entries the PR's own branch actually had. A shallow clone cannot compute a merge
base, so warn and skip rather than fail closed on an infrastructure limitation.
Checkout now fetches full history for this job; the repository is 8MB.

Verified against the real #176: base=main tip reports
"MohammadHadi2031 (id 17288524)" as removed and exits 1; base=merge-base
reports nothing and exits 0. A ledger with one signature genuinely deleted
still fails, naming tgrahammer (id 3223680).
@shockalotti
shockalotti merged commit 3771ad2 into main Oct 29, 2026
11 checks passed

@pullfrog pullfrog Bot left a comment

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.

✅ No new issues found.

Reviewed changes

Reviewed the single-file CI change that re-anchors the CLA append-only guard to the PR's merge base.

  • Full-history checkout — fetch-depth: 0 added so git merge-base can be computed in the job.
  • Merge-base comparison — the guard now diffs the PR head against git merge-base HEAD origin/<base> instead of today's main tip, so a signature added to main after the PR's merge ref was generated is no longer misreported as revoked.
  • Skip when no merge base — a missing merge base now logs a warning and exits 0 rather than failing closed.
  • Bootstrap case preserved — a missing ledger at the merge base still exits 0 with the same message.

I verified the premise locally: for pull/176/merge, git merge-base FETCH_HEAD origin/main resolves to the merge commit's first parent (the base tip at merge-ref generation), and the ledger at that base, at the merge commit, and at today's origin/main all carry the same 12 ids, including MohammadHadi2031. The merge-commit working tree also reflects head-branch deletions, so a genuine removal is still caught against a base that still holds the id. bash -n on the extracted step passes and the YAML parses.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash | 𝕏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant