fix(github): keep a webhook delivery alive when link metadata will not parse

apps/api/src/plugins/github/utils/parse-link-metadata.ts

@@ -0,0 +1,25 @@
+// An external link's metadata is a JSON string in the database, so a row
+// written by an older version, or truncated, is not the caller's fault and
+// should not take the webhook delivery down with it. The Gitea handlers already
+// warn and carry on with an empty object; this is the same behaviour in one
+// place, since the GitHub side needs it in four.
+export function parseLinkMetadata(
+  raw: string | null | undefined,
+  context: { externalLinkId: string; source: string },
+): Record<string, unknown> {
+  if (!raw) {
+    return {};
+  }
+
+  try {
+    return JSON.parse(raw) as Record<string, unknown>;
+  } catch (error) {
+    console.warn("Failed to parse GitHub external link metadata", {
+      ...context,
+      metadata: raw,
+      error,
+    });
+
+    return {};
+  }
+}

apps/api/src/plugins/github/webhooks/issue-closed.ts

@@ -7,6 +7,7 @@ import {
   findAllIntegrationsByRepo,
   updateTaskStatus,
 } from "../services/task-service";
+import { parseLinkMetadata } from "../utils/parse-link-metadata";
 import { resolveTargetStatus } from "../utils/resolve-column";
 
 type IssueClosedPayload = {
@@ -53,9 +54,10 @@ export async function handleIssueClosed(payload: IssueClosedPayload) {
       continue;
     }
 
-    const existingMetadata = externalLink.metadata
-      ? JSON.parse(externalLink.metadata)
-      : {};
+    const existingMetadata = parseLinkMetadata(externalLink.metadata, {
+      externalLinkId: externalLink.id,
+      source: "issue_closed",
+    });
 
     if (existingMetadata.createdFrom === "kaneo") {
       continue;
@@ -90,7 +92,5 @@ export async function handleIssueClosed(payload: IssueClosedPayload) {
         state: "closed",
       },
     });
-
-    return;
   }
 }

apps/api/src/plugins/github/webhooks/issue-edited.ts

@@ -4,6 +4,7 @@ import { taskTable } from "../../../database/schema";
 import { findExternalLink, updateExternalLink } from "../services/link-manager";
 import { findAllIntegrationsByRepo } from "../services/task-service";
 import { formatTaskDescriptionFromIssue } from "../utils/format";
+import { parseLinkMetadata } from "../utils/parse-link-metadata";
 
 type IssueEditedPayload = {
   action: string;
@@ -63,9 +64,10 @@ export async function handleIssueEdited(payload: IssueEditedPayload) {
       continue;
     }
 
-    const metadata = externalLink.metadata
-      ? JSON.parse(externalLink.metadata)
-      : {};
+    const metadata = parseLinkMetadata(externalLink.metadata, {
+      externalLinkId: externalLink.id,
+      source: "issue_edited",
+    });
 
     const updateData: Record<string, unknown> = {};
     const updatedMetadata = { ...metadata };

apps/api/src/plugins/github/webhooks/pull-request-closed.ts

@@ -9,6 +9,7 @@ import {
   findTaskById,
   updateTaskStatus,
 } from "../services/task-service";
+import { parseLinkMetadata } from "../utils/parse-link-metadata";
 import { resolveTargetStatus } from "../utils/resolve-column";
 
 type PRClosedPayload = {
@@ -59,9 +60,10 @@ export async function handlePullRequestClosed(payload: PRClosedPayload) {
       continue;
     }
 
-    const existingMetadata = externalLink.metadata
-      ? JSON.parse(externalLink.metadata)
-      : {};
+    const existingMetadata = parseLinkMetadata(externalLink.metadata, {
+      externalLinkId: externalLink.id,
+      source: "pull_request_closed",
+    });
 
     await updateExternalLink(externalLink.id, {
       metadata: {
@@ -82,7 +84,10 @@ export async function handlePullRequestClosed(payload: PRClosedPayload) {
 
       const hasOpenPRs = allTaskPRs.some((pr) => {
         if (pr.id === externalLink.id) return false;
-        const metadata = pr.metadata ? JSON.parse(pr.metadata) : {};
+        const metadata = parseLinkMetadata(pr.metadata, {
+          externalLinkId: pr.id,
+          source: "pull_request_closed",
+        });
         return metadata.state === "open";
       });
 

tests/api/plugins/github/utils/parse-link-metadata.test.ts

@@ -0,0 +1,38 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { parseLinkMetadata } from "../../../../../apps/api/src/plugins/github/utils/parse-link-metadata";
+
+const context = { externalLinkId: "link-1", source: "issue_closed" };
+
+afterEach(() => {
+  vi.restoreAllMocks();
+});
+
+describe("parseLinkMetadata", () => {
+  it("reads a well formed object", () => {
+    expect(
+      parseLinkMetadata('{"state":"open","createdFrom":"kaneo"}', context),
+    ).toEqual({
+      state: "open",
+      createdFrom: "kaneo",
+    });
+  });
+
+  it("treats an absent value as no metadata", () => {
+    expect(parseLinkMetadata(null, context)).toEqual({});
+    expect(parseLinkMetadata(undefined, context)).toEqual({});
+    expect(parseLinkMetadata("", context)).toEqual({});
+  });
+
+  it("warns and carries on when the row cannot be parsed", () => {
+    const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined);
+
+    // A row truncated by an older write, which used to throw out of the handler
+    // and fail the whole webhook delivery.
+    expect(parseLinkMetadata('{"state":"op', context)).toEqual({});
+    expect(warn).toHaveBeenCalledOnce();
+    expect(warn.mock.calls[0][1]).toMatchObject({
+      externalLinkId: "link-1",
+      source: "issue_closed",
+    });
+  });
+});