From 71806a3033ff57563c11e5cec29dc022665b88e6 Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Tue, 1 Sep 2026 10:42:37 -0700
Subject: [PATCH] fix(webhooks): await the buffer handler so a rejection
 releases the claim
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Sprint-review gate catch: `return events(req, res)` un-awaited inside the
try meant an async rejection escaped the catch — the dedup claim survived,
and Telegram's redelivery was acked as a duplicate, dropping the update
permanently. Now `return await`, with a comment stating why.

Also pins the gate's two surviving mutants:
- async-rejecting buffer handler → 500 + claim released (the old sync mock
  could never exercise the escape)
- updates with no update_id bypass the claim entirely (never dedup against
  the literal key 'undefined')

And records the per-bot update_id key scope in the model comment.
17/17 tests; mutation-checked both new tests kill their mutants.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X8MxGhdgPini3Q14ay2vXS
---
 .../unit/routes/telegram.webhook.test.js      | 50 +++++++++++++++++++
 backend/models/WebhookDelivery.ts             |  6 +++
 backend/routes/webhooks/telegram.ts           |  5 +-
 3 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/backend/__tests__/unit/routes/telegram.webhook.test.js b/backend/__tests__/unit/routes/telegram.webhook.test.js
index 9d38eb550..1d0bb0aa7 100644
--- a/backend/__tests__/unit/routes/telegram.webhook.test.js
+++ b/backend/__tests__/unit/routes/telegram.webhook.test.js
@@ -350,6 +350,56 @@ describe('bridge command surface (/mode /status /mute /help)', () => {
       );
     });
 
+    // The real buffer handler is async and can reject (it awaits a Mongo
+    // write). A synchronous mock here would let an un-awaited
+    // `return events(req, res)` pass — the rejection must reach the catch so
+    // the claim is released and Telegram's redelivery is a retry, not a
+    // duplicate-acked drop.
+    it('releases the claim when the async buffer handler rejects', async () => {
+      const integration = {
+        _id: 'integration-1',
+        type: 'telegram',
+        config: { chatId: '42' },
+      };
+      Integration.findOne = jest.fn().mockResolvedValue(integration);
+      const events = jest.fn(async () => {
+        throw new Error('provider write failed');
+      });
+      registry.get.mockReturnValue({ getWebhookHandlers: () => ({ events }) });
+      const res = await request(app)
+        .post('/api/webhooks/telegram')
+        .send(liveUpdate(6));
+      expect(res.status).toBe(500);
+      expect(events).toHaveBeenCalled();
+      expect(WebhookDelivery.deleteOne).toHaveBeenCalledWith(
+        { provider: 'telegram', deliveryId: '6' },
+      );
+    });
+
+    // Updates with no update_id must bypass the claim entirely: without the
+    // guard they would all claim the literal key 'undefined' — the first one
+    // takes it and every later un-id'd update is acked and dropped.
+    it('processes every update that carries no update_id', async () => {
+      const integration = {
+        _id: 'integration-1',
+        type: 'telegram',
+        podId: 'pod-1',
+        config: { chatId: '42', liveRelay: true },
+      };
+      Integration.findOne = jest.fn().mockResolvedValue(integration);
+      bridge.relayTelegramMessageToPod.mockResolvedValue({});
+      const noIdUpdate = liveUpdate(undefined);
+      delete noIdUpdate.update_id;
+
+      const first = await request(app).post('/api/webhooks/telegram').send(noIdUpdate);
+      const second = await request(app).post('/api/webhooks/telegram').send(noIdUpdate);
+
+      expect(first.status).toBe(200);
+      expect(second.status).toBe(200);
+      expect(bridge.relayTelegramMessageToPod).toHaveBeenCalledTimes(2);
+      expect(WebhookDelivery.create).not.toHaveBeenCalled();
+    });
+
     it('a dedup-store outage does not take the bridge down', async () => {
       WebhookDelivery.create.mockRejectedValueOnce(new Error('mongo down'));
       const integration = {
diff --git a/backend/models/WebhookDelivery.ts b/backend/models/WebhookDelivery.ts
index 763258e38..e536e6fe4 100644
--- a/backend/models/WebhookDelivery.ts
+++ b/backend/models/WebhookDelivery.ts
@@ -17,6 +17,12 @@ import mongoose, { Document, Schema } from 'mongoose';
 //      routes/webhooks/telegram.ts.)
 // The TTL bounds the table: a delivery id only needs to be remembered for as
 // long as the provider keeps redelivering it.
+//
+// Key scope: {provider, deliveryId} assumes ONE id space per provider.
+// Telegram's update_id is sequential per BOT — correct while the route serves
+// a single bot; a second bot on the same route would collide id spaces and
+// drop legitimate updates as duplicates. Widen the key (e.g. include bot id)
+// before multi-bot.
 export interface IWebhookDelivery extends Document {
   provider: string;
   deliveryId: string;
diff --git a/backend/routes/webhooks/telegram.ts b/backend/routes/webhooks/telegram.ts
index 6cc220a4d..509c12c34 100644
--- a/backend/routes/webhooks/telegram.ts
+++ b/backend/routes/webhooks/telegram.ts
@@ -462,7 +462,10 @@ router.post('/', async (req: any, res: any) => {
 
     const provider = registry.get('telegram', integration);
     const { events } = provider.getWebhookHandlers();
-    return events(req, res);
+    // await, not return: a rejected promise returned from inside `try` escapes
+    // this catch (express 4 won't catch it either), and the claim above would
+    // survive to swallow Telegram's redelivery.
+    return await events(req, res);
   } catch (error) {
     console.error('Telegram webhook error', error);
     // Forget-on-error: the 500 makes Telegram redeliver this update_id; the