refactor: use EventType[] in code layer, string only at DB boundary

src/constants.ts

@@ -1,8 +1,15 @@
+/** Default event types as array for code layer */
+export const DEFAULT_EVENT_TYPES_ARRAY: readonly EventType[] = [
+  "pr",
+  "issues",
+  "commits",
+  "releases",
+];
+
 /**
- * Default event types for GitHub subscriptions
- * This constant is imported and used across the codebase to ensure consistency
+ * Default event types for GitHub subscriptions (string for DB storage)
  */
-export const DEFAULT_EVENT_TYPES = "pr,issues,commits,releases";
+export const DEFAULT_EVENT_TYPES = DEFAULT_EVENT_TYPES_ARRAY.join(",");
 
 /**
  * Allowed event types that users can subscribe to
@@ -26,6 +33,11 @@ export const ALLOWED_EVENT_TYPES = [
  */
 export type EventType = (typeof ALLOWED_EVENT_TYPES)[number];
 
+/** Pre-allocated Set for O(1) event type validation */
+export const ALLOWED_EVENT_TYPES_SET: ReadonlySet<EventType> = new Set(
+  ALLOWED_EVENT_TYPES
+);
+
 /**
  * Pending message cleanup interval (30 seconds)
  * How often to check for and remove stale pending messages

src/handlers/github-subscription-handler.ts

@@ -1,6 +1,12 @@
 import type { BotHandler } from "@towns-protocol/bot";
 
-import { ALLOWED_EVENT_TYPES, DEFAULT_EVENT_TYPES } from "../constants";
+import {
+  ALLOWED_EVENT_TYPES,
+  ALLOWED_EVENT_TYPES_SET,
+  DEFAULT_EVENT_TYPES,
+  DEFAULT_EVENT_TYPES_ARRAY,
+  type EventType,
+} from "../constants";
 import {
   TokenStatus,
   type GitHubOAuthService,
@@ -91,7 +97,7 @@ async function handleSubscribe(
   }
 
   // Parse and validate event types from args
-  let eventTypes: string;
+  let eventTypes: EventType[];
   try {
     eventTypes = parseEventTypes(args);
   } catch (error) {
@@ -318,12 +324,12 @@ async function handleStatus(
 
 /**
  * Parse and validate event types from --events flag
- * Returns default types if no flag, or comma-separated validated event types
+ * Returns default types if no flag, or validated event types array
  * @throws Error if any event type is invalid
  */
