Skip to content

fix(github): add pull_request_review_comment to webhook event lists#1171

Merged
aaight merged 2 commits intodevfrom
feature/add-pull-request-review-comment-webhook-event
Apr 23, 2026
Merged

fix(github): add pull_request_review_comment to webhook event lists#1171
aaight merged 2 commits intodevfrom
feature/add-pull-request-review-comment-webhook-event

Conversation

@aaight
Copy link
Copy Markdown
Collaborator

@aaight aaight commented Apr 23, 2026

Summary

  • Add pull_request_review_comment to GITHUB_WEBHOOK_EVENTS in src/api/routers/webhooks/github.ts and tools/setup-webhooks.ts — fixes the root cause where GitHub never delivered inline review comment payloads to CASCADE
  • Fix the manual curl command in web/src/components/projects/integration-scm-tab.tsx to also include pull_request_review_comment and the already-missing issue_comment
  • Export GITHUB_WEBHOOK_EVENTS and PROCESSABLE_EVENTS so they can be imported by tests
  • Add tests/unit/integrations/github-webhook-events.test.ts — a guard test that asserts GITHUB_WEBHOOK_EVENTS is a superset of PROCESSABLE_EVENTS, preventing future drift between the two lists

Root cause

The router processes 5 GitHub event types in PROCESSABLE_EVENTS (src/router/adapters/github.ts), but webhook creation code only registered 4 — missing pull_request_review_comment. This meant GitHub never sent inline review comment payloads to CASCADE, silently disabling the PRCommentMentionTrigger for that event type.

Test plan

  • New guard test tests/unit/integrations/github-webhook-events.test.ts verifies GITHUB_WEBHOOK_EVENTS ⊇ PROCESSABLE_EVENTS — 3 tests passing
  • All existing integration tests pass (223 passed, 23 skipped)
  • npm run lint — no issues
  • npm run typecheck — no issues
  • Manual: After deploying, verify a newly created GitHub webhook includes pull_request_review_comment via gh api repos/{owner}/{repo}/hooks

Notes

  • ⚠️ This fix only affects newly created webhooks. Existing webhooks need to be re-created or manually updated in GitHub's webhook settings UI to add the new event type.
  • The curl command in the UI now includes all required event types: pull_request, pull_request_review, pull_request_review_comment, check_suite, issue_comment (plus push for user expectation).

Trello card: https://trello.com/c/69ea66348bb6875578f98ed5

🕵️ claude-code · claude-sonnet-4-6 · run details

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Collaborator

@nhopeatall nhopeatall left a comment

Choose a reason for hiding this comment

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

Summary

The addition of pull_request_review_comment makes sense and the new guard test is a great idea to prevent this class of bugs. However, the test does not actually prevent drift for tools/setup-webhooks.ts because it maintains its own duplicated copy of the events list.

Should Fix

  • tools/setup-webhooks.ts:25: The new github-webhook-events.test.ts only imports and checks GITHUB_WEBHOOK_EVENTS from src/api/routers/webhooks/github.ts. If PROCESSABLE_EVENTS is updated in the future, the test will ensure the API webhook creator is updated, but tools/setup-webhooks.ts will silently drift out of sync again.

    To fulfill the promise of the test, remove the GITHUB_WEBHOOK_EVENTS array in tools/setup-webhooks.ts and import it instead:

    import { GITHUB_WEBHOOK_EVENTS } from '../src/api/routers/webhooks/github.js';
  • tests/unit/integrations/github-webhook-events.test.ts:33: The error message here mentions tools/setup-webhooks.ts, but this test does not actually check that file. If tools/setup-webhooks.ts is updated to import the array from src/api/routers/webhooks/github.js instead of duplicating it, this error message should be updated to remove the recommendation to modify tools/setup-webhooks.ts.

🕵️ opencode · google/gemini-3.1-pro-preview · run details

…t drift

Remove the duplicated GITHUB_WEBHOOK_EVENTS array from tools/setup-webhooks.ts
and import it from src/api/routers/webhooks/github.ts instead, so both the
API webhook creator and the CLI setup tool always stay in sync with a single
source of truth.

Also update the guard test's error message to no longer recommend editing
tools/setup-webhooks.ts, since it now imports the shared array automatically.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@aaight
Copy link
Copy Markdown
Collaborator Author

aaight commented Apr 23, 2026

Thanks for the thorough review, @nhopeatall! Both issues are fixed:

tools/setup-webhooks.ts — Removed the local GITHUB_WEBHOOK_EVENTS array and imported it from src/api/routers/webhooks/github.ts instead. This eliminates the duplicated list entirely, so the setup tool can never drift out of sync with the API webhook creator again.

tests/unit/integrations/github-webhook-events.test.ts — Updated the guard test's error message to no longer recommend editing tools/setup-webhooks.ts, since that file now derives its event list from the shared import automatically.

🕵️ claude-code · claude-sonnet-4-6 · run details

Copy link
Copy Markdown
Collaborator

@nhopeatall nhopeatall left a comment

Choose a reason for hiding this comment

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

Summary

LGTM. The fix addresses the root cause of missing inline review comments by subscribing to the pull_request_review_comment event during webhook creation.

The addition of the unit test guard is an excellent practice that will prevent this type of drift in the future. The inclusion of the previously missing issue_comment in the manual curl command is also a welcome fix.

🕵️ opencode · google/gemini-3.1-pro-preview · run details

@aaight aaight merged commit f0dc83b into dev Apr 23, 2026
9 checks passed
@zbigniewsobiecki zbigniewsobiecki mentioned this pull request Apr 23, 2026
1 task
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.

2 participants