Skip to content

Commit 1430e51

Browse files
shuhuiluoclaude
andauthored
feat: include PR conversation comments for review_comments subscribers (#96)
* feat: include PR conversation comments for review_comments subscribers 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 * test: add PR comment routing tests and document behavior 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. --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent f2497d2 commit 1430e51

File tree

3 files changed

+138
-8
lines changed

3 files changed

+138
-8
lines changed

README.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ First-time subscriptions open an OAuth window; after authorization the callback
6868

6969
**Event types:** `pr`, `issues`, `commits`, `releases`, `ci`, `comments`, `reviews`, `branches`, `review_comments`, `forks`, `stars`, `all`
7070

71+
> **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).
72+
7173
**Branch filter:** `--branches main,develop` or `--branches release/*` or `--branches all` (default: default branch only)
7274

7375
> 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
109111
- `push` - Commits to branches
110112
- `release` - Published
111113
- `workflow_run` - CI/CD status
112-
- `issue_comment` - New comments (threaded to PR/issue)
114+
- `issue_comment` - Comments on issues and PR conversations (threaded; routes to `comments`, also `review_comments` for PRs)
113115
- `pull_request_review` - PR reviews (threaded to PR)
114-
- `pull_request_review_comment` - Review comments (threaded to PR)
116+
- `pull_request_review_comment` - Inline code review comments (threaded to PR; routes to `review_comments`)
115117
- `create` / `delete` - Branch/tag creation and deletion
116118
- `fork` - Repository forks
117119
- `watch` - Repository stars

src/github-app/event-processor.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,18 @@ export class EventProcessor {
101101
);
102102

103103
// Filter by event preferences
104-
let interestedChannels = channels.filter(ch =>
105-
ch.eventTypes.includes(eventType)
106-
);
104+
let interestedChannels = channels.filter(ch => {
105+
if (ch.eventTypes.includes(eventType)) return true;
106+
// PR conversation comments also notify review_comments subscribers
107+
if (
108+
eventType === "comments" &&
109+
entityContext?.parentType === "pr" &&
110+
ch.eventTypes.includes("review_comments")
111+
) {
112+
return true;
113+
}
114+
return false;
115+
});
107116

108117
// Apply branch filtering for branch-specific events
109118
if (branch) {

tests/unit/github-app/event-processor.test.ts

Lines changed: 122 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
1-
import { describe, expect, test } from "bun:test";
2-
3-
import { matchesBranchFilter } from "../../../src/github-app/event-processor";
1+
import { describe, expect, mock, test } from "bun:test";
2+
3+
import type { EventType } from "../../../src/constants";
4+
import type { GitHubApp } from "../../../src/github-app/app";
5+
import {
6+
EventProcessor,
7+
matchesBranchFilter,
8+
} from "../../../src/github-app/event-processor";
9+
import type { MessageDeliveryService } from "../../../src/services/message-delivery-service";
10+
import type { SubscriptionService } from "../../../src/services/subscription-service";
11+
import type { IssueCommentPayload } from "../../../src/types/webhooks";
412

513
describe("matchesBranchFilter", () => {
614
const defaultBranch = "main";
@@ -133,3 +141,114 @@ describe("matchesBranchFilter", () => {
133141
});
134142
});
135143
});
144+
145+
describe("PR comment routing", () => {
146+
// Helper to create minimal IssueCommentPayload for testing
147+
const createIssueCommentPayload = (
148+
isPrComment: boolean
149+
): IssueCommentPayload =>
150+
({
151+
action: "created",
152+
repository: {
153+
full_name: "owner/repo",
154+
default_branch: "main",
155+
},
156+
issue: {
157+
number: 123,
158+
// GitHub includes pull_request field only for PR comments
159+
...(isPrComment ? { pull_request: { url: "https://..." } } : {}),
160+
},
161+
comment: {
162+
id: 456,
163+
updated_at: new Date().toISOString(),
164+
},
165+
}) as unknown as IssueCommentPayload;
166+
167+
// Helper to create mock services
168+
const createMocks = (channels: Array<{ eventTypes: EventType[] }>) => {
169+
const deliveredChannels: string[] = [];
170+
171+
const mockSubscriptionService = {
172+
getRepoSubscribers: mock(() =>
173+
Promise.resolve(
174+
channels.map((ch, i) => ({
175+
channelId: `channel-${i}`,
176+
spaceId: `space-${i}`,
177+
eventTypes: ch.eventTypes,
178+
branchFilter: "all" as const,
179+
}))
180+
)
181+
),
182+
} as unknown as SubscriptionService;
183+
184+
const mockMessageDeliveryService = {
185+
deliver: mock(({ channelId }: { channelId: string }) => {
186+
deliveredChannels.push(channelId);
187+
return Promise.resolve();
188+
}),
189+
} as unknown as MessageDeliveryService;
190+
191+
const mockGitHubApp = {} as GitHubApp;
192+
193+
const processor = new EventProcessor(
194+
mockGitHubApp,
195+
mockSubscriptionService,
196+
mockMessageDeliveryService
197+
);
198+
199+
return { processor, deliveredChannels };
200+
};
201+
202+
test("channel subscribed to 'comments' receives PR conversation comment", async () => {
203+
const { processor, deliveredChannels } = createMocks([
204+
{ eventTypes: ["comments"] },
205+
]);
206+
207+
await processor.onIssueComment(createIssueCommentPayload(true));
208+
209+
expect(deliveredChannels).toContain("channel-0");
210+
});
211+
212+
test("channel subscribed to 'review_comments' receives PR conversation comment", async () => {
213+
const { processor, deliveredChannels } = createMocks([
214+
{ eventTypes: ["review_comments"] },
215+
]);
216+
217+
await processor.onIssueComment(createIssueCommentPayload(true));
218+
219+
expect(deliveredChannels).toContain("channel-0");
220+
});
221+
222+
test("channel subscribed to 'review_comments' does NOT receive issue comment", async () => {
223+
const { processor, deliveredChannels } = createMocks([
224+
{ eventTypes: ["review_comments"] },
225+
]);
226+
227+
await processor.onIssueComment(createIssueCommentPayload(false));
228+
229+
expect(deliveredChannels).not.toContain("channel-0");
230+
});
231+
232+
test("both subscribers receive PR conversation comment", async () => {
233+
const { processor, deliveredChannels } = createMocks([
234+
{ eventTypes: ["comments"] },
235+
{ eventTypes: ["review_comments"] },
236+
]);
237+
238+
await processor.onIssueComment(createIssueCommentPayload(true));
239+
240+
expect(deliveredChannels).toContain("channel-0");
241+
expect(deliveredChannels).toContain("channel-1");
242+
expect(deliveredChannels.length).toBe(2);
243+
});
244+
245+
test("channel without comment subscriptions does not receive comment", async () => {
246+
const { processor, deliveredChannels } = createMocks([
247+
{ eventTypes: ["pr", "issues"] },
248+
]);
249+
250+
await processor.onIssueComment(createIssueCommentPayload(true));
251+
252+
expect(deliveredChannels.length).toBe(0);
253+
});
254+
});

0 commit comments

Comments
 (0)