feat(github): deliver issue and PR alerts to linked topics (4/5)

openspec/changes/github-alerts/tasks.md

@@ -56,12 +56,12 @@ Chain strategy: stacked-to-main
 
 ## Phase 4: Delivery Wiring (PR4)
 
-- [ ] 4.1 RED: mapper allowlist test — commit email fixture never appears in mapped `GithubEvent`; `merged` derived from `closed`+`pull_request.merged`; `reviewer` from login/team slug.
-- [ ] 4.2 GREEN: `src/adapters/github/event-mapper.ts`.
-- [ ] 4.3 RED: `AlertSender` test — `sendMessage` carries `message_thread_id`; send failure surfaces as `AlertSendFailedError`, not thrown to caller.
-- [ ] 4.4 GREEN: `src/adapters/telegram/alert-sender.ts` (`new Api(BOT_TOKEN)`, no `Bot`/`PII_KEYRING`).
-- [ ] 4.5 GREEN: `composition.ts` `buildGithubRouter(env)` wiring deps end to end.
-- [ ] 4.6 RED: e2e — linked repo alert delivered; unlinked/unclaimed silent; send failure logs reason-only and returns 2xx; log output has no payload fixture strings (spec: Delivery Failure, Allowlisted Fields).
+- [x] 4.1 RED: mapper allowlist test — commit email fixture never appears in mapped `GithubEvent`; `merged` derived from `closed`+`pull_request.merged`; `reviewer` from login/team slug.
+- [x] 4.2 GREEN: `src/adapters/github/event-mapper.ts`.
+- [x] 4.3 RED: `AlertSender` test — `sendMessage` carries `message_thread_id`; send failure surfaces as `AlertSendFailedError`, not thrown to caller.
+- [x] 4.4 GREEN: `src/adapters/telegram/alert-sender.ts` (`new Api(BOT_TOKEN)`, no `Bot`/`PII_KEYRING`).
+- [x] 4.5 GREEN: `composition.ts` `buildGithubRouter(env)` wiring deps end to end.
+- [x] 4.6 RED: e2e — linked repo alert delivered; unlinked/unclaimed silent; send failure logs reason-only and returns 2xx; log output has no payload fixture strings (spec: Delivery Failure, Allowlisted Fields).
 
 ## Phase 5: Link Commands (PR5)
 

src/adapters/github/event-mapper.ts

@@ -0,0 +1,102 @@
+import { parseRepoFullName } from "../../domain/github";
+import type {
+  GithubEvent,
+  GithubEventAction,
+  GithubEventKind,
+} from "../../domain/github";
+
+// Adapter mapper (design.md "Architecture Decisions", Event filtering): the
+// ONLY place that reads raw GitHub webhook JSON. Reads only the allowlisted
+// fields needed for GithubEvent — never a commit author, an email, or any
+// other payload field — and returns null for anything outside the
+// supported event/action set (spec: github-webhook "Unsupported Event or
+// Action Ignored"). The domain never sees this payload shape.
+
+const SUPPORTED_PR_ACTIONS = new Set(["opened", "closed", "review_requested"]);
+const SUPPORTED_ISSUE_ACTIONS = new Set(["opened", "closed"]);
+
+interface RawPayload {
+  action?: unknown;
+  number?: unknown;
+  sender?: { login?: unknown };
+  repository?: { full_name?: unknown };
+  pull_request?: { title?: unknown; html_url?: unknown; merged?: unknown };
+  issue?: { title?: unknown; html_url?: unknown };
+  requested_reviewer?: { login?: unknown };
+  requested_team?: { slug?: unknown };
+}
+
+function asString(value: unknown): string | null {
+  return typeof value === "string" ? value : null;
+}
+
+export function mapGithubEvent(
+  githubEventType: string,
+  payload: unknown,
+): GithubEvent | null {
+  if (githubEventType !== "pull_request" && githubEventType !== "issues") {
+    return null;
+  }
+  if (typeof payload !== "object" || payload === null || Array.isArray(payload)) {
+    return null;
+  }
+  const raw = payload as RawPayload;
+
+  const fullName = asString(raw.repository?.full_name);
+  const repo = fullName !== null ? parseRepoFullName(fullName) : null;
+  if (!repo) return null;
+  // Derived from the already-lowercased, validated repo (design.md/task 4.1:
+  // "org and repo are lowercased consistently with parseRepoFullName and the
+  // claim repo"), never from a separately-cased payload field. `repo`
+  // already matched REPO_FULL_NAME_PATTERN, so the "/" split always yields
+  // a non-empty first segment.
+  const org = repo.slice(0, repo.indexOf("/"));
+
+  const action = asString(raw.action);
+  const number = typeof raw.number === "number" ? raw.number : null;
+  const actor = asString(raw.sender?.login);
+  if (action === null || number === null || actor === null) return null;
+
+  const kind: GithubEventKind = githubEventType;
+  let resolvedAction: GithubEventAction;
+  let title: string | null;
+  let url: string | null;
+  let reviewer: string | undefined;
+
+  if (kind === "pull_request") {
+    if (!SUPPORTED_PR_ACTIONS.has(action)) return null;
+    title = asString(raw.pull_request?.title);
+    url = asString(raw.pull_request?.html_url);
+
+    if (action === "closed") {
+      resolvedAction = raw.pull_request?.merged === true ? "merged" : "closed";
+    } else if (action === "review_requested") {
+      resolvedAction = "review_requested";
+      reviewer =
+        asString(raw.requested_reviewer?.login) ??
+        asString(raw.requested_team?.slug) ??
+        undefined;
+    } else {
+      resolvedAction = "opened";
+    }
+  } else {
+    if (!SUPPORTED_ISSUE_ACTIONS.has(action)) return null;
+    resolvedAction = action as GithubEventAction;
+    title = asString(raw.issue?.title);
+    url = asString(raw.issue?.html_url);
+  }
+
+  if (title === null || url === null) return null;
+
+  return {
+    org,
+    repo,
+    kind,
+    action: resolvedAction,
+    number,
+    title,
+    url,
+    actor,
+    ...(reviewer !== undefined ? { reviewer } : {}),
+  };
+}

