Skip to content

fix: treat an issue comment without a body as having no commands - #374

Merged
jeefy merged 1 commit into
cncf:mainfrom
jeefy:fix/comment-without-body
Oct 8, 2026
Merged

jeefy merged 1 commit into
cncf:mainfrom
jeefy:fix/comment-without-body

Conversation

@jeefy

@jeefy jeefy commented Oct 8, 2026

Copy link
Copy Markdown
Member

Bug

GitHub allows an empty issue comment; its comment.body arrives as null. handleIssueComment passed context.payload.comment?.body unguarded to hasCommand, whose line splitting (commandLines → splitLines) calls body.replace(...), so the run fails:

TypeError: Cannot read properties of null (reading 'replace')

(... of undefined ... when the body or the comment is absent).

Fix

handleIssueComment reads a missing/null body as '': no command matches, nothing runs, the run succeeds. One-line guard, no refactor. I checked the other readers of comment.body: the handlers only run after hasCommand has matched (so the body is non-empty), /meow already checks typeof body === 'string', and the approve plugin / comment scanners already use body ?? '' — the dispatcher was the only reachable crash. dist/ regenerated with npm run pack.

Tests

  • New __tests__/issueCommentTest/commentWithoutBody.test.ts, parametrized over a null body, a comment with no body field and a payload with no comment, with nine commands configured: handleIssueComment resolves, setFailed is not called, no fetch, and no API request (msw with failOnUnhandledRequest plus a request:start recorder). All three fail on main with the TypeError above and pass here.
  • npm run all (1828 tests), npm run test:coverage (100 / 99.82 / 100 / 100), npm run test:coverage:e2e pass; a second npm run pack leaves dist/ clean.

Note: open PR #361 pins this crash text to exercise run()'s catch; after this lands a null body no longer throws, so that test will need to reach the catch another way.

GitHub allows an empty issue comment, whose `comment.body` arrives as
null. handleIssueComment passed it straight to hasCommand, whose line
splitting calls `body.replace(...)`, so the run failed with
`TypeError: Cannot read properties of null (reading 'replace')`.

A missing or null body is now read as the empty string: no command
matches, nothing is called and the run succeeds. The handlers only run
after hasCommand has matched, and /meow, the approve plugin and the
comment scanners already guard their bodies, so the dispatcher is the
only place the crash was reachable.

Signed-off-by: Jeffrey Sica <me@jeefy.dev>
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