diff --git a/backend/__tests__/unit/routes/slackOAuth.confirmRace.test.js b/backend/__tests__/unit/routes/slackOAuth.confirmRace.test.js new file mode 100644 index 0000000000..9a02463b71 --- /dev/null +++ b/backend/__tests__/unit/routes/slackOAuth.confirmRace.test.js @@ -0,0 +1,163 @@ +// A real Mongo row catches gate updates that a mocked findOneAndUpdate cannot. +jest.mock('jsonwebtoken', () => ({})); +jest.mock('../../../middleware/auth', () => (req, _res, next) => { + req.user = { id: '64b64c48c4f37a6b2f34c111' }; + next(); +}); +jest.mock('../../../middleware/integrationRateLimit', () => ({ + writeIntegrationsRateLimit: (_req, _res, next) => next(), + listIntegrationsRateLimit: (_req, _res, next) => next(), +})); +jest.mock('../../../models/InstallableInstallation', () => ({ findOne: jest.fn() })); +jest.mock('../../../services/connectorSecrets', () => ({ get: jest.fn() })); +jest.mock('../../../services/slackOAuthService', () => ({ + SlackOAuthConfigurationError: class extends Error {}, + SlackOAuthExchangeError: class extends Error {}, +})); +jest.mock('../../../services/installable/installableInstallationService', () => ({ + install: jest.fn(), uninstall: jest.fn(), + InstallLockLostError: class extends Error {}, + InstallableAlreadyInstalledError: class extends Error {}, + InstallableNotFoundError: class extends Error {}, + InstallableProjectionError: class extends Error {}, + InstallInProgressError: class extends Error {}, + InstallationPausedError: class extends Error {}, +})); +jest.mock('../../../services/connectorDeliveryFailureService', () => ({ + noteBoundChatDeliveryFailure: jest.fn().mockResolvedValue(false), +})); +jest.mock('../../../services/slackApi', () => { + const actual = jest.requireActual('../../../services/slackApi'); + const postMessage = jest.fn().mockResolvedValue({ ok: true, ts: '1.1' }); + const SlackApi = jest.fn().mockImplementation(() => ({ postMessage })); + SlackApi.escapeSlackMrkdwn = actual.escapeSlackMrkdwn; + SlackApi.postMessage = postMessage; + return SlackApi; +}); + +const express = require('express'); +const request = require('supertest'); +const mongoose = require('mongoose'); +const Integration = require('../../../models/Integration'); +const Pod = require('../../../models/Pod'); +const User = require('../../../models/User'); +const InstallableInstallation = require('../../../models/InstallableInstallation'); +const connectorSecrets = require('../../../services/connectorSecrets'); +const SlackApi = require('../../../services/slackApi'); +const { relayAgentMessageToSlack } = require('../../../services/slackBridgeService'); +const { setupMongoDb, closeMongoDb, clearMongoDb } = require('../../utils/testUtils'); +const routes = require('../../../routes/installables'); + +const app = express(); +app.use(express.json()); +app.use('/api/installables', routes); + +const ownerId = new mongoose.Types.ObjectId('64b64c48c4f37a6b2f34c111'); +const botId = new mongoose.Types.ObjectId(); + +describe('Slack lead confirmation gate race', () => { + beforeAll(async () => { + await setupMongoDb(); + await Integration.syncIndexes(); + }); + afterEach(async () => { + jest.restoreAllMocks(); + SlackApi.postMessage.mockClear(); + await clearMongoDb(); + }); + afterAll(async () => closeMongoDb()); + + const setup = async (gate) => { + await User.create([ + { _id: ownerId, username: 'owner', email: 'owner@example.test', password: 'Password123!' }, + { _id: botId, username: 'kai', email: 'kai@example.test', password: 'Password123!', isBot: true }, + ]); + const pod = await Pod.create({ + name: 'Team', type: 'team', createdBy: ownerId, members: [ownerId, botId], + }); + const installationId = new mongoose.Types.ObjectId().toString(); + InstallableInstallation.findOne.mockResolvedValue({ _id: installationId }); + connectorSecrets.get.mockResolvedValue('xoxb-test'); + const integration = await Integration.create({ + installationId, type: 'slack', scope: 'user', status: 'pending', + podId: pod._id, createdBy: ownerId, isActive: true, + config: { + pendingBind: { + teamId: 'T1', chatId: 'D1', slackUserId: 'U1', + botTokenRef: 'secret-ref', expiresAt: new Date(Date.now() + 60_000), + }, + ...(gate ? { gates: { [String(pod._id)]: gate } } : {}), + }, + }); + return { pod, integration }; + }; + + it.each([ + [{ enabled: false, mode: 'attention' }], + [{ enabled: true, mode: 'attention' }], + ])('preserves a concurrent owner gate change and does not relay an ordinary message', async (ownerGate) => { + const { pod, integration } = await setup({ + enabled: true, mode: 'mirror', since: new Date('2026-10-01'), + }); + const write = Integration.findOneAndUpdate.bind(Integration); + jest.spyOn(Integration, 'findOneAndUpdate').mockImplementationOnce(async (...args) => { + await Integration.updateOne( + { _id: integration._id }, + { $set: { + [`config.gates.${pod._id}.enabled`]: ownerGate.enabled, + [`config.gates.${pod._id}.mode`]: ownerGate.mode, + } }, + ); + return write(...args); + }); + + const response = await request(app).post('/api/installables/slack/confirm') + .send({ leadAgentUsername: 'kai' }); + expect(response.status).toBe(200); + const stored = await Integration.findById(integration._id).lean(); + expect(stored.config.gates[String(pod._id)]).toMatchObject({ ...ownerGate, lead: 'kai' }); + + SlackApi.postMessage.mockClear(); // Ignore the intentional connect marker. + await relayAgentMessageToSlack({ + podId: String(pod._id), agentUsername: 'other', displayName: 'Other', + content: 'ordinary pod message', integration: stored, + }); + expect(SlackApi.postMessage).not.toHaveBeenCalled(); + }); + + it('does not overwrite a gate created while the bind was being confirmed', async () => { + const { pod, integration } = await setup(null); + const write = Integration.findOneAndUpdate.bind(Integration); + jest.spyOn(Integration, 'findOneAndUpdate').mockImplementationOnce(async (...args) => { + await Integration.updateOne( + { _id: integration._id }, + { $set: { + [`config.gates.${pod._id}`]: { + enabled: false, mode: 'attention', since: new Date(), + }, + } }, + ); + return write(...args); + }); + + const response = await request(app).post('/api/installables/slack/confirm') + .send({ leadAgentUsername: 'kai' }); + expect(response.status).toBe(409); + const stored = await Integration.findById(integration._id).lean(); + expect(stored.config.gates[String(pod._id)]).toMatchObject({ + enabled: false, mode: 'attention', + }); + expect(stored.config.chatId).toBeUndefined(); + }); + + it('initializes a missing gate when nobody changed it during confirmation', async () => { + const { pod, integration } = await setup(null); + const response = await request(app).post('/api/installables/slack/confirm') + .send({ leadAgentUsername: 'kai' }); + expect(response.status).toBe(200); + const stored = await Integration.findById(integration._id).lean(); + expect(stored.config.gates[String(pod._id)]).toMatchObject({ + enabled: true, lead: 'kai', since: expect.any(Date), + }); + }); +}); diff --git a/backend/__tests__/unit/routes/slackOAuth.installables.test.js b/backend/__tests__/unit/routes/slackOAuth.installables.test.js index 1a382b2a9c..b1f3db0d06 100644 --- a/backend/__tests__/unit/routes/slackOAuth.installables.test.js +++ b/backend/__tests__/unit/routes/slackOAuth.installables.test.js @@ -13,7 +13,7 @@ jest.mock('../../../middleware/integrationRateLimit', () => ({ listIntegrationsRateLimit: (_req, _res, next) => next(), })); jest.mock('../../../models/Pod', () => ({ findById: jest.fn() })); -jest.mock('../../../models/User', () => ({ findById: jest.fn() })); +jest.mock('../../../models/User', () => ({ findById: jest.fn(), findOne: jest.fn() })); jest.mock('../../../models/Integration', () => ({ findOne: jest.fn(), findOneAndUpdate: jest.fn(), findById: jest.fn(), updateOne: jest.fn(), collection: { indexExists: jest.fn() }, @@ -294,6 +294,54 @@ describe('Slack installable OAuth routes', () => { expect(SlackApi.mock.results[0].value.postMessage).toHaveBeenCalledWith('D1', '[<Evil|pod>] connected'); }); + test('stores the selected lead on the active pod gate at confirmation', async () => { + const pending = { + teamId: 'T1', slackUserId: 'U1', chatId: 'D1', botTokenRef: 'secret-ref', + expiresAt: new Date(Date.now() + 60_000), + }; + const since = new Date('2026-10-01T00:00:00.000Z'); + Integration.findOne.mockResolvedValue({ + ...integration, + config: { pendingBind: pending, gates: { [integration.podId]: { enabled: false, mode: 'attention', since } } }, + }); + Pod.findById + .mockResolvedValueOnce({ members: [ownerId, 'bot-1'], type: 'team' }) + .mockReturnValueOnce({ select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ name: 'Pod' }) }) }); + User.findOne.mockResolvedValue({ _id: 'bot-1', username: 'kai', isBot: true }); + Integration.findOneAndUpdate.mockResolvedValue({ ...integration, status: 'connected', config: { ...pending, chatType: 'im' } }); + SlackApi.mockImplementationOnce(() => ({ postMessage: jest.fn().mockResolvedValue({ ok: true, ts: '1.1' }) })); + + const response = await request(app).post('/api/installables/slack/confirm') + .send({ leadAgentUsername: 'kai' }); + + expect(response.status).toBe(200); + expect(User.findOne).toHaveBeenCalledWith({ + _id: { $in: [ownerId, 'bot-1'] }, username: 'kai', isBot: true, + }); + expect(Integration.findOneAndUpdate.mock.calls[0][1].$set[ + `config.gates.${integration.podId}.lead` + ]).toBe('kai'); + expect(Integration.findOneAndUpdate.mock.calls[0][1].$set[ + `config.gates.${integration.podId}` + ]).toBeUndefined(); + }); + + test('refuses a selected lead who is not a bot member without binding the DM', async () => { + Integration.findOne.mockResolvedValue({ + ...integration, + config: { pendingBind: { teamId: 'T1', chatId: 'D1', botTokenRef: 'secret-ref', expiresAt: new Date(Date.now() + 60_000) } }, + }); + Pod.findById.mockResolvedValue({ members: [ownerId], type: 'team' }); + User.findOne.mockResolvedValue(null); + + const response = await request(app).post('/api/installables/slack/confirm') + .send({ leadAgentUsername: 'outsider' }); + + expect(response.status).toBe(400); + expect(response.body.code).toBe('slack_lead_invalid'); + expect(Integration.findOneAndUpdate).not.toHaveBeenCalled(); + }); + test('refuses a Slack DM already bound to another Commonly account by name', async () => { const pending = { teamId: 'T1', slackUserId: 'U1', chatId: 'D1', botTokenRef: 'secret-ref', diff --git a/backend/__tests__/unit/services/slackBridgeService.test.js b/backend/__tests__/unit/services/slackBridgeService.test.js index ea065358cf..0e1bd9378e 100644 --- a/backend/__tests__/unit/services/slackBridgeService.test.js +++ b/backend/__tests__/unit/services/slackBridgeService.test.js @@ -1,6 +1,6 @@ jest.mock('../../../models/Integration', () => ({ findOne: jest.fn(), findByIdAndUpdate: jest.fn() })); jest.mock('../../../models/Pod', () => ({ findById: jest.fn() })); -jest.mock('../../../models/User', () => ({ findById: jest.fn() })); +jest.mock('../../../models/User', () => ({ findById: jest.fn(), findOne: jest.fn() })); jest.mock('../../../models/pg/Message', () => ({ create: jest.fn(), findById: jest.fn() })); jest.mock('../../../services/messageAgentDeliveryService', () => ({ deliverMessageToAgents: jest.fn() })); jest.mock('../../../config/socket', () => ({ getIO: jest.fn(() => null) })); @@ -142,9 +142,9 @@ describe('Slack installable bridge', () => { // Two fixture shapes, because the routed pod and the active pod are different // documents: the first names the pod that may not be reachable any more. - const podWithMembers = (name) => ({ + const podWithMembers = (name, members = ['user-1']) => ({ select: jest.fn().mockReturnValue({ - lean: jest.fn().mockResolvedValue({ name, type: 'team', members: ['user-1'] }), + lean: jest.fn().mockResolvedValue({ name, type: 'team', members }), }), }); const podsById = (gatedPodName) => (id) => ({ @@ -211,6 +211,132 @@ describe('Slack installable bridge', () => { expect(PGMessage.create.mock.calls[0][0]).toBe('pod-1'); }); + test('addresses an unquoted plain message to the active pod lead', async () => { + Pod.findById.mockReturnValue(podWithMembers('Alpha', ['user-1', 'bot-1'])); + User.findOne.mockResolvedValue({ _id: 'bot-1', username: 'kai', isBot: true }); + PGMessage.create.mockImplementation((_podId, _userId, content) => Promise.resolve({ id: 'pg-1', content })); + PGMessage.findById.mockResolvedValue(null); + User.findById.mockReturnValue({ + select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ username: 'sam' }) }), + }); + deliverMessageToAgents.mockResolvedValue(undefined); + + const result = await relaySlackMessageToPod({ + integration: { + ...integration, + scope: 'user', + config: { ...integration.config, linkedUserId: 'user-1', slackUserId: 'U1', gates: { 'pod-1': { enabled: true, lead: 'kai' } } }, + }, + event: { text: 'hello', user: 'U1' }, + }); + + expect(result).toEqual({ relayed: true, routedAgent: 'kai' }); + expect(PGMessage.create.mock.calls[0][2]).toBe('💬 (via Slack): @kai hello'); + expect(User.findOne).toHaveBeenCalledWith({ + _id: { $in: ['user-1', 'bot-1'] }, username: 'kai', isBot: true, + }); + expect(deliverMessageToAgents.mock.calls[0][0].message).toEqual(expect.objectContaining({ + id: 'pg-1', content: '💬 (via Slack): @kai hello', + })); + expect(SlackApi).not.toHaveBeenCalled(); + }); + + test('keeps an explicit mention instead of substituting the selected lead', async () => { + Pod.findById.mockReturnValue(podWithMembers('Alpha')); + PGMessage.create.mockResolvedValue({ id: 'pg-1' }); + PGMessage.findById.mockResolvedValue({ id: 'pg-1', content: 'relayed' }); + User.findById.mockReturnValue({ + select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ username: 'sam' }) }), + }); + deliverMessageToAgents.mockResolvedValue(undefined); + + await relaySlackMessageToPod({ + integration: { + ...integration, + scope: 'user', + config: { ...integration.config, linkedUserId: 'user-1', slackUserId: 'U1', gates: { 'pod-1': { enabled: true, lead: 'kai' } } }, + }, + event: { text: '@vera please review', user: 'U1' }, + }); + + expect(PGMessage.create.mock.calls[0][2]).toBe('💬 (via Slack): @vera please review'); + expect(SlackApi).not.toHaveBeenCalled(); + }); + + test('acknowledges an unaddressed message when the active pod has no lead', async () => { + Pod.findById.mockReturnValue(podWithMembers('Alpha')); + PGMessage.create.mockResolvedValue({ id: 'pg-1' }); + PGMessage.findById.mockResolvedValue({ id: 'pg-1', content: 'relayed' }); + User.findById.mockReturnValue({ + select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ username: 'sam' }) }), + }); + deliverMessageToAgents.mockResolvedValue(undefined); + + await relaySlackMessageToPod({ + integration: { + ...integration, + scope: 'user', + config: { ...integration.config, linkedUserId: 'user-1', slackUserId: 'U1', gates: {} }, + }, + event: { text: 'hello', user: 'U1' }, + }); + + expect(PGMessage.create.mock.calls[0][2]).toBe('💬 (via Slack): hello'); + await new Promise((resolve) => setImmediate(resolve)); + expect(SlackApi.mock.results[0].value.postMessage).toHaveBeenCalledWith( + 'D1', expect.stringContaining('no lead agent is selected'), + ); + }); + + test('a stalled no-lead Slack notice cannot hold pod delivery', async () => { + Pod.findById.mockReturnValue(podWithMembers('Alpha')); + PGMessage.create.mockResolvedValue({ id: 'pg-1' }); + PGMessage.findById.mockResolvedValue({ id: 'pg-1', content: 'relayed' }); + User.findById.mockReturnValue({ + select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ username: 'sam' }) }), + }); + deliverMessageToAgents.mockResolvedValue(undefined); + SlackApi.mockImplementationOnce(() => ({ postMessage: jest.fn(() => new Promise(() => {})) })); + + const result = await relaySlackMessageToPod({ + integration: { + ...integration, + scope: 'user', + config: { ...integration.config, linkedUserId: 'user-1', slackUserId: 'U1', gates: {} }, + }, + event: { text: 'hello', user: 'U1' }, + }); + + expect(result.relayed).toBe(true); + expect(deliverMessageToAgents).toHaveBeenCalledTimes(1); + }); + + test('does not wake a selected lead after that bot leaves the pod', async () => { + Pod.findById.mockReturnValue(podWithMembers('Alpha')); + User.findOne.mockResolvedValue(null); + PGMessage.create.mockResolvedValue({ id: 'pg-1' }); + PGMessage.findById.mockResolvedValue({ id: 'pg-1', content: 'relayed' }); + User.findById.mockReturnValue({ + select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ username: 'sam' }) }), + }); + deliverMessageToAgents.mockResolvedValue(undefined); + + await relaySlackMessageToPod({ + integration: { + ...integration, + scope: 'user', + config: { ...integration.config, linkedUserId: 'user-1', slackUserId: 'U1', gates: { 'pod-1': { enabled: true, lead: 'kai' } } }, + }, + event: { text: 'hello', user: 'U1' }, + }); + + expect(PGMessage.create.mock.calls[0][2]).toBe('💬 (via Slack): hello'); + await new Promise((resolve) => setImmediate(resolve)); + expect(SlackApi.mock.results[0].value.postMessage).toHaveBeenCalledWith( + 'D1', expect.stringContaining('no lead agent is selected'), + ); + }); + test('refuses a thread reply into a pod whose gate is off, naming it, posting nothing', async () => { Pod.findById.mockImplementation(podsById('Launch')); diff --git a/backend/routes/installables.ts b/backend/routes/installables.ts index 4d08c5919a..9562212c01 100644 --- a/backend/routes/installables.ts +++ b/backend/routes/installables.ts @@ -443,6 +443,16 @@ router.post('/slack/confirm', writeIntegrationsRateLimit, auth, async (req: Auth if (!pod || !isConnectorTargetPod(pod, userId)) { return slackError(res, 403, 'slack_pod_access_denied', 'You no longer have access to this pod.'); } + const selectedLead = req.body?.leadAgentUsername; + if (selectedLead !== undefined) { + if (typeof selectedLead !== 'string' || !/^[a-z0-9_-]{2,}$/i.test(selectedLead)) { + return slackError(res, 400, 'slack_lead_invalid', 'Choose an agent in the active pod.'); + } + const member = await User.findOne({ + _id: { $in: pod.members }, username: selectedLead, isBot: true, + }); + if (!member) return slackError(res, 400, 'slack_lead_invalid', 'Choose an agent in the active pod.'); + } // A preflight lookup alone cannot prevent two replicas confirming the same // DM at once. Refuse new binds until the unique index has been installed. const unavailable = 'Slack connections are temporarily unavailable. Try again later.'; @@ -456,6 +466,13 @@ router.post('/slack/confirm', writeIntegrationsRateLimit, auth, async (req: Auth const bindingSelector = activeSlackDm(String(pending.teamId), String(pending.chatId), owned.integration._id); const existing = await Integration.findOne(bindingSelector); if (existing && String(existing._id) !== String(owned.integration._id)) return slackBindConflict(res, existing); + const activePodId = String(owned.integration.podId); + const gates = owned.integration.config?.gates; + const oldGate = gates?.get?.(activePodId) || gates?.[activePodId]; + // For an existing gate, change only the selected lead. Replaying the gate + // snapshot would undo an owner's concurrent enable/mode change. A missing + // gate may be initialized only while it is still absent at the atomic bind. + const initializeGate = Boolean(selectedLead && !oldGate); let confirmed: any; try { confirmed = await Integration.findOneAndUpdate( @@ -466,6 +483,9 @@ router.post('/slack/confirm', writeIntegrationsRateLimit, auth, async (req: Auth 'config.pendingBind.botTokenRef': pending.botTokenRef, 'config.pendingBind.expiresAt': { $gt: new Date() }, 'config.chatId': { $exists: false }, + ...(selectedLead ? { + [`config.gates.${activePodId}`]: { $exists: !initializeGate }, + } : {}), }, { $set: { @@ -483,6 +503,9 @@ router.post('/slack/confirm', writeIntegrationsRateLimit, auth, async (req: Auth 'config.chatId': pending.chatId, 'config.chatType': 'im', 'config.botTokenRef': pending.botTokenRef, + ...(selectedLead ? (initializeGate + ? { [`config.gates.${activePodId}`]: { enabled: true, lead: selectedLead, since: new Date() } } + : { [`config.gates.${activePodId}.lead`]: selectedLead }) : {}), }, $unset: { 'config.pendingBind': 1 }, }, diff --git a/backend/services/slackBridgeService.ts b/backend/services/slackBridgeService.ts index e1b2a082fa..1b135de69b 100644 --- a/backend/services/slackBridgeService.ts +++ b/backend/services/slackBridgeService.ts @@ -44,7 +44,7 @@ interface SlackIntegrationDoc { slackUserId?: string; liveRelay?: boolean; relayAllAgentMessages?: boolean; - gates?: Record; + gates?: Record; leadAgentUsername?: string; relayMutedUntil?: Date | string; adminPause?: { reason?: string; at?: Date | string; adminId?: string }; @@ -130,6 +130,8 @@ const isInboundRelayableIntegration = (integration: SlackIntegrationDoc, podId: ); const NO_ACTIVE_POD_REPLY = 'This connector has no active pod. Choose one in Commonly first.'; +const NO_LEAD_REPLY = 'Your message reached the pod, but no lead agent is selected there. ' + + 'Choose a lead in Commonly to get replies here.'; // ADR-025 D11: a quote-reply whose pod is no longer reachable is refused in the // chat and posted nowhere. Naming the pod is the whole point — "your reply did @@ -172,6 +174,19 @@ const replyRoutedPodRefused = async ( } }; +const replyNoLead = async (integration: SlackIntegrationDoc): Promise => { + const chatId = integration.config?.chatId; + const botTokenRef = integration.config?.botTokenRef; + if (!chatId || !botTokenRef) return; + try { + const token = await connectorSecrets.get(String(botTokenRef)); + const sent = await new SlackApi(token).postMessage(String(chatId), NO_LEAD_REPLY); + await deliveryFailures.noteBoundChatDeliveryFailure(integration, chatId, sent); + } catch (error) { + console.warn('[slack-bridge] could not send no-lead reply:', (error as Error).message); + } +}; + const findLiveIntegration = async (podId: string): Promise => ( Integration.findOne({ type: 'slack', @@ -443,13 +458,14 @@ export const relaySlackMessageToPod = async (opts: { } podId = String(routed.podId); } - const { content: routedText, routedAgent } = routed; + // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require + const { extractMentions } = require('./agentMentionService'); + const lead = config.gates?.[podId]?.lead; + const plainMessage = !routed.routedAgent && !event.thread_ts && !cardReply.lateReply + && extractMentions(rawText).length === 0; const replyToMessageId = cardReply.lateReply?.messageId || null; const linkedUserId = String(config.linkedUserId); const senderName = event.user_profile?.display_name || event.user_profile?.real_name; - const content = senderName - ? `💬 ${senderName} (via Slack): ${routedText}` - : `💬 (via Slack): ${routedText}`; // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require const User = require('../models/User'); @@ -473,6 +489,20 @@ export const relaySlackMessageToPod = async (opts: { console.warn('[slack-bridge] inbound dropped — linked user missing'); return { relayed: false }; } + let addressedLead: string | null = null; + if (plainMessage && typeof lead === 'string' && /^[a-z0-9_-]{2,}$/i.test(lead)) { + try { + const member = await User.findOne({ _id: { $in: pod.members }, username: lead, isBot: true }); + if (member) addressedLead = lead; + } catch (error) { + console.warn('[slack-bridge] lead membership lookup failed:', (error as Error).message); + } + } + const routedText = addressedLead ? `@${addressedLead} ${routed.content}` : routed.content; + const routedAgent = routed.routedAgent || addressedLead; + const content = senderName + ? `💬 ${senderName} (via Slack): ${routedText}` + : `💬 (via Slack): ${routedText}`; try { // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require const PGPod = require('../models/pg/Pod'); @@ -534,6 +564,9 @@ export const relaySlackMessageToPod = async (opts: { } catch (error) { console.warn('[slack-bridge] socket emit failed:', (error as Error).message); } + // Slack's notice is best effort. A slow Slack API must not hold the inbound + // webhook after the pod message has been stored and delivered. + if (plainMessage && !addressedLead) void replyNoLead(integration); return { relayed: true, routedAgent }; }; diff --git a/docs-site/integrations/slack.mdx b/docs-site/integrations/slack.mdx index 7f6e596465..2ff6121a74 100644 --- a/docs-site/integrations/slack.mdx +++ b/docs-site/integrations/slack.mdx @@ -9,7 +9,8 @@ description: Pod messages in a Slack direct message 2. Choose a pod under **Pod to bridge**, then select **Connect**. 3. Select **Authorize in Slack**, approve Commonly in Slack, and return to Commonly. -4. Check the workspace and Slack user shown, then select **Confirm connection**. +4. Check the workspace and Slack user shown, choose an agent for plain messages, + then select **Confirm connection**. The live mobile view shows **Add** on Slack before authorization. The connected state appears after confirmation. @@ -18,9 +19,10 @@ state appears after confirmation. Slack Add button on the mobile Connectors page before authorization -With an agent in the pod, its messages can reach a Slack direct message. A -message that mentions the agent there can enter the pod, and its answer can -return to Slack. The relay identifies the pod and agent. +Plain Slack messages reach the active pod and address its selected agent. +An explicit `@agent` or a reply to a relayed agent line keeps that target. +Agent replies can return to Slack. If the pod has no agent yet, messages still +reach it and Slack tells you to choose a lead later. If you operate the Slack app for a self-hosted instance, enable the App Home messages tab and verify the Events Request URL in Slack's app settings so diff --git a/docs/slack/README.md b/docs/slack/README.md index c84797f331..f18d5ef31b 100644 --- a/docs/slack/README.md +++ b/docs/slack/README.md @@ -2,8 +2,9 @@ Commonly connects a Slack direct message to a pod. The person connecting it authorizes the Slack app, checks the workspace and Slack user in Commonly, and -confirms the connection. Messages that mention an agent can enter the pod; the -agent's answer can return to the DM. See the [user walkthrough](../../docs-site/integrations/slack.mdx). +confirms the connection. A plain DM addresses the active pod's selected lead; +an explicit `@agent` or a reply to a relayed agent line keeps its own target. +The agent's answer can return to the DM. See the [user walkthrough](../../docs-site/integrations/slack.mdx). ## Configure the Slack app @@ -41,9 +42,13 @@ connection. Do not paste a signing secret or bot token into a connection row. ## Verify the connection In Commonly, choose **Connectors → Slack → Add**, select a pod, authorize the -app in Slack, and confirm the returned workspace and user. Send a DM that -mentions a pod agent and check that it reaches the pod and that the answer -returns to Slack. The HTTP events route accepts signed `message.im` events; +app in Slack, choose an agent from the selected pod at confirmation, and +confirm the returned workspace and user. Send a plain DM and check that it +addresses that agent in the pod and that the answer returns to Slack. If the +pod has no agent yet, confirmation remains available; a plain DM reaches the +pod and gets an explicit notice in Slack that no lead is selected. The lead +belongs to each pod's gate, so changing the active pod changes the plain DM +target. The HTTP events route accepts signed `message.im` events; the relay requires an active DM binding. `/commonly status` reports the relay mode, and `/commonly mode mirror|attention` changes it for that binding. diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index a31ff9e11c..85e4adf6d5 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -2071,7 +2071,7 @@ "connectChannelHint": "Choose a channel and the pod it should join.", "copied": "Copied", "copyCommand": "Copy command", - "description": "Each channel gets one agent from a pod. It answers there in its own name; the rest of the team stays behind it.", + "description": "Connect a channel to a pod. Messages arrive there as you; choose what reaches the channel.", "errorLine": "The connection dropped.", "errorReconnect": "reconnect to resume", "gateLead": "Lead agent for {{pod}}", @@ -2148,6 +2148,11 @@ "slackConfirmConnection": "Confirm connection", "slackConfirmDetail": "waiting for you to confirm", "slackConfirmHint": "{{workspace}} wants to connect as {{user}}.", + "slackBindLead": "Agent for plain messages", + "slackBindLeadNone": "Choose an agent", + "slackLeadInvalid": "Choose an agent in the active pod.", + "slackLeadNone": "No lead selected", + "slackNoAgent": "No agent is in this pod yet. Messages will reach the pod; choose a lead later.", "slackConfirmRow": "{{workspace}} says {{user}} connected — is that you?", "slackReject": "This is not me", "title": "Connectors", diff --git a/frontend/src/i18n/locales/zh-CN.json b/frontend/src/i18n/locales/zh-CN.json index 3345572649..6dd96d3226 100644 --- a/frontend/src/i18n/locales/zh-CN.json +++ b/frontend/src/i18n/locales/zh-CN.json @@ -2062,7 +2062,7 @@ "connectChannelHint": "选择频道，以及它要加入的 Pod。", "copied": "已复制", "copyCommand": "复制命令", - "description": "每个频道从一个 Pod 接入一个智能体。它会在那里以自己的名义回复，其余团队成员都在幕后。", + "description": "将频道连接到 Pod。消息会以你的身份进入 Pod；你可以选择哪些内容回传到频道。", "errorLine": "连接已断开。", "errorReconnect": "重新连接以恢复", "gateLead": "{{pod}} 的主导智能体", @@ -2139,6 +2139,11 @@ "slackConfirmConnection": "确认连接", "slackConfirmDetail": "等待你确认", "slackConfirmHint": "{{workspace}} 想以 {{user}} 的身份连接。", + "slackBindLead": "回复普通消息的智能体", + "slackBindLeadNone": "选择智能体", + "slackLeadInvalid": "请选择当前 Pod 中的智能体。", + "slackLeadNone": "尚未选择负责人", + "slackNoAgent": "这个 Pod 里还没有智能体。消息会送达 Pod；你可以稍后选择负责人。", "slackConfirmRow": "{{workspace}} 显示 {{user}} 已连接，是你吗？", "slackReject": "这不是我", "title": "连接器", diff --git a/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx b/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx index 5f8a853f06..5c9e797002 100644 --- a/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx +++ b/frontend/src/v2/__tests__/V2ConnectorsPage.test.tsx @@ -609,6 +609,7 @@ describe('V2ConnectorsPage', () => { renderPage(); expect(await screen.findByText('Commonly HQ wants to connect as @sam.')).toBeInTheDocument(); + expect(screen.getByText('No agent is in this pod yet. Messages will reach the pod; choose a lead later.')).toBeInTheDocument(); fireEvent.click(screen.getByRole('button', { name: 'Confirm connection' })); await waitFor(() => expect(axios.post).toHaveBeenCalledWith('/api/installables/slack/confirm', {}, expect.anything())); @@ -635,6 +636,51 @@ describe('V2ConnectorsPage', () => { )); }); + it('requires a pod agent at Slack bind and sends the selected lead', async () => { + mockGets([{ + _id: 'i-slack-pending', installationId: 'install-slack-u1', type: 'slack', status: 'pending', + config: { pendingBind: { teamName: 'Commonly HQ', slackUserName: 'sam' } }, + podId: { _id: 'p1', name: 'Rewire Live Demo' }, + }]); + const priorGet = axios.get.getMockImplementation(); + axios.get.mockImplementation((url) => url === '/api/pods' + ? Promise.resolve({ data: [{ + _id: 'p1', name: 'Rewire Live Demo', type: 'chat', + members: [{ _id: 'agent-1', username: 'kai', isBot: true }], + }] }) + : priorGet(url)); + axios.post.mockResolvedValue({ data: { status: 'connected' } }); + renderPage(); + + const confirm = await screen.findByRole('button', { name: 'Confirm connection' }); + expect(confirm).toBeDisabled(); + const row = (await screen.findByRole('button', { name: 'View Slack' })).closest('article') as HTMLElement; + fireEvent.click(within(row).getByRole('button', { name: 'Confirm' })); + expect(axios.post).not.toHaveBeenCalledWith('/api/installables/slack/confirm', expect.anything(), expect.anything()); + fireEvent.change(screen.getByLabelText('Agent for plain messages'), { target: { value: 'kai' } }); + expect(confirm).toBeEnabled(); + fireEvent.click(confirm); + await waitFor(() => expect(axios.post).toHaveBeenCalledWith( + '/api/installables/slack/confirm', { leadAgentUsername: 'kai' }, expect.anything(), + )); + }); + + it('does not treat a failed pod read as an empty pod at Slack confirmation', async () => { + mockGets([{ + _id: 'i-slack-pending', installationId: 'install-slack-u1', type: 'slack', status: 'pending', + config: { pendingBind: { teamName: 'Commonly HQ', slackUserName: 'sam' } }, + podId: { _id: 'p1', name: 'Rewire Live Demo' }, + }]); + const priorGet = axios.get.getMockImplementation(); + axios.get.mockImplementation((url) => url === '/api/pods' + ? Promise.reject(new Error('pods unavailable')) : priorGet(url)); + renderPage(); + + expect(await screen.findByText('Commonly HQ wants to connect as @sam.')).toBeInTheDocument(); + expect(screen.queryByText('No agent is in this pod yet. Messages will reach the pod; choose a lead later.')).toBeNull(); + expect(screen.getByRole('button', { name: 'Confirm connection' })).toBeDisabled(); + }); + it('keeps rejection reachable from the selected Slack aside', async () => { mockGets([{ _id: 'i-slack-pending', installationId: 'install-slack-u1', type: 'slack', status: 'pending', diff --git a/frontend/src/v2/components/V2ConnectorsPage.tsx b/frontend/src/v2/components/V2ConnectorsPage.tsx index 092b27764e..7c3457cdcb 100644 --- a/frontend/src/v2/components/V2ConnectorsPage.tsx +++ b/frontend/src/v2/components/V2ConnectorsPage.tsx @@ -229,6 +229,7 @@ const SLACK_REFUSALS: Record = { slack_installation_not_found: { key: 'connectors.slackInstallationNotFound', defaultValue: 'Slack installation not found.' }, slack_bind_missing: { key: 'connectors.slackBindMissing', defaultValue: 'There is no Slack authorization to confirm.' }, slack_bind_expired: { key: 'connectors.slackBindExpired', defaultValue: 'Slack authorization expired. Start again.' }, + slack_lead_invalid: { key: 'connectors.slackLeadInvalid', defaultValue: 'Choose an agent in the active pod.' }, slack_pod_access_denied: { key: 'connectors.slackPodAccessDenied', defaultValue: 'You no longer have access to this pod.' }, }; @@ -322,6 +323,7 @@ const V2ConnectorsPage: React.FC = () => { // only in the page-level slot at the foot of the page. const [rowRefusal, setRowRefusal] = useState<{ key: string; message: string } | null>(null); const [slackCallbackError, setSlackCallbackError] = useState(null); + const [slackLeadChoices, setSlackLeadChoices] = useState>({}); const [hostedMcpCallbackError, setHostedMcpCallbackError] = useState(null); const pollRef = useRef | null>(null); const podPickerRef = useRef(null); @@ -699,13 +701,14 @@ const V2ConnectorsPage: React.FC = () => { } }; - const resolveSlackBind = async (connector: Connector, action: 'confirm' | 'reject', rowKey: string) => { + const resolveSlackBind = async (connector: Connector, action: 'confirm' | 'reject', rowKey: string, lead?: string) => { if (busyId) return; setBusyId(connector._id); setError(null); setRowRefusal(null); try { - await api.post(`/api/installables/slack/${action}`, {}); + await api.post(`/api/installables/slack/${action}`, action === 'confirm' && lead + ? { leadAgentUsername: lead } : {}); await load(); } catch (requestError) { setRowRefusal({ @@ -1203,7 +1206,9 @@ const V2ConnectorsPage: React.FC = () => { if (!connector) return undefined; if (action === 'new-code') return regenerateCode(connector); if (action === 'authorize') return authorizeSlack(connector, item.key); - if (action === 'confirm') return resolveSlackBind(connector, 'confirm', item.key); + // The row opens the aside where the active pod's lead can be chosen. + // Sending a confirm here would silently omit that choice. + if (action === 'confirm') return undefined; return undefined; }; @@ -1375,7 +1380,9 @@ const V2ConnectorsPage: React.FC = () => { disabled={disabled} onChange={(event) => { void writeGate(connector, pod._id, { ...gate, enabled, lead: event.target.value || undefined }); }} > - + {bots.map((bot) => )} {!active && ( @@ -1477,6 +1484,12 @@ const V2ConnectorsPage: React.FC = () => { const supportsRelayControls = isTelegram || isSlack; const mirror = connector.config?.relayAllAgentMessages === true; const pendingBind = connector.config?.pendingBind; + const activePod = podList.find((pod) => pod._id === connectorPodId(connector)); + const availableLeads = botMembers(activePod); + const candidateSlackLead = slackLeadChoices[connector._id] + ?? connector.config?.gates?.[String(connectorPodId(connector))]?.lead ?? ''; + const selectedSlackLead = availableLeads.some((bot) => bot.username === candidateSlackLead) + ? candidateSlackLead : ''; const userScoped = connector.scope === 'user' || Boolean(connector.config?.gates); const busy = busyFor(item); @@ -1517,8 +1530,24 @@ const V2ConnectorsPage: React.FC = () => { workspace: pendingBind.teamName || 'This Slack workspace', user: pendingBind.slackUserName ? `@${pendingBind.slackUserName}` : 'your Slack user', })}

+ + + {pods !== null && availableLeads.length === 0 && ( +

{t('connectors.slackNoAgent', { defaultValue: 'No agent is in this pod yet. Messages will reach the pod; choose a lead later.' })}

+ )}
-