diff --git a/apps/api/src/handlers/mcp/__tests__/slack-thread-reply-quotes.test.ts b/apps/api/src/handlers/mcp/__tests__/slack-thread-reply-quotes.test.ts index cb9d378e3..78505ed91 100644 --- a/apps/api/src/handlers/mcp/__tests__/slack-thread-reply-quotes.test.ts +++ b/apps/api/src/handlers/mcp/__tests__/slack-thread-reply-quotes.test.ts @@ -11,6 +11,7 @@ const { getTaskChannelBindingsMock, maybeSendCommunicationThreadReplyMock, postMessageMock, + postTopLevelMessageMock, slackInstallationFindFirstMock, taskRunFindFirstMock, } = vi.hoisted(() => ({ @@ -21,6 +22,7 @@ const { getTaskChannelBindingsMock: vi.fn(), maybeSendCommunicationThreadReplyMock: vi.fn(), postMessageMock: vi.fn(), + postTopLevelMessageMock: vi.fn(), slackInstallationFindFirstMock: vi.fn(), taskRunFindFirstMock: vi.fn(), })); @@ -74,6 +76,7 @@ vi.mock('@roomote/slack', () => ({ SlackNotifier: vi.fn( class { postMessage = postMessageMock; + postTopLevelMessage = postTopLevelMessageMock; }, ), trackLatestUserMessageForSlackQuote: vi.fn(), @@ -178,6 +181,7 @@ describe('Slack thread reply quotes', () => { }, ]); postMessageMock.mockResolvedValue('333.444'); + postTopLevelMessageMock.mockResolvedValue({ messageTs: '333.444' }); clearLatestUserMessageForReplyQuoteIfIdMock.mockResolvedValue(true); }); @@ -213,6 +217,32 @@ describe('Slack thread reply quotes', () => { ); }); + it('preserves Slack API errors when a late-bound automation root cannot be posted', async () => { + taskRunFindFirstMock.mockResolvedValue({ + id: 42, + actingUserId: null, + taskId: 'task-1', + payload: { channel: 'C123', customAutomationId: 'automation-1' }, + }); + getTaskChannelBindingsMock.mockResolvedValue({ + slackChannelId: 'C123', + slackThreadTs: null, + }); + buildThreadReplyImageBlocksMock.mockResolvedValue([]); + postTopLevelMessageMock.mockResolvedValue({ error: 'not_in_channel' }); + + const response = await createApp().request('/mcp/thread_reply', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ text: 'done' }), + }); + + expect(response.status).toBe(502); + await expect(response.json()).resolves.toEqual({ + error: 'Slack chat.postMessage failed: not_in_channel', + }); + }); + it('consumes the exact pending quote after an image-only reply without rendering it', async () => { const response = await createApp().request('/mcp/thread_reply', { method: 'POST', diff --git a/apps/api/src/handlers/mcp/slack.ts b/apps/api/src/handlers/mcp/slack.ts index ac8a52421..88339302b 100644 --- a/apps/api/src/handlers/mcp/slack.ts +++ b/apps/api/src/handlers/mcp/slack.ts @@ -971,16 +971,19 @@ slackMcp.post('/thread_reply', async (c) => { blocks.push(...imageBlocks); blocks.push(...rootFooterBlocks); - const rootMessageTs = await slack.postMessage({ + const rootPost = await slack.postTopLevelMessage({ channel: slackReplyTarget.channel, text: getSlackFallbackText(fallbackText, imageBlocks.length), unfurl_links: false, unfurl_media: false, blocks, }); + const rootMessageTs = rootPost.messageTs; if (!rootMessageTs) { - throw new Error('Slack chat.postMessage returned no message timestamp'); + throw new Error( + `Slack chat.postMessage failed${rootPost.error ? `: ${rootPost.error}` : ' without a message timestamp'}`, + ); } // The root message is already visible in Slack; failing the reply here @@ -1343,11 +1346,8 @@ slackMcp.post('/thread_reply', async (c) => { ); } - if (message === 'Slack chat.postMessage returned no message timestamp') { - return c.json( - { error: 'Slack chat.postMessage returned no message timestamp' }, - 502, - ); + if (message.startsWith('Slack chat.postMessage failed')) { + return c.json({ error: message }, 502); } if (message === 'Slack thread source message no longer exists') { diff --git a/packages/sdk/src/server/automations/__tests__/custom-automations.test.ts b/packages/sdk/src/server/automations/__tests__/custom-automations.test.ts index a0a3349f2..9a918a9c7 100644 --- a/packages/sdk/src/server/automations/__tests__/custom-automations.test.ts +++ b/packages/sdk/src/server/automations/__tests__/custom-automations.test.ts @@ -1,5 +1,9 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; +const { isAppInChannelMock } = vi.hoisted(() => ({ + isAppInChannelMock: vi.fn(async () => true), +})); + vi.mock('@roomote/cloud-agents/server', () => ({ enqueueTask: vi.fn(), })); @@ -33,6 +37,12 @@ vi.mock('../destination', () => ({ listConnectedCommunicationProviders: vi.fn(async () => ['slack', 'teams']), })); +vi.mock('../../lib/communication-providers', () => ({ + getCommunicationProviderAdapter: vi.fn(async () => ({ + isAppInChannel: isAppInChannelMock, + })), +})); + vi.mock('../scheduling-utils', () => ({ DAILY_WEEKLY_SCHEDULE_HOUR_LOCAL: 3, isRunDue: vi.fn(), @@ -112,6 +122,7 @@ describe('customAutomationsJob', () => { vi.mocked(enqueueTask).mockResolvedValue({ taskId: 'task_abc', } as never); + isAppInChannelMock.mockResolvedValue(true); }); it('launches a StandardTask for due automations', async () => { @@ -221,6 +232,25 @@ describe('customAutomationsJob', () => { ); }); + it('fails before launch when the Slack app cannot access the report channel', async () => { + isAppInChannelMock.mockResolvedValue(false); + + const result = await customAutomationsJob(); + + expect(result.launchedTaskId).toBeNull(); + expect(enqueueTask).not.toHaveBeenCalled(); + expect(recordCustomAutomationRunOutcome).toHaveBeenCalledWith( + db, + expect.objectContaining({ + id: automation.id, + status: 'failed', + error: expect.stringContaining( + 'Slack app cannot access report channel C123', + ), + }), + ); + }); + it('launches without channel anchoring when no report channel is configured', async () => { vi.mocked(listEnabledCustomAutomations).mockResolvedValue([ { ...automation, target: {} } as never, @@ -339,6 +369,7 @@ describe('customAutomationsJob', () => { describe('runCustomAutomationNow', () => { beforeEach(() => { vi.clearAllMocks(); + isAppInChannelMock.mockResolvedValue(true); vi.mocked(getCustomAutomationById).mockResolvedValue(automation as never); vi.mocked(getCustomAutomationFrequency).mockReturnValue('daily'); vi.mocked(tryClaimCustomAutomationLaunch).mockResolvedValue(new Date()); diff --git a/packages/sdk/src/server/automations/custom-automations.ts b/packages/sdk/src/server/automations/custom-automations.ts index 51bd30882..78ed91a09 100644 --- a/packages/sdk/src/server/automations/custom-automations.ts +++ b/packages/sdk/src/server/automations/custom-automations.ts @@ -26,6 +26,7 @@ import { listConnectedCommunicationProviders, type ResolvedAutomationDestination, } from './destination'; +import { getCommunicationProviderAdapter } from '../lib/communication-providers'; import { isCronRunDue, resolveDeploymentTimeZone, @@ -246,6 +247,28 @@ async function launchCustomAutomationRow( }); return result; } + + if (destination.provider === 'slack') { + const slack = await getCommunicationProviderAdapter('slack'); + const isMember = + slack?.provider === 'slack' + ? await slack.isAppInChannel(destination.channelId) + : null; + if (isMember !== true) { + const message = + isMember === false + ? `Slack app cannot access report channel ${destination.channelId}. Invite the app to the channel or choose another destination.` + : `Slack report channel ${destination.channelId} could not be verified. Try again before running this automation.`; + result.skippedReason = message; + result.errors.push(message); + await recordCustomAutomationRunOutcome(db, { + id: automation.id, + status: 'failed', + error: message, + }); + return result; + } + } } // The short claim fence prevents concurrent launchers from double-launching diff --git a/packages/slack/src/__tests__/slack-notifier.test.ts b/packages/slack/src/__tests__/slack-notifier.test.ts index 780b0d3cf..39e0eb36b 100644 --- a/packages/slack/src/__tests__/slack-notifier.test.ts +++ b/packages/slack/src/__tests__/slack-notifier.test.ts @@ -131,6 +131,20 @@ describe('SlackNotifier', () => { expect(ts).toBeUndefined(); }); + it('preserves Slack API errors for top-level message callers', async () => { + getGlobalWithFetch().fetch = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ ok: false, error: 'not_in_channel' }), + }); + + const result = await notifier.postTopLevelMessage({ + channel: 'C123', + text: 'failure case', + }); + + expect(result).toEqual({ error: 'not_in_channel' }); + }); + it('does not change unfurl behavior for plain text messages', async () => { getGlobalWithFetch().fetch = vi.fn().mockResolvedValue({ ok: true, diff --git a/packages/slack/src/slack-notifier.ts b/packages/slack/src/slack-notifier.ts index 3771e43ba..b99fbb2ec 100644 --- a/packages/slack/src/slack-notifier.ts +++ b/packages/slack/src/slack-notifier.ts @@ -886,6 +886,26 @@ export class SlackNotifier { return response?.ts; } + /** + * Posts a top-level message while preserving Slack's API error code for + * callers that need to report why a new conversation could not be created. + */ + public async postTopLevelMessage(message: SlackMessage): Promise<{ + messageTs?: string; + error?: string; + }> { + const response = await this.sendMessage( + 'chat.postMessage', + message, + 'regular', + ); + + return { + ...(response?.ts ? { messageTs: response.ts } : {}), + ...(response?.error ? { error: response.error } : {}), + }; + } + public async postEphemeralMessage(message: SlackMessage & { user: string }) { const response = await this.sendMessage( 'chat.postEphemeral',