refactor: reduce boilerplate in event-processor with generic helper

[shuhuiluo]

• Create formatCreate and formatDelete formatters in webhook-events.ts
• Add processEvent generic helper method to encapsulate common pattern
• Refactor all 10 event handlers to use processEvent helper
• Remove redundant processing logs (centralized in processEvent)
• Export EventType union from constants for type safety
• Simplify Promise.allSettled pattern in polling-service.ts

Result: Reduced event-processor.ts from ~570 to 236 lines (-390 lines) All handlers now 5-8 lines instead of 45-55 lines each Eliminates ~450 lines of duplicated boilerplate code

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• New Features

  • Added support for Create and Delete webhook events, now formatted and delivered through configured channels.

• Improvements

  • OAuth authentication prompts now persist as regular messages instead of auto-disappearing, improving visibility during authentication workflows.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

This PR introduces a cohesive set of type system improvements and event handling refactoring across multiple modules. It adds a derived EventType type alias in constants, extends webhook event formatting with formatCreate and formatDelete handlers, centralizes event processing logic into a generic processEvent helper in the EventProcessor class that standardizes handling across all webhook handlers, strengthens event type typing in the polling service with a new isEventTypeMatch helper, and removes the ephemeral flag from OAuth prompt messages.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Event processor refactoring: The new generic processEvent helper applies a consistent pattern across 10+ handlers (onPullRequest, onPush, onIssues, onRelease, onWorkflowRun, onIssueComment, onPullRequestReview, onBranchEvent, onFork, onWatch), centralizing promise settlement and error logging—requires verification that the pattern is correctly applied and maintains handler semantics
• Polling service type and logic changes: The EVENT_TYPE_MAP type change from Record<string, string> to Record<EventType, string> with new event mappings, plus the new isEventTypeMatch helper and Promise.allSettled refactoring, involves both type system and runtime behavior changes that need careful review
• New formatters: The addition of formatCreate and formatDelete functions follows an existing pattern but should be validated for consistency with other formatters
• OAuth behavior change: The removal of the ephemeral flag alters message visibility behavior and should be confirmed as intentional

Possibly related PRs

• PR #21: Directly relates to the EventType type definition and polling-service event filtering logic that was originally introduced there
• PR #41: Modifies the same webhook event formatting module (src/formatters/webhook-events.ts) with formatter function additions
• PR #32: Overlaps significantly on EventProcessor and PollingService refactoring, introducing SubscriptionService integration alongside this PR's processEvent helper and event-type typing improvements

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: refactoring event-processor to reduce boilerplate using a generic helper method, which aligns with the core objective.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor/event

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/github-app/event-processor.ts (1)

49-101: Fix eventTypes filtering: substring and “all” semantics are incorrect

interestedChannels currently uses:

const interestedChannels = channels.filter(ch =>
  ch.eventTypes.includes(eventType)
);

Given eventTypes is stored as a comma‑separated string (e.g. "pr,issues,commits,releases" or "all"), this has two problems:

• "all" will not match any specific eventType, so webhook subscribers who chose “all events” won’t receive anything here.
• Substring matches can be wrong, e.g. a subscription of "review_comments" will also match the "comments" event type because "review_comments".includes("comments") is true.

You should align this logic with the polling service’s semantics by tokenizing and handling "all" explicitly. For example:

-    // Filter by event preferences
-    const interestedChannels = channels.filter(ch =>
-      ch.eventTypes.includes(eventType)
-    );
+    // Filter by event preferences (supports comma-separated lists and "all")
+    const interestedChannels = channels.filter(ch => {
+      const types = ch.eventTypes;
+      if (!types || types === "all") return true;
+      return types
+        .split(",")
+        .map(t => t.trim())
+        .includes(eventType);
+    });

This keeps webhook behavior consistent with isEventTypeMatch in the polling service and avoids subtle misrouting.

🧹 Nitpick comments (3)

src/utils/oauth-helpers.ts (1)

21-27: Confirm non‑ephemeral OAuth prompt behavior

With the ephemeral option removed, the OAuth prompt (including a user-specific auth URL) will now be posted to the whole channel rather than just the requesting user. If the intent was to keep these prompts private and reduce channel noise, consider keeping them ephemeral or switching to a DM-style flow instead.