-function parseEventTypes(args: string[]): string {
+function parseEventTypes(args: string[]): EventType[] {
   const eventsIndex = args.findIndex(arg => arg.startsWith("--events"));
-  if (eventsIndex === -1) return DEFAULT_EVENT_TYPES;
+  if (eventsIndex === -1) return [...DEFAULT_EVENT_TYPES_ARRAY];
 
   let rawEventTypes: string;
 
@@ -334,7 +340,7 @@ function parseEventTypes(args: string[]): string {
     // Check for --events pr,issues format (next arg)
     rawEventTypes = args[eventsIndex + 1];
   } else {
-    return DEFAULT_EVENT_TYPES;
+    return [...DEFAULT_EVENT_TYPES_ARRAY];
   }
 
   // Parse and validate event types
@@ -345,14 +351,13 @@ function parseEventTypes(args: string[]): string {
 
   // Handle "all" as special case
   if (tokens.includes("all")) {
-    return ALLOWED_EVENT_TYPES.join(",");
+    return [...ALLOWED_EVENT_TYPES];
   }
 
   // Validate each token
   const invalidTokens: string[] = [];
-  const allowedSet = new Set(ALLOWED_EVENT_TYPES);
   for (const token of tokens) {
-    if (!allowedSet.has(token as (typeof ALLOWED_EVENT_TYPES)[number])) {
+    if (!ALLOWED_EVENT_TYPES_SET.has(token as EventType)) {
       invalidTokens.push(token);
     }
   }
@@ -366,17 +371,13 @@ function parseEventTypes(args: string[]): string {
     );
   }
 
-  // Remove duplicates using Set and return
-  const uniqueTokens = Array.from(new Set(tokens));
-  return uniqueTokens.join(",");
+  // Remove duplicates and return
+  return [...new Set(tokens)] as EventType[];
 }
 
 /**
  * Format event types for display
  */
-function formatEventTypes(eventTypes: string): string {
-  return eventTypes
-    .split(",")
-    .map(t => t.trim())
-    .join(", ");
+function formatEventTypes(eventTypes: EventType[]): string {
+  return eventTypes.join(", ");
 }

src/routes/oauth-callback.ts

@@ -1,7 +1,7 @@
 import type { Context } from "hono";
 
 import { getOwnerIdFromUsername, parseRepo } from "../api/github-client";
-import { DEFAULT_EVENT_TYPES } from "../constants";
+import { DEFAULT_EVENT_TYPES_ARRAY } from "../constants";
 import {
   generateInstallUrl,
   type InstallationService,
@@ -70,7 +70,9 @@ export async function handleOAuthCallback(
           spaceId: result.spaceId,
           channelId: result.channelId,
           repoIdentifier: result.redirectData.repo,
-          eventTypes: result.redirectData.eventTypes || DEFAULT_EVENT_TYPES,
+          eventTypes: result.redirectData.eventTypes ?? [
+            ...DEFAULT_EVENT_TYPES_ARRAY,
+          ],
         });
 
         if (subResult.success) {

src/services/polling-service.ts

@@ -329,22 +329,18 @@ export class PollingService {
 /**
  * Check if an event matches the subscription's event type filter
  * @param eventType - GitHub event type (e.g., "PullRequestEvent") or null
- * @param subscriptionTypes - Comma-separated event types (e.g., "pr,issues") or "all" or null
+ * @param subscriptionTypes - Array of event types (e.g., ["pr", "issues"])
  */
 function isEventTypeMatch(
   eventType: string | null,
-  subscriptionTypes: string | null | undefined
+  subscriptionTypes: EventType[]
 ): boolean {
-  // Treat null event type or null/undefined/"all" subscription as match
-  if (!eventType || !subscriptionTypes || subscriptionTypes === "all")
-    return true;
-
-  const subscribedTypes = subscriptionTypes.split(",").map(t => t.trim());
+  // Treat null event type or empty subscription as match-all
+  if (!eventType || subscriptionTypes.length === 0) return true;
 
   // Check if the event type matches any of the subscribed short names
-  for (const shortName of subscribedTypes) {
-    const mappedTypes =
-      EVENT_TYPE_MAP[shortName as EventType]?.split(",") ?? [];
+  for (const shortName of subscriptionTypes) {
+    const mappedTypes = EVENT_TYPE_MAP[shortName]?.split(",") ?? [];
     if (mappedTypes.includes(eventType)) {
       return true;
     }

src/services/subscription-service.ts

@@ -13,6 +13,7 @@ import {
   PENDING_MESSAGE_MAX_AGE_MS,
   PENDING_SUBSCRIPTION_CLEANUP_INTERVAL_MS,
   PENDING_SUBSCRIPTION_EXPIRATION_MS,
+  type EventType,
 } from "../constants";
 import { db } from "../db";
 import { githubSubscriptions, pendingSubscriptions } from "../db/schema";
@@ -31,7 +32,7 @@ export interface SubscribeParams {
   spaceId: string;
   channelId: string;
   repoIdentifier: string; // Format: "owner/repo"
-  eventTypes?: string;
+  eventTypes: EventType[];
 }
 
 /**
@@ -44,13 +45,13 @@ type SubscribeSuccess =
       success: true;
       deliveryMode: "webhook";
       repoFullName: string;
-      eventTypes: string;
+      eventTypes: EventType[];
     }
   | {
       success: true;
       deliveryMode: "polling";
       repoFullName: string;
-      eventTypes: string;
+      eventTypes: EventType[];
       installUrl: string;
     };
 
@@ -61,7 +62,7 @@ type SubscribeFailure =
       requiresInstallation: true;
       installUrl: string;
       repoFullName: string;
-      eventTypes: string;
+      eventTypes: EventType[];
       error: string;
     };
 
@@ -106,9 +107,6 @@ export class SubscriptionService {
     const { townsUserId, spaceId, channelId, repoIdentifier, eventTypes } =
       params;
 
-    // Normalize event types once to avoid duplication
-    const requestedEventTypes = eventTypes || DEFAULT_EVENT_TYPES;
-
     // Parse owner/repo
     const [owner, repo] = repoIdentifier.split("/");
     if (!owner || !repo) {
@@ -160,23 +158,14 @@ export class SubscriptionService {
         // Get owner ID from public profile for installation URL
         const ownerId = await getOwnerIdFromUsername(owner);
 
-        // Store pending subscription for completion after installation
-        await this.storePendingSubscription({
+        return this.requiresInstallationFailure({
           townsUserId,
           spaceId,
           channelId,
           repoFullName: repoIdentifier,
-          eventTypes: requestedEventTypes,
+          eventTypes,
+          ownerId,
         });
-
-        return {
-          success: false,
-          requiresInstallation: true,
-          installUrl: generateInstallUrl(ownerId),
-          repoFullName: repoIdentifier,
-          eventTypes: requestedEventTypes,
-          error: `Repository not found or you don't have access.`,
-        };
       }
 
       // Check if this might be an org approval issue
@@ -207,23 +196,14 @@ export class SubscriptionService {
     if (repoInfo.isPrivate) {
       // Private repos MUST have GitHub App installed
       if (!installationId) {
-        // Store pending subscription for completion after installation
-        await this.storePendingSubscription({
+        return this.requiresInstallationFailure({
           townsUserId,
           spaceId,
           channelId,
           repoFullName: repoInfo.fullName,
-          eventTypes: requestedEventTypes,
+          eventTypes,
+          ownerId: repoInfo.owner.id,
         });
-
-        return {
-          success: false,
-          requiresInstallation: true,
-          installUrl: generateInstallUrl(repoInfo.owner.id),
-          repoFullName: repoInfo.fullName,
-          eventTypes: requestedEventTypes,
-          error: `Private repository requires GitHub App installation`,
-        };
       }
 
       deliveryMode = "webhook";
@@ -265,7 +245,7 @@ export class SubscriptionService {
       createdByGithubLogin: githubUser.login,
       installationId,
       enabled: true,
-      eventTypes: requestedEventTypes,
+      eventTypes: eventTypes.join(","),
       createdAt: now,
       updatedAt: now,
     });
@@ -275,19 +255,48 @@ export class SubscriptionService {
         success: true,
         deliveryMode: "polling",
         repoFullName: repoInfo.fullName,
-        eventTypes: requestedEventTypes,
+        eventTypes,
         installUrl: generateInstallUrl(repoInfo.owner.id),
       };
     } else {
       return {
         success: true,
         deliveryMode: "webhook",
         repoFullName: repoInfo.fullName,
-        eventTypes: requestedEventTypes,
+        eventTypes,
       };
     }
   }
 
+  /**
+   * Store pending subscription and return failure requiring installation
+   */
+  private async requiresInstallationFailure(params: {
+    townsUserId: string;
+    spaceId: string;
+    channelId: string;
+    repoFullName: string;
+    eventTypes: EventType[];
+    ownerId?: number;
+  }): Promise<SubscribeFailure> {
+    await this.storePendingSubscription({
+      townsUserId: params.townsUserId,
+      spaceId: params.spaceId,
+      channelId: params.channelId,
+      repoFullName: params.repoFullName,
+      eventTypes: params.eventTypes,
+    });
+
+    return {
+      success: false,
+      requiresInstallation: true,
+      installUrl: generateInstallUrl(params.ownerId),
+      repoFullName: params.repoFullName,
+      eventTypes: params.eventTypes,
+      error: "Private repository requires GitHub App installation",
+    };
+  }
+
   /**
    * Store a pending subscription for completion after GitHub App installation
    * Used when user tries to subscribe to private repo before installing GitHub App
@@ -297,7 +306,7 @@ export class SubscriptionService {
     spaceId: string;
     channelId: string;
     repoFullName: string;
-    eventTypes: string;
+    eventTypes: EventType[];
   }): Promise<void> {
     const now = new Date();
     const expiresAt = new Date(
@@ -311,7 +320,7 @@ export class SubscriptionService {
         spaceId: params.spaceId,
         channelId: params.channelId,
         repoFullName: params.repoFullName,
-        eventTypes: params.eventTypes,
+        eventTypes: params.eventTypes.join(","),
         createdAt: now,
         expiresAt,
       })
@@ -360,7 +369,7 @@ export class SubscriptionService {
           spaceId: sub.spaceId,
           channelId: sub.channelId,
           repoIdentifier: repoFullName,
-          eventTypes: sub.eventTypes,
+          eventTypes: sub.eventTypes.split(",") as EventType[],
         });
 
         if (result.success && this.bot) {
@@ -419,7 +428,7 @@ export class SubscriptionService {
     channelId: string,
     spaceId: string
   ): Promise<
-    Array<{ repo: string; eventTypes: string; deliveryMode: string }>
+    Array<{ repo: string; eventTypes: EventType[]; deliveryMode: string }>
   > {
     const results = await db
       .select({
@@ -437,7 +446,9 @@ export class SubscriptionService {
 
     return results.map(r => ({
       repo: r.repo,
-      eventTypes: r.eventTypes || DEFAULT_EVENT_TYPES,
+      eventTypes: (r.eventTypes || DEFAULT_EVENT_TYPES).split(
+        ","
+      ) as EventType[],
       deliveryMode: r.deliveryMode,
     }));
   }
@@ -453,7 +464,7 @@ export class SubscriptionService {
   async getRepoSubscribers(
     repoFullName: string,
     deliveryMode?: "webhook" | "polling"
-  ): Promise<Array<{ channelId: string; eventTypes: string }>> {
+  ): Promise<Array<{ channelId: string; eventTypes: EventType[] }>> {
     const conditions = [eq(githubSubscriptions.repoFullName, repoFullName)];
 
     if (deliveryMode) {
@@ -470,7 +481,9 @@ export class SubscriptionService {
 
     return results.map(r => ({
       channelId: r.channelId,
-      eventTypes: r.eventTypes || DEFAULT_EVENT_TYPES,
+      eventTypes: (r.eventTypes || DEFAULT_EVENT_TYPES).split(
+        ","
+      ) as EventType[],
     }));
   }