Skip to content

fix(workflows): replace invalid trim() in LGTM matcher [DOPS-600] #2

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

Merged
kevnm67 merged 1 commit into from
Apr 30, 2026
Merged

fix(workflows): replace invalid trim() in LGTM matcher [DOPS-600] #2

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion .github/workflows/merge-on-green.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ jobs:
(
github.event_name == 'issue_comment' &&
github.event.issue.pull_request &&
contains(fromJSON('["LGTM","lgtm","Lgtm"]'), trim(github.event.comment.body))
(contains(github.event.comment.body, 'LGTM') || contains(github.event.comment.body, 'lgtm'))

@sentry sentry Bot Apr 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The workflow trigger condition incorrectly uses a substring match for "LGTM", which can cause it to run on non-approval comments.
Severity: MEDIUM

Suggested Fix

Revert to the previous logic using contains(fromJSON('["LGTM", "lgtm", "Lgtm"]'), trim(github.event.comment.body)) to perform an exact match against a list of approval strings. This prevents unintended triggers from comments that only contain the approval text as a substring.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: .github/workflows/merge-on-green.yml#L24

Potential issue: The workflow trigger condition was changed from an exact match against
an array of approval strings to a substring search. This is overly permissive and will
cause the merge workflow to trigger on comments that are not approvals but contain the
substring "LGTM", such as "This is NOT LGTM". Additionally, the mixed-case `Lgtm`
variant was removed from the condition, so that specific approval comment will no longer
trigger the workflow.

Did we get this right? 👍 / 👎 to inform future reviews.

) ||
(
github.event_name == 'pull_request_review' &&
Expand Down