src/formatters/webhook-events.ts (1)

208-230: Create/Delete formatter behavior and consistency

The new formatCreate/formatDelete helpers look correct and match the existing formatting style (short, Markdown-friendly, using repository.full_name and backticked ref). Two optional polish ideas:

• Consider adding a 🔗 prefix before ${repository.html_url} to match other formatters like formatWatch/formatWorkflowRun.
• If you ever want to special-case repository-level create events, you could use repository.full_name instead of "unknown" when ref is null and ref_type === "repository".

src/services/polling-service.ts (1)

329-354: isEventTypeMatch semantics look sound (tiny edge‑case nit)

The helper correctly treats null/undefined/"all" subscription types as “match everything” and maps short names via EVENT_TYPE_MAP before comparing against the GitHub event type, which fixes the previous ad‑hoc filtering. One minor edge case: an empty string "" for subscriptionTypes is currently treated like “all”; if that can occur due to DB defaults or bad input, you may want to special‑case it instead of folding it into “match all”.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9bfbe49 and d1b7146.

📒 Files selected for processing (5)

• src/constants.ts (1 hunks)
• src/formatters/webhook-events.ts (2 hunks)
• src/github-app/event-processor.ts (9 hunks)
• src/services/polling-service.ts (5 hunks)
• src/utils/oauth-helpers.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (3)

src/formatters/webhook-events.ts (1)

src/types/webhooks.ts (2)

• CreatePayload (24-24)
• DeletePayload (25-25)

src/services/polling-service.ts (1)

src/constants.ts (1)

• EventType (27-27)

src/github-app/event-processor.ts (3)

src/constants.ts (1)

• EventType (27-27)

src/types/webhooks.ts (6)

• PullRequestPayload (14-14)
• PushPayload (16-16)
• CreatePayload (24-24)
• DeletePayload (25-25)
• ForkPayload (26-26)
• WatchPayload (27-27)

src/formatters/webhook-events.ts (11)

• formatPullRequest (21-44)
• formatPush (73-103)
• formatIssue (46-71)
• formatRelease (105-121)
• formatWorkflowRun (123-140)
• formatIssueComment (142-158)
• formatPullRequestReview (160-180)
• formatCreate (208-218)
• formatDelete (220-230)
• formatFork (182-191)
• formatWatch (193-206)

🔇 Additional comments (6)

src/formatters/webhook-events.ts (1)

6-18: Typed Create/Delete payload imports look good

Wiring in CreatePayload and DeletePayload from ../types/webhooks keeps formatter signatures aligned with the webhook type layer and avoids stringly-typed payloads. No issues here.

src/constants.ts (1)

24-28: EventType alias is clean and future‑proof

Deriving EventType from ALLOWED_EVENT_TYPES is the right approach to keep the union synced with the source list and avoid drift. Looks good.

src/services/polling-service.ts (3)

8-32: Typed EVENT_TYPE_MAP strengthens event handling

Importing EventType and constraining EVENT_TYPE_MAP to Record<EventType, string> nicely ensures every allowed short name is mapped and prevents accidental extra keys. This makes the event filtering logic more robust and self-documenting.

---

241-250: Safer PR number extraction from repo events

The additional guards on event.payload ("number" in event.payload and typeof event.payload.number === "number") make the PR prefetch step more resilient to unexpected payload shapes. This avoids adding invalid IDs to prNumbers and is a good hardening change.

---

295-307: Consolidated channel sends with Promise.allSettled

Switching to a single Promise.allSettled over channelsForEvent keeps all sends in flight while still surfacing per-channel failures via the index back into channelsForEvent. This is a solid pattern here, and the logging looks correct.

src/github-app/event-processor.ts (1)

103-236: Generic processEvent helper and handler refactor look solid

The new processEvent helper cleanly centralizes logging, subscriber lookup, formatting, and parallel sending with Promise.allSettled, and the individual on* handlers now read as small, declarative mappings from payload → (EventType, formatter, log context). This significantly reduces duplication and should make future event additions straightforward once the filtering semantics above are corrected.