fix(webhooks): isolate per-issue errors and deduplicate linked issues in handlePullRequest (#47)

src/github/github-webhooks.service.spec.ts

@@ -136,6 +136,82 @@ describe('GithubWebhooksService', () => {
     expect(bountiesService.markMergedAndRelease).not.toHaveBeenCalled();
   });
 
+  it('deduplicates issue numbers in PR body and processes bounty once (#47)', async () => {
+    issueRepo.findOne.mockResolvedValue({
+      id: 'issue-1',
+      bounty: { id: 'bounty-1' },
+    });
+    bountyRepo.findOne.mockResolvedValue({ id: 'bounty-1', status: 'claimed' });
+
+    const payload = {
+      action: 'closed',
+      number: 10,
+      pull_request: {
+        html_url: 'https://github.com/acme/repo/pull/10',
+        number: 10,
+        merged: true,
+        body: 'Fixes #12. This also resolves #12 as discussed in review.',
+      },
+      repository: { id: 999, full_name: 'acme/repo' },
+    };
+
+    const event = await service.handleEvent(
+      'pull_request',
+      'delivery-dedup',
+      payload,
+      true,
+    );
+
+    expect(bountiesService.markInReview).toHaveBeenCalledTimes(1);
+    expect(bountiesService.markMergedAndRelease).toHaveBeenCalledTimes(1);
+    expect(event.status).toBe(WebhookEventStatus.PROCESSED);
+  });
+
+  it('isolates per-issue errors and processes subsequent bounties when one fails (#47)', async () => {
+    issueRepo.findOne.mockImplementation(({ where }) => {
+      const num = where.number;
+      return Promise.resolve({
+        id: `issue-${num}`,
+        bounty: { id: `bounty-${num}` },
+      });
+    });
+    bountyRepo.findOne.mockImplementation(({ where }) => {
+      return Promise.resolve({ id: where.id, status: 'claimed' });
+    });
+
+    bountiesService.markMergedAndRelease.mockImplementation((bountyId: string) => {
+      if (bountyId === 'bounty-12') {
+        return Promise.reject(new Error('escrow release failed'));
+      }
+      return Promise.resolve({ id: bountyId, status: 'paid' });
+    });
+
+    const payload = {
+      action: 'closed',
+      number: 15,
+      pull_request: {
+        html_url: 'https://github.com/acme/repo/pull/15',
+        number: 15,
+        merged: true,
+        body: 'Fixes #12, Closes #34, Resolves #56',
+      },
+      repository: { id: 999, full_name: 'acme/repo' },
+    };
+
+    const event = await service.handleEvent(
+      'pull_request',
+      'delivery-multi',
+      payload,
+      true,
+    );
+
+    expect(bountiesService.markMergedAndRelease).toHaveBeenCalledWith('bounty-12');
+    expect(bountiesService.markMergedAndRelease).toHaveBeenCalledWith('bounty-34');
+    expect(bountiesService.markMergedAndRelease).toHaveBeenCalledWith('bounty-56');
+    expect(event.status).toBe(WebhookEventStatus.FAILED);
+    expect(event.error).toContain('Issue #12: escrow release failed');
+  });
+
   describe('"issues" webhook events (#24)', () => {
     const payload = {
       action: 'edited',

src/github/github-webhooks.service.ts

@@ -111,40 +111,56 @@ export class GithubWebhooksService {
       return;
     }
 
-    const issueNumbers = this.extractLinkedIssueNumbers(
+    const rawIssueNumbers = this.extractLinkedIssueNumbers(
       payload.pull_request.body ?? '',
     );
-    if (issueNumbers.length === 0) {
+    if (rawIssueNumbers.length === 0) {
       this.logger.warn(
         `PR #${payload.number} in ${payload.repository.full_name} merged but references no issue`,
       );
       return;
     }
 
+    // Deduplicate issue numbers so PR bodies repeating the same issue don't trigger duplicate processing
+    const issueNumbers = [...new Set(rawIssueNumbers)];
+    const errors: string[] = [];
+
     for (const number of issueNumbers) {
-      const issue = await this.issueRepo.findOne({
-        where: {
-          number,
-          repository: { githubRepoId: String(payload.repository.id) },
-        },
-        relations: { repository: true, bounty: true },
-      });
-      if (!issue?.bounty) continue;
-
-      // Mark in_review first if it hadn't been (idempotent no-op if already there).
-      const bounty = await this.bountyRepo.findOne({
-        where: { id: issue.bounty.id },
-      });
-      if (!bounty) continue;
-
-      if (bounty.status === BountyStatus.CLAIMED) {
-        await this.bountiesService.markInReview(
-          bounty.id,
-          payload.pull_request.html_url,
-          payload.pull_request.number,
+      try {
+        const issue = await this.issueRepo.findOne({
+          where: {
+            number,
+            repository: { githubRepoId: String(payload.repository.id) },
+          },
+          relations: { repository: true, bounty: true },
+        });
+        if (!issue?.bounty) continue;
+
+        // Mark in_review first if it hadn't been (idempotent no-op if already there).
+        const bounty = await this.bountyRepo.findOne({
+          where: { id: issue.bounty.id },
+        });
+        if (!bounty) continue;
+
+        if (bounty.status === BountyStatus.CLAIMED) {
+          await this.bountiesService.markInReview(
+            bounty.id,
+            payload.pull_request.html_url,
+            payload.pull_request.number,
+          );
+        }
+        await this.bountiesService.markMergedAndRelease(bounty.id);
+      } catch (err) {
+        const msg = `Issue #${number}: ${(err as Error).message}`;
+        this.logger.error(
+          `Failed processing linked bounty for ${msg} in PR #${payload.number}: ${(err as Error).stack}`,
         );
+        errors.push(msg);
       }
-      await this.bountiesService.markMergedAndRelease(bounty.id);
+    }
+
+    if (errors.length > 0) {
+      throw new Error(`Failed processing some linked bounties: ${errors.join('; ')}`);
     }
   }