fix(connectors): a dead chat becomes a named failure, and the relay stops

backend/__tests__/unit/routes/slackOAuth.installables.test.js

@@ -14,17 +14,24 @@ jest.mock('../../../middleware/integrationRateLimit', () => ({
 }));
 jest.mock('../../../models/Pod', () => ({ findById: jest.fn() }));
 jest.mock('../../../models/Integration', () => ({
-  findOne: jest.fn(), findOneAndUpdate: jest.fn(), updateOne: jest.fn(),
+  findOne: jest.fn(), findOneAndUpdate: jest.fn(), findById: jest.fn(), updateOne: jest.fn(),
 }));
 jest.mock('../../../models/InstallableInstallation', () => ({ findOne: jest.fn() }));
 jest.mock('../../../utils/secret', () => ({ randomSecret: jest.fn(() => 'nonce-value') }));
 jest.mock('../../../services/telegramConnectCode', () => ({
   mintConnectCode: jest.fn(() => ({ connectCode: 'r'.repeat(32), connectCodeExpiresAt: new Date(Date.now() + 60_000) })),
 }));
 jest.mock('../../../services/connectorSecrets', () => ({ get: jest.fn(), put: jest.fn(), revoke: jest.fn() }));
-jest.mock('../../../services/slackApi', () => jest.fn().mockImplementation(() => ({
-  openConversation: jest.fn(),
-})));
+// Same reason as slackBridgeService.test.js: the constructor is stubbed, the
+// escape is real — the connect marker's escaping is what this file asserts.
+jest.mock('../../../services/slackApi', () => {
+  const actual = jest.requireActual('../../../services/slackApi');
+  const mock = jest.fn().mockImplementation(() => ({
+    openConversation: jest.fn(),
+  }));
+  mock.escapeSlackMrkdwn = actual.escapeSlackMrkdwn;
+  return mock;
+});
 jest.mock('../../../services/slackOAuthService', () => {
   class SlackOAuthConfigurationError extends Error { constructor() { super('not configured'); this.code = 'slack_oauth_not_configured'; } }
   class SlackOAuthExchangeError extends Error { constructor() { super('exchange failed'); this.code = 'slack_oauth_exchange_failed'; } }
@@ -50,6 +57,7 @@ const InstallableInstallation = require('../../../models/InstallableInstallation
 const connectorSecrets = require('../../../services/connectorSecrets');
 const { mintConnectCode } = require('../../../services/telegramConnectCode');
 const SlackApi = require('../../../services/slackApi');
+const deliveryFailures = require('../../../services/connectorDeliveryFailureService');
 const slackOAuth = require('../../../services/slackOAuthService');
 const installableRoutes = require('../../../routes/installables');
 
@@ -230,7 +238,9 @@ describe('Slack installable OAuth routes', () => {
     Pod.findById
       .mockResolvedValueOnce({ createdBy: { toString: () => 'another' }, members: [ownerId] })
       .mockReturnValueOnce({
-        select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ name: 'Launch' }) }),
+        // Purposefully hostile: this name is interpolated into the marker a
+        // human reads in Slack, and mrkdwn would draw `<...>` as markup.
+        select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ name: '<Evil|pod>' }) }),
       });
     Integration.findOneAndUpdate.mockResolvedValue({
       ...integration,
@@ -252,7 +262,74 @@ describe('Slack installable OAuth routes', () => {
     );
     expect(JSON.stringify(response.body)).not.toContain('secret-ref');
     expect(JSON.stringify(response.body)).not.toContain('nonce-never-return');
-    expect(SlackApi.mock.results[0].value.postMessage).toHaveBeenCalledWith('D1', '[Launch] connected');
+    expect(SlackApi.mock.results[0].value.postMessage).toHaveBeenCalledWith('D1', '[&lt;Evil|pod&gt;] connected');
+  });
+
+  test('a reconnect clears the reason an earlier flip left on the row (wren 73838)', async () => {
+    const pending = {
+      teamId: 'T1', slackUserId: 'U1', chatId: 'D1', botTokenRef: 'secret-ref',
+      expiresAt: new Date(Date.now() + 60_000),
+    };
+    Integration.findOne.mockResolvedValue({ ...integration, config: { pendingBind: pending } });
+    Pod.findById
+      .mockResolvedValueOnce({ createdBy: { toString: () => 'another' }, members: [ownerId] })
+      .mockReturnValueOnce({ select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ name: 'Pod' }) }) });
+    Integration.findOneAndUpdate.mockResolvedValue({ ...integration, status: 'connected', config: { ...pending, chatType: 'im' } });
+    connectorSecrets.get.mockResolvedValue('xoxb-secret');
+    SlackApi.mockImplementationOnce(() => ({ postMessage: jest.fn().mockResolvedValue({ ok: true, ts: '1.1' }) }));
+
+    const response = await request(app).post('/api/installables/slack/confirm');
+
+    expect(response.status).toBe(200);
+    expect(response.body.status).toBe('connected');
+    // A bind is the only thing that proves the connector works again, so it is
+    // the only thing that may clear what a flip wrote — otherwise a reconnect
+    // after any failure keeps naming a reason that is no longer true.
+    expect(Integration.findOneAndUpdate).toHaveBeenCalledWith(
+      expect.objectContaining({ _id: 'integration-1' }),
+      expect.objectContaining({ $set: expect.objectContaining({ errorMessage: null, errorMessageUserFacing: false }) }),
+      expect.anything(),
+    );
+  });
+
+  test('a permanent refusal on the confirmation marker flips the connector and names the reason (wren 73837)', async () => {
+    const pending = {
+      teamId: 'T1', slackUserId: 'U1', chatId: 'D1', botTokenRef: 'secret-ref',
+      expiresAt: new Date(Date.now() + 60_000),
+    };
+    // The marker goes to the chat this confirm just stored, so this is a
+    // bound-chat send — the twin of the Telegram bind confirmation.
+    Integration.findOne.mockResolvedValue({ ...integration, config: { pendingBind: pending } });
+    Pod.findById
+      .mockResolvedValueOnce({ createdBy: { toString: () => 'another' }, members: [ownerId] })
+      .mockReturnValueOnce({ select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ name: 'Pod' }) }) });
+    Integration.findOneAndUpdate.mockResolvedValue({ ...integration, status: 'connected', config: { ...pending, chatType: 'im' } });
+    connectorSecrets.get.mockResolvedValue('xoxb-secret');
+    SlackApi.mockImplementationOnce(() => ({ postMessage: jest.fn().mockResolvedValue({ ok: false, error: 'channel_not_found' }) }));
+    // The route re-reads after a flip, so the response cannot keep saying
+    // "connected" while the row says the connector needs attention.
+    Integration.findById.mockResolvedValue({
+      ...integration, status: 'error', errorMessage: deliveryFailures.SLACK_CHANNEL_GONE_REASON, config: {},
+    });
+
+    const response = await request(app).post('/api/installables/slack/confirm');
+
+    expect(response.status).toBe(200);
+    expect(response.body.status).toBe('error');
+    expect(Integration.findOneAndUpdate).toHaveBeenCalledWith(
+      { _id: 'integration-1', 'config.chatId': 'D1' },
+      {
+        $set: {
+          status: 'error',
+          errorMessage: deliveryFailures.SLACK_CHANNEL_GONE_REASON,
+          // The page renders the message only when this flag is set, so the
+          // marker's own refusal has to write the pair, not just the string.
+          errorMessageUserFacing: true,
+        },
+        $unset: { 'config.chatId': '' },
+      },
+      { new: true },
+    );
   });
 
   test('rejects a pending Slack bind and revokes its secret reference', async () => {

backend/__tests__/unit/routes/telegram.webhook.connectCode.test.js

@@ -9,7 +9,12 @@ jest.mock('../../../models/Pod');
 jest.mock('../../../models/Summary', () => ({ findOne: jest.fn() }));
 jest.mock('../../../services/integrationSummaryService', () => ({ createSummary: jest.fn() }));
 jest.mock('../../../services/agentEventService', () => ({ enqueue: jest.fn() }));
-jest.mock('../../../services/telegramService', () => ({ sendMessage: jest.fn() }));
+// Stub the network, keep the behaviour: the bridge escapes through this module's
+// escapeHtml, so a bare stub leaves it undefined and the send is swallowed.
+jest.mock('../../../services/telegramService', () => ({
+  ...jest.requireActual('../../../services/telegramService'),
+  sendMessage: jest.fn(),
+}));
 jest.mock('../../../integrations', () => ({ get: jest.fn() }));
 jest.mock('../../../services/telegramBridgeService', () => ({ relayTelegramMessageToPod: jest.fn() }));
 jest.mock('../../../models/WebhookDelivery', () => ({

backend/__tests__/unit/routes/telegram.webhook.test.js

@@ -6,7 +6,13 @@ jest.mock('../../../models/Pod');
 jest.mock('../../../models/Summary', () => ({ findOne: jest.fn() }));
 jest.mock('../../../services/integrationSummaryService', () => ({ createSummary: jest.fn() }));
 jest.mock('../../../services/agentEventService', () => ({ enqueue: jest.fn() }));
-jest.mock('../../../services/telegramService', () => ({ sendMessage: jest.fn() }));
+// `sendMessage` is stubbed because it is the network; `escapeHtml` must stay
+// real, because escaping is behaviour this route is responsible for and a mock
+// that dropped it turned every escaped send into a 500 (vera 73812).
+jest.mock('../../../services/telegramService', () => ({
+  ...jest.requireActual('../../../services/telegramService'),
+  sendMessage: jest.fn(),
+}));
 jest.mock('../../../integrations', () => ({ get: jest.fn() }));
 jest.mock('../../../services/telegramBridgeService', () => ({
   relayTelegramMessageToPod: jest.fn(),
@@ -82,7 +88,16 @@ describe('Telegram webhook routes', () => {
           'config.chatId': '42',
           'config.chatType': 'group',
         }),
+        // The bind is the event that clears a named failure, and only a bind
+        // knows the connector works again (wren 73779). The flag is the pair's
+        // other half: it is what the page reads before rendering the message.
+        errorMessage: null,
+        errorMessageUserFacing: false,
       }),
+      // `new: true` is read back by the confirmation's classification: without
+      // it the doc in hand still carries the PRE-bind chat id, and a permanent
+      // failure of that confirmation could never undo the bind it just wrote.
+      { new: true },
     );
     expect(telegramService.sendMessage).toHaveBeenCalledWith(
       'bot-token',
@@ -91,6 +106,34 @@ describe('Telegram webhook routes', () => {
     );
   });
 
+  it('escapes a pod name in the bind confirmation, which Telegram parses as HTML', async () => {
+    const integration = {
+      _id: 'integration-1',
+      podId: 'pod-1',
+      config: { connectCode: 'abc123', connectCodeExpiresAt: new Date(Date.now() + 60000) },
+    };
+    Integration.findOne = jest.fn().mockResolvedValueOnce(integration).mockResolvedValueOnce(null);
+    Integration.findByIdAndUpdate = jest.fn().mockResolvedValue({ config: { chatId: '42' } });
+    Pod.findById.mockReturnValue({
+      lean: jest.fn().mockResolvedValue({ name: 'R&D <b>ops</b>' }),
+    });
+
+    const res = await request(app)
+      .post('/api/webhooks/telegram')
+      .send({
+        message: {
+          text: '/commonly-enable abc123',
+          chat: { id: 42, title: 'Test Chat', type: 'group' },
+          from: { id: 7, first_name: 'Sam' },
+        },
+      });
+
+    expect(res.status).toBe(200);
+    const text = telegramService.sendMessage.mock.calls[0][2];
+    expect(text).toContain('Connected this chat to <b>R&amp;D &lt;b&gt;ops&lt;/b&gt;</b>');
+    expect(text).not.toContain('<b>ops</b>');
+  });
+
   it('posts integration summary on /summary', async () => {
     const integration = {
       _id: 'integration-1',
@@ -285,7 +328,7 @@ describe('bridge command surface (/mode /status /mute /help)', () => {
     expect(res.status).toBe(200);
     const [, update] = Integration.findByIdAndUpdate.mock.calls[0];
     const until = new Date(update.$set['config.relayMutedUntil']).getTime();
-    expect(until).toBeLessThanOrEqual(Date.now() + 24 * 60 * 60_000 + 5000);
+    expect(until).toBeLessThanOrEqual(Date.now() + 24 * 60 * 60000 + 5000);
     expect(until).toBeGreaterThan(Date.now());
   });
 
@@ -310,6 +353,52 @@ describe('bridge command surface (/mode /status /mute /help)', () => {
     );
   });
 
+  it('/status escapes the pod name and the lead agent handle it reports', async () => {
+    Integration.findOne = jest.fn().mockResolvedValue({
+      ...linked(),
+      config: {
+        ...linked().config,
+        leadAgentUsername: '<a href="https://evil.example">lead</a>',
+      },
+    });
+    Pod.findById.mockReturnValue({
+      lean: jest.fn().mockResolvedValue({ name: 'R&D <b>chat</b>' }),
+    });
+
+    const res = await post('/status');
+
+    expect(res.status).toBe(200);
+    const text = telegramService.sendMessage.mock.calls[0][2];
+    // Telegram HTML accepts <a href>, so an unescaped name renders a clickable
+    // link inside a message the user reads as the bot's own words.
+    expect(text).toContain('R&amp;D &lt;b&gt;chat&lt;/b&gt;');
+    expect(text).not.toContain('<b>chat</b>');
+    expect(text).toContain('&lt;a href="https://evil.example"&gt;lead&lt;/a&gt;');
+    expect(text).not.toContain('<a href');
+    // The formatting the route itself writes is untouched by the escaping.
+    expect(text).toContain('Pod: <b>');
+  });
+
+  it('/tldr escapes the summary title and body, which are generated from pod text', async () => {
+    Integration.findOne = jest.fn().mockResolvedValue(linked());
+    Summary.findOne.mockReturnValue({
+      sort: jest.fn().mockReturnValue({
+        lean: jest.fn().mockResolvedValue({
+          title: 'Sprint <script>alert(1)</script>',
+          content: 'Agent said <b>ship it</b> & left',
+        }),
+      }),
+    });
+
+    const res = await post('/tldr');
+
+    expect(res.status).toBe(200);
+    const text = telegramService.sendMessage.mock.calls[0][2];
+    expect(text).toContain('&lt;script&gt;alert(1)&lt;/script&gt;');
+    expect(text).not.toContain('<script>');
+    expect(text).toContain('Agent said &lt;b&gt;ship it&lt;/b&gt; &amp; left');
+  });
+
   it('commands never fall through to the live relay', async () => {
     const bridge = require('../../../services/telegramBridgeService');
     await post('/status');