anthropics / anthropics/claude-code-action

checkContainsTrigger: ternary with two identical branches, and the trigger regex rebuilt five times

Open
#1,766 1 comment 0 reactions 0 assignees View on GitHub
dev-experience good first issue p3
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.