From 74fa6556da6ad09b8a4e4bd076880795252918fd Mon Sep 17 00:00:00 2001 From: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com> Date: Fri, 19 Dec 2025 16:20:31 -0500 Subject: [PATCH 1/2] feat: include PR conversation comments for review_comments subscribers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When user subscribes to review_comments, PR conversation comments (from the Conversation tab) are now also delivered. This matches user expectation that review_comments covers all PR comments. - Issue comments still only match "comments" subscribers - Inline code review comments still only match "review_comments" - PR conversation comments now match both 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- src/github-app/event-processor.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/github-app/event-processor.ts b/src/github-app/event-processor.ts index a27c446..2b4d7f0 100644 --- a/src/github-app/event-processor.ts +++ b/src/github-app/event-processor.ts @@ -101,9 +101,18 @@ export class EventProcessor { ); // Filter by event preferences - let interestedChannels = channels.filter(ch => - ch.eventTypes.includes(eventType) - ); + let interestedChannels = channels.filter(ch => { + if (ch.eventTypes.includes(eventType)) return true; + // PR conversation comments also notify review_comments subscribers + if ( + eventType === "comments" && + entityContext?.parentType === "pr" && + ch.eventTypes.includes("review_comments") + ) { + return true; + } + return false; + }); // Apply branch filtering for branch-specific events if (branch) { From 6bfd54f670c176e56e7fe097532a66bbd64922d9 Mon Sep 17 00:00:00 2001 From: Shuhui Luo <107524008+shuhuiluo@users.noreply.github.com> Date: Fri, 19 Dec 2025 16:49:47 -0500 Subject: [PATCH 2/2] test: add PR comment routing tests and document behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add unit tests verifying: - comments subscribers receive PR conversation comments - review_comments subscribers receive PR conversation comments - review_comments subscribers do NOT receive issue comments Update README to document comment routing behavior. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- README.md | 6 +- tests/unit/github-app/event-processor.test.ts | 125 +++++++++++++++++- 2 files changed, 126 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 7ebf3d4..61c1cc5 100644 --- a/README.md +++ b/README.md @@ -68,6 +68,8 @@ First-time subscriptions open an OAuth window; after authorization the callback **Event types:** `pr`, `issues`, `commits`, `releases`, `ci`, `comments`, `reviews`, `branches`, `review_comments`, `forks`, `stars`, `all` +> **Comment routing:** `comments` includes both issue comments and PR conversation comments. `review_comments` covers inline code review comments AND also receives PR conversation comments (useful for PR-focused subscriptions without issue noise). + **Branch filter:** `--branches main,develop` or `--branches release/*` or `--branches all` (default: default branch only) > Branch filtering applies to: `pr`, `commits`, `ci`, `reviews`, `review_comments`, `branches`. Other events (`issues`, `releases`, `comments`, `forks`, `stars`) are not branch-specific. @@ -109,9 +111,9 @@ First-time subscriptions open an OAuth window; after authorization the callback - `push` - Commits to branches - `release` - Published - `workflow_run` - CI/CD status -- `issue_comment` - New comments (threaded to PR/issue) +- `issue_comment` - Comments on issues and PR conversations (threaded; routes to `comments`, also `review_comments` for PRs) - `pull_request_review` - PR reviews (threaded to PR) -- `pull_request_review_comment` - Review comments (threaded to PR) +- `pull_request_review_comment` - Inline code review comments (threaded to PR; routes to `review_comments`) - `create` / `delete` - Branch/tag creation and deletion - `fork` - Repository forks - `watch` - Repository stars diff --git a/tests/unit/github-app/event-processor.test.ts b/tests/unit/github-app/event-processor.test.ts index 49b6d75..ea24c2f 100644 --- a/tests/unit/github-app/event-processor.test.ts +++ b/tests/unit/github-app/event-processor.test.ts @@ -1,6 +1,14 @@ -import { describe, expect, test } from "bun:test"; - -import { matchesBranchFilter } from "../../../src/github-app/event-processor"; +import { describe, expect, mock, test } from "bun:test"; + +import type { EventType } from "../../../src/constants"; +import type { GitHubApp } from "../../../src/github-app/app"; +import { + EventProcessor, + matchesBranchFilter, +} from "../../../src/github-app/event-processor"; +import type { MessageDeliveryService } from "../../../src/services/message-delivery-service"; +import type { SubscriptionService } from "../../../src/services/subscription-service"; +import type { IssueCommentPayload } from "../../../src/types/webhooks"; describe("matchesBranchFilter", () => { const defaultBranch = "main"; @@ -133,3 +141,114 @@ describe("matchesBranchFilter", () => { }); }); }); + +describe("PR comment routing", () => { + // Helper to create minimal IssueCommentPayload for testing + const createIssueCommentPayload = ( + isPrComment: boolean + ): IssueCommentPayload => + ({ + action: "created", + repository: { + full_name: "owner/repo", + default_branch: "main", + }, + issue: { + number: 123, + // GitHub includes pull_request field only for PR comments + ...(isPrComment ? { pull_request: { url: "https://..." } } : {}), + }, + comment: { + id: 456, + updated_at: new Date().toISOString(), + }, + }) as unknown as IssueCommentPayload; + + // Helper to create mock services + const createMocks = (channels: Array<{ eventTypes: EventType[] }>) => { + const deliveredChannels: string[] = []; + + const mockSubscriptionService = { + getRepoSubscribers: mock(() => + Promise.resolve( + channels.map((ch, i) => ({ + channelId: `channel-${i}`, + spaceId: `space-${i}`, + eventTypes: ch.eventTypes, + branchFilter: "all" as const, + })) + ) + ), + } as unknown as SubscriptionService; + + const mockMessageDeliveryService = { + deliver: mock(({ channelId }: { channelId: string }) => { + deliveredChannels.push(channelId); + return Promise.resolve(); + }), + } as unknown as MessageDeliveryService; + + const mockGitHubApp = {} as GitHubApp; + + const processor = new EventProcessor( + mockGitHubApp, + mockSubscriptionService, + mockMessageDeliveryService + ); + + return { processor, deliveredChannels }; + }; + + test("channel subscribed to 'comments' receives PR conversation comment", async () => { + const { processor, deliveredChannels } = createMocks([ + { eventTypes: ["comments"] }, + ]); + + await processor.onIssueComment(createIssueCommentPayload(true)); + + expect(deliveredChannels).toContain("channel-0"); + }); + + test("channel subscribed to 'review_comments' receives PR conversation comment", async () => { + const { processor, deliveredChannels } = createMocks([ + { eventTypes: ["review_comments"] }, + ]); + + await processor.onIssueComment(createIssueCommentPayload(true)); + + expect(deliveredChannels).toContain("channel-0"); + }); + + test("channel subscribed to 'review_comments' does NOT receive issue comment", async () => { + const { processor, deliveredChannels } = createMocks([ + { eventTypes: ["review_comments"] }, + ]); + + await processor.onIssueComment(createIssueCommentPayload(false)); + + expect(deliveredChannels).not.toContain("channel-0"); + }); + + test("both subscribers receive PR conversation comment", async () => { + const { processor, deliveredChannels } = createMocks([ + { eventTypes: ["comments"] }, + { eventTypes: ["review_comments"] }, + ]); + + await processor.onIssueComment(createIssueCommentPayload(true)); + + expect(deliveredChannels).toContain("channel-0"); + expect(deliveredChannels).toContain("channel-1"); + expect(deliveredChannels.length).toBe(2); + }); + + test("channel without comment subscriptions does not receive comment", async () => { + const { processor, deliveredChannels } = createMocks([ + { eventTypes: ["pr", "issues"] }, + ]); + + await processor.onIssueComment(createIssueCommentPayload(true)); + + expect(deliveredChannels.length).toBe(0); + }); +});