src/adapters/telegram/alert-sender.ts

@@ -0,0 +1,53 @@
+import { GrammyError, HttpError } from "grammy";
+import type { Api } from "grammy";
+import { AlertSendFailedError } from "../../domain/errors";
+import type { AlertSendFailureClass } from "../../domain/errors";
+import type { AlertSender } from "../../domain/ports";
+
+// design.md "Sender": `new Api(BOT_TOKEN)` in composition, no `Bot` and no
+// `PII_KEYRING` on this route. Plain text (no `parse_mode`) so a title with
+// special characters cannot break sending via a MarkdownV2/HTML escaping
+// bug (design.md "Message"). `link_preview_options.is_disabled` keeps the
+// alert compact — a PR/issue URL preview adds nothing here.
+
+// PR4 correction (RES-001): classifies the underlying failure into a
+// fixed, non-sensitive bucket the caller can safely log as `reason`. This
+// never changes the spec'd 2xx/no-retry behavior — only what a caller may
+// log. `GrammyError` is Telegram's own JSON-encoded `{ok:false,
+// error_code, description}` response (a genuine API-level rejection);
+// anything else (a non-JSON/5xx response, or the fetch call itself
+// failing) surfaces as grammY's `HttpError` and is treated as
+// "telegram-unavailable" — the same bucket used for a Telegram-side 5xx.
+function classifyFailure(err: unknown): AlertSendFailureClass {
+  if (err instanceof GrammyError) {
+    if (err.error_code === 429) return "rate-limited";
+    if (err.error_code >= 500) return "telegram-unavailable";
+    return "rejected";
+  }
+  if (err instanceof HttpError) return "telegram-unavailable";
+  return "telegram-unavailable";
+}
+
+// A failed send (e.g. the linked topic was deleted, a rate limit, or
+// Telegram being unavailable) is a permanent-for-this-request delivery
+// failure GitHub does not need to redeliver, so it is converted to
+// AlertSendFailedError rather than propagated as-is — the port contract
+// (ports.ts) requires this so routeGithubEvent can report "send-failed"
+// instead of a 500 (design.md "GitHub route status policy"). The thrown
+// error's message is always a fixed, non-sensitive string — Telegram's
+// `description` (which can echo operator-specific detail) is never
+// carried over, and neither is the chat id or the token.
+export function createTelegramAlertSender(api: Api): AlertSender {
+  return {
+    async send(chatId: number, threadId: number, text: string): Promise<void> {
+      try {
+        await api.sendMessage(chatId, text, {
+          message_thread_id: threadId,
+          link_preview_options: { is_disabled: true },
+        });
+      } catch (err) {
+        throw new AlertSendFailedError("sendMessage failed", classifyFailure(err));
+      }
+    },
+  };
+}

src/composition.ts

@@ -1,16 +1,21 @@
+import { Api } from "grammy";
 import { createAesGcmCipher } from "./adapters/crypto/aes-gcm-cipher";
 import { parseKeyRing } from "./adapters/crypto/key-ring";
 import { createD1DmSelectionRepo } from "./adapters/d1/dm-selection-repo";
+import { createD1GithubOrgClaimRepo } from "./adapters/d1/github-org-claim-repo";
 import { createD1MemberRepo } from "./adapters/d1/member-repo";
 import { createD1MembershipRepo } from "./adapters/d1/membership-repo";
 import { createD1ProfileRepo } from "./adapters/d1/profile-repo";
