Skip to content

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

Merged
kevnm67 merged 1 commit into
mainfrom
fix/DOPS-600-trim-in-merge-on-green
Apr 30, 2026
Merged

fix(workflows): replace invalid trim() in LGTM matcher [DOPS-600]#2
kevnm67 merged 1 commit into
mainfrom
fix/DOPS-600-trim-in-merge-on-green

Conversation

@kevnm67

@kevnm67 kevnm67 commented Apr 29, 2026

Copy link
Copy Markdown
Member

trim() is not a valid GHA expression — silently broke LGTM-by-comment merge. See LocumTenens-com/ltx#117 for source fix.

Test plan

  • Comment LGTM on a PR → workflow fires

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

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.

@kevnm67
kevnm67 merged commit 426325f into main Apr 30, 2026
2 checks passed
@kevnm67
kevnm67 deleted the fix/DOPS-600-trim-in-merge-on-green branch April 30, 2026 00:06
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