From f0dc83bd70e3c24aabf8661f4d4288a6fe3eaf4b Mon Sep 17 00:00:00 2001 From: aaight Date: Thu, 23 Apr 2026 22:04:49 +0200 Subject: [PATCH] fix(github): add pull_request_review_comment to webhook event lists (#1171) * fix(github): add pull_request_review_comment to webhook event lists * fix(github): import GITHUB_WEBHOOK_EVENTS in setup-webhooks to prevent 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 --------- Co-authored-by: Cascade Bot Co-authored-by: Claude Sonnet 4.6 --- src/api/routers/webhooks/github.ts | 3 +- src/router/adapters/github.ts | 2 +- .../github-webhook-events.test.ts | 44 +++++++++++++++++++ tools/setup-webhooks.ts | 8 +--- .../projects/integration-scm-tab.tsx | 2 +- 5 files changed, 49 insertions(+), 10 deletions(-) create mode 100644 tests/unit/integrations/github-webhook-events.test.ts diff --git a/src/api/routers/webhooks/github.ts b/src/api/routers/webhooks/github.ts index 3c352c69..d330f1ea 100644 --- a/src/api/routers/webhooks/github.ts +++ b/src/api/routers/webhooks/github.ts @@ -3,9 +3,10 @@ import { logger } from '../../../utils/logging.js'; import { parseRepoFullName } from '../../../utils/repo.js'; import type { GitHubWebhook, ProjectContext } from './types.js'; -const GITHUB_WEBHOOK_EVENTS = [ +export const GITHUB_WEBHOOK_EVENTS = [ 'pull_request', 'pull_request_review', + 'pull_request_review_comment', 'check_suite', 'issue_comment', ]; diff --git a/src/router/adapters/github.ts b/src/router/adapters/github.ts index 65bd914f..e187f5cb 100644 --- a/src/router/adapters/github.ts +++ b/src/router/adapters/github.ts @@ -95,7 +95,7 @@ async function postGitHubPRAck( return undefined; } -const PROCESSABLE_EVENTS = [ +export const PROCESSABLE_EVENTS = [ 'pull_request', 'pull_request_review', 'pull_request_review_comment', diff --git a/tests/unit/integrations/github-webhook-events.test.ts b/tests/unit/integrations/github-webhook-events.test.ts new file mode 100644 index 00000000..64aeff10 --- /dev/null +++ b/tests/unit/integrations/github-webhook-events.test.ts @@ -0,0 +1,44 @@ +/** + * GitHub webhook event-list consistency guard. + * + * The router processes events enumerated in `PROCESSABLE_EVENTS` + * (`src/router/adapters/github.ts`). For CASCADE to receive those events from + * GitHub, every processable event type must also appear in the + * `GITHUB_WEBHOOK_EVENTS` array used when programmatically creating webhooks + * (`src/api/routers/webhooks/github.ts`). + * + * This test prevents drift between the two lists. If a new event type is + * added to `PROCESSABLE_EVENTS` but forgotten in `GITHUB_WEBHOOK_EVENTS`, + * GitHub will never send that event payload to CASCADE — the trigger will + * silently never fire, which is the root cause this guard catches. + * + * Root-cause example: `pull_request_review_comment` was processed by + * `PRCommentMentionTrigger` but missing from the webhook creation list, so + * inline review comments were silently ignored. + */ + +import { describe, expect, it } from 'vitest'; +import { GITHUB_WEBHOOK_EVENTS } from '../../../src/api/routers/webhooks/github.js'; +import { PROCESSABLE_EVENTS } from '../../../src/router/adapters/github.js'; + +describe('GitHub webhook event-list consistency', () => { + it('GITHUB_WEBHOOK_EVENTS contains every event in PROCESSABLE_EVENTS', () => { + const missing = PROCESSABLE_EVENTS.filter((event) => !GITHUB_WEBHOOK_EVENTS.includes(event)); + + expect( + missing, + `GITHUB_WEBHOOK_EVENTS is missing event(s) that are in PROCESSABLE_EVENTS: ${missing.join(', ')}. ` + + `GitHub will never deliver these events to CASCADE unless they are included in the ` + + `webhook creation payload. Add the missing events to GITHUB_WEBHOOK_EVENTS in ` + + `src/api/routers/webhooks/github.ts.`, + ).toEqual([]); + }); + + it('GITHUB_WEBHOOK_EVENTS is a non-empty array', () => { + expect(GITHUB_WEBHOOK_EVENTS.length).toBeGreaterThan(0); + }); + + it('PROCESSABLE_EVENTS is a non-empty array', () => { + expect(PROCESSABLE_EVENTS.length).toBeGreaterThan(0); + }); +}); diff --git a/tools/setup-webhooks.ts b/tools/setup-webhooks.ts index bcca8fcc..a944b283 100644 --- a/tools/setup-webhooks.ts +++ b/tools/setup-webhooks.ts @@ -15,17 +15,11 @@ */ import { Octokit } from '@octokit/rest'; +import { GITHUB_WEBHOOK_EVENTS } from '../src/api/routers/webhooks/github.js'; import { closeDb } from '../src/db/client.js'; import { findProjectByIdFromDb } from '../src/db/repositories/configRepository.js'; import { resolveAllProjectCredentials } from '../src/db/repositories/credentialsRepository.js'; -const GITHUB_WEBHOOK_EVENTS = [ - 'pull_request', - 'pull_request_review', - 'check_suite', - 'issue_comment', -]; - interface TrelloWebhook { id: string; description: string; diff --git a/web/src/components/projects/integration-scm-tab.tsx b/web/src/components/projects/integration-scm-tab.tsx index 471eb15c..a09895c5 100644 --- a/web/src/components/projects/integration-scm-tab.tsx +++ b/web/src/components/projects/integration-scm-tab.tsx @@ -152,7 +152,7 @@ function GitHubWebhookSection({ projectId }: { projectId: string }) { " -d '{", ' "name": "web",', ' "active": true,', - ' "events": ["push", "pull_request", "check_suite", "pull_request_review"],', + ' "events": ["push", "pull_request", "pull_request_review", "pull_request_review_comment", "check_suite", "issue_comment"],', ' "config": {', ` "url": "${webhookCallbackUrl}",`, ' "content_type": "json"',