+import { createD1RepoTopicLinkRepo } from "./adapters/d1/repo-topic-link-repo";
 import { createD1TeamRepo } from "./adapters/d1/team-repo";
 import { createSafeLogger } from "./adapters/log/safe-logger";
+import { createTelegramAlertSender } from "./adapters/telegram/alert-sender";
 import { createBot } from "./adapters/telegram/bot";
 import { createChatAdminChecker } from "./adapters/telegram/chat-admin-checker";
 import { registerCommands } from "./adapters/telegram/commands";
 import { ConfigError } from "./config-error";
 import type { FieldCipher } from "./domain/ports";
+import type { RouteGithubEventDeps } from "./domain/usecases/route-github-event";
 import type { UserFromGetMe } from "grammy/types";
 import type { Env } from "./env";
 
@@ -93,3 +98,17 @@ export function buildBot(env: Env) {
 
   return bot;
 }
+
+// design.md "Sender": `new Api(BOT_TOKEN)` here, no `Bot` and no
+// `PII_KEYRING` on this route — a broken keyring would otherwise break
+// GitHub alerts too, and this composition path never touches PII fields.
+export function buildGithubRouter(env: Env): RouteGithubEventDeps {
+  const api = new Api(env.BOT_TOKEN);
+
+  return {
+    githubOrgClaimRepo: createD1GithubOrgClaimRepo(env.DB),
+    repoTopicLinkRepo: createD1RepoTopicLinkRepo(env.DB),
+    teamRepo: createD1TeamRepo(env.DB, idGen, clock),
+    alertSender: createTelegramAlertSender(api),
+  };
+}

src/domain/errors.ts

@@ -14,12 +14,32 @@ export class DmSelectionRequiredError extends DomainError {}
 export class FieldUnreadableError extends DomainError {}
 export class InvalidRepoError extends DomainError {}
 export class OrgNotClaimedError extends DomainError {}
+// A fixed, non-sensitive classification of why a send failed — never
+// Telegram's error description, the chat id, or the token (design.md
+// "Logging"; PR4 correction RES-001). "rate-limited" is a 429, "rejected"
+// is any other 4xx (e.g. the topic was deleted), "telegram-unavailable" is
+// a 5xx or a network/transport failure (grammY's HttpError). All three are
+// still a permanent-for-this-request delivery failure per the spec ("no
+// retry within the same request") — the classification only changes what
+// is logged, never the 2xx/no-retry behavior.
+export type AlertSendFailureClass =
+  | "rate-limited"
+  | "rejected"
+  | "telegram-unavailable";
+
 // Thrown by an AlertSender implementation when the underlying send fails
 // (e.g. Telegram rejects the request because the topic was deleted).
 // route-github-event.ts catches this specific error and returns a
 // "send-failed" outcome instead of letting it propagate (design.md
 // "GitHub route status policy").
-export class AlertSendFailedError extends DomainError {}
+export class AlertSendFailedError extends DomainError {
+  readonly failureClass: AlertSendFailureClass;
+
+  constructor(message: string, failureClass: AlertSendFailureClass) {
+    super(message);
+    this.failureClass = failureClass;
+  }
+}
 // Thrown when a tenant-scoped write's explicit `teamId` argument disagrees
 // with the `teamId` embedded in the entity being written (e.g.
 // RepoTopicLinkRepo.upsert). The explicit argument is always authoritative

src/domain/usecases/route-github-event.ts

@@ -1,4 +1,5 @@
 import { AlertSendFailedError, NotFoundError } from "../errors";
+import type { AlertSendFailureClass } from "../errors";
 import { formatGithubAlert } from "../github";
 import type { GithubEvent } from "../github";
 import type { TeamId } from "../ids";
