Skip to content

Accept CLA signature comments with trailing punctuation - #156

Open
Dynoooooo wants to merge 1 commit into
drawthingsai:mainfrom
Dynoooooo:cla-signature-match
Open

Dynoooooo wants to merge 1 commit into
drawthingsai:mainfrom
Dynoooooo:cla-signature-match

Conversation

@Dynoooooo

Copy link
Copy Markdown

The CLA workflow only runs its step when the comment body is exactly the sign phrase. A comment like "I have read the CLA Document and I hereby sign the CLA." (trailing period), or one with a trailing newline, skips the step without recording a signature, and the run still reports success. This happened on #155: the first signing comment had a period, and only a second, exact comment went through.

The action itself is lenient. At v2.3.0, src/pullrequest/signatureComment.ts trims and lowercases the comment and matches /^.*i \s*have \s*read \s*the \s*cla \s*document \s*and \s*i \s*hereby \s*sign \s*the \s*cla.*$/. So the only strict check is the if: gate, which keeps such comments from reaching the action.

This changes the gate to contains(...). GitHub Actions' contains() is case-insensitive, so the gate now passes what the action accepts, and the action still decides whether a comment is a valid signature. recheck and the pull_request_target trigger are unchanged. Because pull_request_target and issue_comment run the workflow from main, this PR's own check still uses the current workflow.

🤖 Generated with Claude Code

The workflow gate compared the comment body with the sign phrase using
exact equality, so "... I hereby sign the CLA." (or a trailing newline)
skipped the CLA Assistant step without recording a signature, while the
run still reported success. contains() is case-insensitive and matches
what the action itself accepts (it trims, lowercases and allows text
around the phrase), so the action stays the authority on the wording.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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