anthropics / anthropics/claude-code-action
checkContainsTrigger: ternary with two identical branches, and the trigger regex rebuilt five times
- Dominant language
- TypeScript
- Stars
- 8.9k
- Forks
- 2.1k
- PR merge metrics
- PR metrics pending
Description
**Type:** code quality / dead code
**Severity:** low
**Area:** `src/github/validation/trigger.ts`
**Effort:** trivial
## Summary
Two small defects in the trigger matcher, which is the function that decides
whether the action runs at all.
### 1. Ternary with identical branches
`src/github/validation/trigger.ts:128-130`
```ts
const commentBody = isIssueCommentEvent(context)
? context.payload.comment.body
: context.payload.comment.body;
```
Both arms are the same expression. The type narrowing is already established by
the enclosing `if (isIssueCommentEvent(context) || isPullRequestReviewCommentEvent(context))`,
so the ternary does nothing. It reads as though the two event shapes differ,
which is misleading for anyone touching this code - they do not, both payloads
expose `comment.body`.
### 2. The same regex is rebuilt five times
The identical `new RegExp(...)` literal appears at lines 54-57, 79-82, 106-109,
131-134 and inside the issue-title branch:
```ts
const regex = new RegExp(
`(^|\s)${escapeRegExp(triggerPhrase)}([\s.,!?;:]|$)`,
"i",
);
```
Five copies of a pattern that must stay in agreement. If the character class is
ever widened (say to accept a trailing `)` or `-`), four of the five sites will
keep the old behaviour and the trigger will fire for comments but not issue
bodies, or for PR titles but not review bodies. That class of divergence is
invisible until a user reports "@claude works in comments but not in my PR
description".
## Impact
No current misbehaviour - all five copies are presently identical, and the dead
ternary evaluates correctly. This is purely a maintenance hazard in a
high-traffic, security-relevant function.
## Suggested fix
```ts
/**
* Matches the trigger phrase as a whole token: preceded by start-of-string or
* whitespace, followed by whitespace, sentence punctuation, or end-of-string.
*/
function buildTriggerRegex(triggerPhrase: string): RegExp {
return new RegExp(`(^|\s)${escapeRegExp(triggerPhrase)}([\s.,!?;:]|$)`, "i");
}
```
Build it once at the top of `checkContainsTrigger` and reuse it in all five
branches. Replace the dead ternary with:
```ts
const commentBody = context.payload.comment.body;
```
`escapeRegExp` is already exported from this file, so the helper can live beside
it. Coverage exists in `test/trigger-validation.test.ts`.
Contributor guide
Assessment
This issue has not been assessed yet.