@@ -18,7 +19,10 @@ export interface RouteGithubEventDeps {
 
 export type RouteGithubEventResult =
   | { kind: "delivered"; teamId: TeamId }
-  | { kind: "send-failed"; teamId: TeamId }
+  // PR4 correction (RES-001): failureClass is AlertSendFailedError's fixed,
+  // non-sensitive classification — carried through so the HTTP adapter can
+  // log a distinguishable reason without ever seeing Telegram's raw error.
+  | { kind: "send-failed"; teamId: TeamId; failureClass: AlertSendFailureClass }
   | { kind: "ignored"; reason: "unclaimed-org" | "unlinked-repo" };
 
 // design.md "GitHub route status policy": unclaimed org or unlinked repo
@@ -54,7 +58,7 @@ export async function routeGithubEvent(
     await deps.alertSender.send(team.chatId, link.threadId, text);
   } catch (err) {
     if (err instanceof AlertSendFailedError) {
-      return { kind: "send-failed", teamId };
+      return { kind: "send-failed", teamId, failureClass: err.failureClass };
     }
     throw err;
   }

src/index.ts

@@ -1,10 +1,12 @@
 import { Hono } from "hono";
 import type { Update } from "grammy/types";
 import { createSafeLogger } from "./adapters/log/safe-logger";
+import { mapGithubEvent } from "./adapters/github/event-mapper";
 import { verifyGithubSignature } from "./adapters/github/signature";
 import { timingSafeCompare } from "./adapters/crypto/timing-safe-compare";
-import { buildBot } from "./composition";
+import { buildBot, buildGithubRouter } from "./composition";
 import { ConfigError } from "./config-error";
+import { routeGithubEvent } from "./domain/usecases/route-github-event";
 import type { Env } from "./env";
 
 export type { Env } from "./env";
@@ -86,11 +88,10 @@ app.post("/telegram/webhook", async (c) => {
   return c.text("ok");
 });
 
-// design.md "GitHub route status policy" — route skeleton only (PR3). The
-// mapper, org/repo routing and Telegram delivery land in Phase 4; until
-// then every signature-verified, well-formed, non-ping event is
-// acknowledged and dropped (the same 2xx the design table gives an
-// unsupported event/action, since nothing is wired to support one yet).
+// design.md "GitHub route status policy": HMAC-verify the raw body, map
+// the payload to an allowlisted domain event (or drop it as unsupported),
+// then route it to the linked team's forum topic through the composition
+// root (buildGithubRouter). No payload field is ever logged.
 app.post("/github/webhook", async (c) => {
   // RES-001, corrected: an unreadable raw body is a transient, transport-
   // level failure (e.g. a broken/aborted request stream) on a request that
@@ -146,15 +147,59 @@ app.post("/github/webhook", async (c) => {
     return c.text("ok", 200);
   }
 
-  if (c.req.header("X-GitHub-Event") === "ping") {
+  const githubEventType = c.req.header("X-GitHub-Event") ?? "";
+  if (githubEventType === "ping") {
+    return c.text("ok", 200);
+  }
+
+  // The mapper (adapters/github/event-mapper.ts) is the only place that
+  // reads the raw payload shape (design.md "Event filtering"). `null`
+  // covers every unsupported event type or action (spec: "Unsupported
+  // Event or Action Ignored") — 200, logged, no payload field in `reason`.
+  const event = mapGithubEvent(githubEventType, payload);
+  if (!event) {
+    logger.log({ event: "github-webhook", outcome: "ok", reason: "ignored:unsupported-event" });
+    return c.text("ok", 200);
+  }
+
+  let result: Awaited<ReturnType<typeof routeGithubEvent>>;
+  try {
+    result = await routeGithubEvent(event, buildGithubRouter(c.env));
+  } catch (err) {
+    // Unexpected failure (e.g. D1) — design.md "GitHub route status
+    // policy": 500, logged by error name only, so the delivery stays
+    // visible in GitHub for a manual redeliver.
+    logger.log({
+      event: "github-webhook",
+      outcome: "error",
+      errorCode: err instanceof Error ? err.name : "UnknownError",
+    });
+    return c.text("Internal Server Error", 500);
+  }
+
+  if (result.kind === "ignored") {
+    // `reason` is one of a fixed, non-sensitive set ("unclaimed-org",
+    // "unlinked-repo") — never a repo name or any payload field.
+    logger.log({ event: "github-webhook", outcome: "ok", reason: `ignored:${result.reason}` });
+    return c.text("ok", 200);
+  }
+  if (result.kind === "send-failed") {
+    // Telegram delivery failure (e.g. the topic was deleted, a rate limit,
+    // or Telegram being unavailable) is a permanent-for-this-request
+    // failure, not an infrastructure one — 2xx, no retry (design.md
+    // "GitHub route status policy", spec: "Delivery Failure Is Logged and
+    // Acknowledged"). `reason` is AlertSendFailedError's fixed,
+    // non-sensitive classification (PR4 correction RES-001) — never
+    // Telegram's error description, the chat id, or the token.
+    logger.log({
+      event: "github-webhook",
+      outcome: "error",
+      errorCode: "AlertSendFailed",
+      reason: result.failureClass,
+    });
     return c.text("ok", 200);
   }
 
-  // Placeholder until Phase 4 wires the mapper/router: acknowledge, drop,
-  // and log (RES-002 / design.md:27 "unsupported event or action: 200,
-  // logged"). `reason` is a fixed, non-sensitive string per the logging
-  // allowlist — never the event type, action, or any payload field.
-  logger.log({ event: "github-webhook", outcome: "ok", reason: "ignored:not-yet-routed" });
   return c.text("ok", 200);
 });