From bce66470dbea93a1d3ba2e3b58a2051bd0af51aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nathan=20=F0=9F=94=B6=20Tarbert?= <66887028+NathanTarbert@users.noreply.github.com> Date: Sat, 1 Aug 2026 14:02:42 -0400 Subject: [PATCH 1/9] feat(slack): mirror GitHub + Discord tickets into one internal Slack channel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every ticket opens a Slack thread; follow-ups and the AI's reply post underneath it, so one thread is the whole life of one ticket. Read-only in v1 — replying in Slack does not post back to the source. Thread identity reuses TicketExternalLink (plugin `slack`, externalId `channelId:ts`), the same table the Linear and GitHub links use, so the unique(ticketId, plugin) constraint is what prevents a ticket from ever opening two threads. Ships inert. SLACK_MIRROR_MODE defaults to `off` and an unrecognized value fails closed; with no SLACK_MIRROR_CHANNEL_ID the producers never enqueue. The flag is deliberately independent of SHADOW_MODE: that flag protects community surfaces where real reporters are watching, while this targets an internal team channel, so staging posting here is intended rather than a violation of the standing shadow-mode rule. Two correctness details worth calling out: - An AI reply is labelled with whether it actually reached the reporter. Shadow mode, a failed post, and a suppressed (ungrounded) draft all leave an AI Message row that nobody outside saw; mirroring those as if they were delivered would reproduce the divergence #148 describes. A suppressed run counts as undelivered even though a post succeeded, because what went out was the safe replacement copy, not the draft the mirror renders. - Slack-sourced tickets are never mirrored. If the mirror channel were also monitored, each mirror post would arrive as inbound, open a ticket, mirror again, and loop. Mirror failures never touch the reporter's path: enqueue errors are logged and swallowed in both producers. --- .env.example | 19 ++ apps/worker/src/index.ts | 3 + docs/deployment.md | 1 + .../queue/src/__tests__/ai-response.test.ts | 102 +++++++ .../outpost/queue/src/__tests__/queue.test.ts | 4 +- .../handlers/__tests__/slack-mirror.test.ts | 255 ++++++++++++++++++ .../outpost/queue/src/handlers/ai-response.ts | 35 ++- .../queue/src/handlers/slack-mirror.ts | 211 +++++++++++++++ packages/outpost/queue/src/index.ts | 2 + packages/outpost/queue/src/types.ts | 29 ++ .../src/__tests__/platforms-inbound.test.ts | 78 ++++++ .../outpost/shared/src/platforms/inbound.ts | 66 ++++- .../outpost/shared/src/platforms/index.ts | 5 + .../src/platforms/slack-mirror-config.ts | 56 ++++ 14 files changed, 862 insertions(+), 4 deletions(-) create mode 100644 packages/outpost/queue/src/handlers/__tests__/slack-mirror.test.ts create mode 100644 packages/outpost/queue/src/handlers/slack-mirror.ts create mode 100644 packages/outpost/shared/src/platforms/slack-mirror-config.ts diff --git a/.env.example b/.env.example index 0c06ed72..e6346101 100644 --- a/.env.example +++ b/.env.example @@ -66,6 +66,25 @@ SLACK_APP_TOKEN=xapp-... SLACK_SIGNING_SECRET= SLACK_SOCKET_MODE=true +# ─── Slack Ticket Mirror ───────────────────────────────────────────────────── +# Mirrors every GitHub + Discord ticket into one Slack channel; community +# follow-ups and the AI reply thread under it. Read-only (replying in Slack +# does NOT post back to the source). +# +# Mode: off (default, handler no-ops) | shadow (logs the message, posts nothing) +# | live (posts). Deliberately INDEPENDENT of SHADOW_MODE — that flag protects +# community surfaces (Discord/GitHub) where real reporters watch; this posts to +# an internal team channel, so staging posting here is intended. +SLACK_MIRROR_MODE=off +# Channel ID, NOT a channel name. Slack: open channel -> click its name -> +# bottom of the details pane -> Channel ID. Looks like C09AB2CD3EF. +SLACK_MIRROR_CHANNEL_ID= +# NOTE: SLACK_BOT_TOKEN above needs chat:write AND must be set on the WORKER +# service (outpost-worker) — the mirror handler runs there, not in the Slack +# bot. The bot must also be invited to the channel or posts fail not_in_channel. +# Do NOT list this channel in MONITORED_CHANNEL_IDS: the bot would read its own +# mirror posts as inbound messages and open a ticket for each one. + # ─── Teams Bot ─────────────────────────────────────────────────────────────── TEAMS_APP_ID= TEAMS_APP_PASSWORD= diff --git a/apps/worker/src/index.ts b/apps/worker/src/index.ts index 1e8a0c39..ff3f395b 100644 --- a/apps/worker/src/index.ts +++ b/apps/worker/src/index.ts @@ -32,6 +32,7 @@ import { createTrackerSyncHandler, handleJobCleanup, handleGithubReactionPoll, + handleSlackMirror, createJob, } from '@copilotkit/outpost/queue'; import { SyncEngine } from '@copilotkit/outpost/shared'; @@ -66,6 +67,7 @@ const worker = new Worker({ [JobType.TRACKER_SYNC]: 1, [JobType.JOB_CLEANUP]: 1, [JobType.GITHUB_REACTION_POLL]: 1, + [JobType.SLACK_MIRROR]: 2, }, jobTimeouts: { [JobType.AI_RESPONSE]: 120_000, // 2 minutes — AI pipeline is slow @@ -85,6 +87,7 @@ worker.on(JobType.HUBSPOT_SYNC, handleHubSpotSync); worker.on(JobType.TRACKER_SYNC, handleTrackerSync); worker.on(JobType.JOB_CLEANUP, handleJobCleanup); worker.on(JobType.GITHUB_REACTION_POLL, handleGithubReactionPoll); +worker.on(JobType.SLACK_MIRROR, handleSlackMirror); // ─── Start Scheduler ────────────────────────────────────────────────────── diff --git a/docs/deployment.md b/docs/deployment.md index ef1609cc..79f17cf7 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -79,6 +79,7 @@ Copy `.env.example` and fill in all values. Key groups: - **Discord**: `DISCORD_TOKEN`, `DISCORD_CLIENT_ID`, `GUILD_ID`, `MONITORED_CHANNEL_IDS` - **GitHub App**: `GITHUB_APP_ID`, `GITHUB_PRIVATE_KEY`, `GITHUB_INSTALLATION_ID`, `GITHUB_WEBHOOK_SECRET`, `GITHUB_TEAM_LOGINS` (optional) - **Slack**: `SLACK_BOT_TOKEN`, `SLACK_APP_TOKEN`, `SLACK_SIGNING_SECRET`, `MONITORED_CHANNEL_IDS`, `TEAM_MEMBER_IDS` (optional) +- **Slack ticket mirror** (`outpost-worker`, not the Slack bot): `SLACK_MIRROR_MODE` (`off` | `shadow` | `live`, default `off`), `SLACK_MIRROR_CHANNEL_ID` (channel ID, not name). Also needs `SLACK_BOT_TOKEN` with `chat:write` **set on the worker service** — it is otherwise only set on `outpost-slack-bot`, and the mirror handler runs in the worker. Invite the bot to the channel or posts fail `not_in_channel`. `SLACK_MIRROR_MODE` is intentionally independent of `SHADOW_MODE`: that flag protects community surfaces, while the mirror targets an internal channel. **Keep `SLACK_MIRROR_CHANNEL_ID` out of the Slack bot's `MONITORED_CHANNEL_IDS`** — a monitored mirror channel would turn each mirror post into a new inbound ticket. Slack-sourced tickets are never mirrored for the same reason, but keeping the channels disjoint is the durable fix. - **Teams**: `TEAMS_APP_ID`, `TEAMS_APP_PASSWORD`, `TEAMS_TENANT_ID` (optional, blank for multi-tenant), `MONITORED_CHANNEL_IDS` - **Linear sync**: `LINEAR_API_KEY`, `LINEAR_WEBHOOK_SECRET`, `LINEAR_TEAM_ID` - **Monitoring**: `SENTRY_DSN` (optional), `LOG_LEVEL` diff --git a/packages/outpost/queue/src/__tests__/ai-response.test.ts b/packages/outpost/queue/src/__tests__/ai-response.test.ts index c29d02b1..3ec12da5 100644 --- a/packages/outpost/queue/src/__tests__/ai-response.test.ts +++ b/packages/outpost/queue/src/__tests__/ai-response.test.ts @@ -65,6 +65,13 @@ const mockGetAdapter = vi.fn().mockReturnValue({ fetchUserInfo: vi.fn(), }); +/** Mutable so a test can switch the mirror on and assert the enqueue. */ +const mockMirrorConfig: { mode: string; channelId: string | null; token: string | null } = { + mode: 'off', + channelId: null, + token: null, +}; + vi.mock('@copilotkit/outpost/shared', () => ({ AI_CONFIDENCE: { AUTO_RESPOND: 0.9, @@ -82,6 +89,9 @@ vi.mock('@copilotkit/outpost/shared', () => ({ vi.mock('@copilotkit/outpost/shared/platforms', () => ({ hasAdapter: mockHasAdapter, getAdapter: mockGetAdapter, + readSlackMirrorConfig: () => mockMirrorConfig, + isSlackMirrorEnabled: (config: { mode: string; channelId: string | null }) => + config.mode !== 'off' && config.channelId !== null, })); // Import after mocks @@ -892,4 +902,96 @@ describe('restoreShadowMode', () => { expect(process.env.SHADOW_MODE).toBe('false'); }); + // ── Slack ticket mirror ────────────────────────────────────────────── + + describe('Slack ticket mirror enqueue', () => { + function mirrorJobs() { + return mockPrismaJob.create.mock.calls + .map((c) => c[0].data) + .filter((d: { type: string }) => d.type === 'SLACK_MIRROR'); + } + + beforeEach(() => { + // This describe sits outside the suite that clears mocks globally, + // so job calls would otherwise accumulate across these cases. + vi.clearAllMocks(); + mockPrismaTicket.update.mockResolvedValue({}); + mockPrismaMessage.create.mockResolvedValue({ id: 'msg-ai-1' }); + mockPrismaMessage.update.mockResolvedValue({}); + mockPrismaJob.create.mockResolvedValue({ id: 'job-1' }); + mockClassifyTicket.mockResolvedValue(sampleClassification); + mockGetFeedbackCalibration.mockResolvedValue(0); + mockPrismaTicket.findUnique.mockResolvedValue(sampleTicket); + mockGenerateSupportResponse.mockResolvedValue(highConfidenceResult); + mockHasAdapter.mockReturnValue(true); + mockGetAdapter.mockReturnValue({ + platform: 'DISCORD', + postResponse: mockPostResponse, + postSystemMessage: vi.fn(), + parseInboundEvent: vi.fn(), + fetchUserInfo: vi.fn(), + }); + }); + + afterEach(() => { + mockMirrorConfig.mode = 'off'; + mockMirrorConfig.channelId = null; + }); + + it('enqueues nothing while the mirror is off', async () => { + await handleAiResponse({ ticketId: 'tkt-1', source: 'discord' }, makeContext()); + + expect(mirrorJobs()).toHaveLength(0); + }); + + it('marks the reply delivered when the adapter posted it', async () => { + mockMirrorConfig.mode = 'live'; + mockMirrorConfig.channelId = 'C0MIRROR'; + + await handleAiResponse({ ticketId: 'tkt-1', source: 'discord' }, makeContext()); + + const jobs = mirrorJobs(); + expect(jobs).toHaveLength(1); + expect(jobs[0].payload).toEqual( + expect.objectContaining({ kind: 'reply', delivered: true }), + ); + }); + + // Shadow mode logs the draft but posts nothing, so the reporter never + // saw it. The mirror must say so rather than implying delivery. + it('marks the reply undelivered in shadow mode', async () => { + mockMirrorConfig.mode = 'live'; + mockMirrorConfig.channelId = 'C0MIRROR'; + const originalShadow = process.env.SHADOW_MODE; + try { + process.env.SHADOW_MODE = 'true'; + + await handleAiResponse({ ticketId: 'tkt-1', source: 'discord' }, makeContext()); + + const jobs = mirrorJobs(); + expect(jobs).toHaveLength(1); + expect(jobs[0].payload).toEqual( + expect.objectContaining({ kind: 'reply', delivered: false }), + ); + } finally { + restoreShadowMode(originalShadow); + } + }); + + // A suppressed run posts safe replacement copy, not the draft stored on + // the Message the mirror renders — so the draft still reached nobody. + it('marks a suppressed reply undelivered even though a post succeeded', async () => { + mockMirrorConfig.mode = 'live'; + mockMirrorConfig.channelId = 'C0MIRROR'; + mockGenerateSupportResponse.mockResolvedValue(suppressedResult); + + await handleAiResponse({ ticketId: 'tkt-1', source: 'discord' }, makeContext()); + + const jobs = mirrorJobs(); + expect(jobs).toHaveLength(1); + expect(jobs[0].payload).toEqual( + expect.objectContaining({ kind: 'reply', delivered: false }), + ); + }); + }); }); diff --git a/packages/outpost/queue/src/__tests__/queue.test.ts b/packages/outpost/queue/src/__tests__/queue.test.ts index 001bf882..5546ca03 100644 --- a/packages/outpost/queue/src/__tests__/queue.test.ts +++ b/packages/outpost/queue/src/__tests__/queue.test.ts @@ -539,8 +539,8 @@ describe('JobType enum', () => { expect(JobType.ONBOARDING_DIGEST).toBe('ONBOARDING_DIGEST'); }); - it('has exactly 10 job types', () => { + it('has exactly 11 job types', () => { const values = Object.values(JobType); - expect(values).toHaveLength(10); + expect(values).toHaveLength(11); }); }); diff --git a/packages/outpost/queue/src/handlers/__tests__/slack-mirror.test.ts b/packages/outpost/queue/src/handlers/__tests__/slack-mirror.test.ts new file mode 100644 index 00000000..87ed281c --- /dev/null +++ b/packages/outpost/queue/src/handlers/__tests__/slack-mirror.test.ts @@ -0,0 +1,255 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +const mockTicketFindUnique = vi.fn(); +const mockMessageFindUnique = vi.fn(); +const mockLinkFindUnique = vi.fn(); +const mockLinkCreate = vi.fn(); + +vi.mock('@copilotkit/outpost/db', () => ({ + prisma: { + ticket: { findUnique: (...a: unknown[]) => mockTicketFindUnique(...a) }, + message: { findUnique: (...a: unknown[]) => mockMessageFindUnique(...a) }, + ticketExternalLink: { + findUnique: (...a: unknown[]) => mockLinkFindUnique(...a), + create: (...a: unknown[]) => mockLinkCreate(...a), + }, + }, +})); + +import { handleSlackMirror, SLACK_MIRROR_PLUGIN } from '../slack-mirror.js'; +import { readSlackMirrorConfig, isSlackMirrorEnabled } from '@copilotkit/outpost/shared/platforms'; +import type { SlackMirrorConfig } from '@copilotkit/outpost/shared/platforms'; + +const context = { reportProgress: vi.fn().mockResolvedValue(undefined), jobId: 'job-1' }; + +const TICKET = { + id: 'tkt-1', + displayId: 'OUT-101', + title: 'Sidebar crashes on mount', + description: 'Repro: render CopilotSidebar with no props.', + source: 'GITHUB', + sourceUrl: 'https://github.com/CopilotKit/CopilotKit/issues/42', +}; + +const liveConfig: SlackMirrorConfig = { mode: 'live', channelId: 'C0MIRROR', token: 'xoxb-1' }; + +function makePoster(ts = '1712345678.000100') { + const postMessage = vi.fn().mockResolvedValue({ ts }); + return { poster: { postMessage }, postMessage }; +} + +describe('readSlackMirrorConfig', () => { + it('defaults to off when the mode is unset', () => { + expect(readSlackMirrorConfig({}).mode).toBe('off'); + }); + + it('fails closed on an unrecognized mode rather than posting', () => { + expect(readSlackMirrorConfig({ SLACK_MIRROR_MODE: 'on' }).mode).toBe('off'); + expect(readSlackMirrorConfig({ SLACK_MIRROR_MODE: 'LIVE' }).mode).toBe('live'); + }); + + it('treats a blank channel ID as unset', () => { + expect(readSlackMirrorConfig({ SLACK_MIRROR_CHANNEL_ID: ' ' }).channelId).toBeNull(); + }); +}); + +describe('isSlackMirrorEnabled', () => { + it('is disabled when off, even with a channel configured', () => { + expect(isSlackMirrorEnabled({ mode: 'off', channelId: 'C1', token: 'x' })).toBe(false); + }); + + it('is disabled when live but no channel is configured', () => { + expect(isSlackMirrorEnabled({ mode: 'live', channelId: null, token: 'x' })).toBe(false); + }); + + it('is enabled in shadow without a token, since shadow never posts', () => { + expect(isSlackMirrorEnabled({ mode: 'shadow', channelId: 'C1', token: null })).toBe(true); + }); +}); + +describe('handleSlackMirror', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockTicketFindUnique.mockResolvedValue(TICKET); + mockLinkFindUnique.mockResolvedValue(null); + mockLinkCreate.mockResolvedValue({}); + }); + + it('posts nothing and touches no DB when the mirror is off', async () => { + const { poster, postMessage } = makePoster(); + const result = await handleSlackMirror({ ticketId: 'tkt-1', kind: 'ticket' }, context, { + config: { mode: 'off', channelId: 'C0MIRROR', token: 'xoxb-1' }, + poster, + }); + + expect(result).toEqual({ success: true, data: { skipped: 'mirror-disabled' } }); + expect(postMessage).not.toHaveBeenCalled(); + expect(mockTicketFindUnique).not.toHaveBeenCalled(); + }); + + it('opens a thread and records the link under plugin slack', async () => { + const { poster, postMessage } = makePoster('1712345678.000100'); + + const result = await handleSlackMirror({ ticketId: 'tkt-1', kind: 'ticket' }, context, { + config: liveConfig, + poster, + }); + + expect(result.success).toBe(true); + expect(postMessage).toHaveBeenCalledTimes(1); + const posted = postMessage.mock.calls[0][0]; + expect(posted.channel).toBe('C0MIRROR'); + expect(posted.thread_ts).toBeUndefined(); + expect(posted.text).toContain('OUT-101'); + expect(posted.text).toContain('Sidebar crashes on mount'); + expect(posted.text).toContain('https://github.com/CopilotKit/CopilotKit/issues/42'); + + expect(mockLinkCreate).toHaveBeenCalledTimes(1); + expect(mockLinkCreate.mock.calls[0][0].data).toMatchObject({ + ticketId: 'tkt-1', + plugin: SLACK_MIRROR_PLUGIN, + externalId: 'C0MIRROR:1712345678.000100', + }); + }); + + it('does not open a second thread for an already-mirrored ticket', async () => { + mockLinkFindUnique.mockResolvedValue({ externalId: 'C0MIRROR:111.222' }); + const { poster, postMessage } = makePoster(); + + const result = await handleSlackMirror({ ticketId: 'tkt-1', kind: 'ticket' }, context, { + config: liveConfig, + poster, + }); + + expect(result).toEqual({ success: true, data: { skipped: 'already-mirrored' } }); + expect(postMessage).not.toHaveBeenCalled(); + expect(mockLinkCreate).not.toHaveBeenCalled(); + }); + + it('threads a reply under the existing ticket thread', async () => { + mockLinkFindUnique.mockResolvedValue({ externalId: 'C0MIRROR:111.222' }); + mockMessageFindUnique.mockResolvedValue({ + id: 'msg-9', + author: 'octocat (12345)', + content: 'Still broken on 1.10.2', + isAiGenerated: false, + }); + const { poster, postMessage } = makePoster(); + + const result = await handleSlackMirror( + { ticketId: 'tkt-1', kind: 'reply', messageId: 'msg-9' }, + context, + { config: liveConfig, poster }, + ); + + expect(result.success).toBe(true); + expect(postMessage).toHaveBeenCalledTimes(1); + const posted = postMessage.mock.calls[0][0]; + expect(posted.thread_ts).toBe('111.222'); + expect(posted.text).toContain('Still broken on 1.10.2'); + expect(mockLinkCreate).not.toHaveBeenCalled(); + }); + + it('opens the thread first when a reply arrives with no thread yet', async () => { + mockLinkFindUnique.mockResolvedValue(null); + mockMessageFindUnique.mockResolvedValue({ + id: 'msg-9', + author: 'octocat (12345)', + content: 'Still broken', + isAiGenerated: false, + }); + const { poster, postMessage } = makePoster('999.111'); + + const result = await handleSlackMirror( + { ticketId: 'tkt-1', kind: 'reply', messageId: 'msg-9' }, + context, + { config: liveConfig, poster }, + ); + + expect(result.success).toBe(true); + expect(postMessage).toHaveBeenCalledTimes(2); + expect(postMessage.mock.calls[0][0].thread_ts).toBeUndefined(); + expect(postMessage.mock.calls[1][0].thread_ts).toBe('999.111'); + expect(mockLinkCreate).toHaveBeenCalledTimes(1); + }); + + it('marks an undelivered AI reply instead of implying the reporter saw it', async () => { + mockLinkFindUnique.mockResolvedValue({ externalId: 'C0MIRROR:111.222' }); + mockMessageFindUnique.mockResolvedValue({ + id: 'msg-ai', + author: 'outpost-ai', + content: '## Bug Confirmed…', + isAiGenerated: true, + }); + const { poster, postMessage } = makePoster(); + + await handleSlackMirror( + { ticketId: 'tkt-1', kind: 'reply', messageId: 'msg-ai', delivered: false }, + context, + { config: liveConfig, poster }, + ); + + expect(postMessage.mock.calls[0][0].text).toContain('not delivered to the reporter'); + }); + + it('does not mark a delivered AI reply', async () => { + mockLinkFindUnique.mockResolvedValue({ externalId: 'C0MIRROR:111.222' }); + mockMessageFindUnique.mockResolvedValue({ + id: 'msg-ai', + author: 'outpost-ai', + content: 'Here is the fix', + isAiGenerated: true, + }); + const { poster, postMessage } = makePoster(); + + await handleSlackMirror( + { ticketId: 'tkt-1', kind: 'reply', messageId: 'msg-ai', delivered: true }, + context, + { config: liveConfig, poster }, + ); + + expect(postMessage.mock.calls[0][0].text).not.toContain('not delivered'); + }); + + it('shadow mode posts nothing and records no link', async () => { + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + const { poster, postMessage } = makePoster(); + + const result = await handleSlackMirror({ ticketId: 'tkt-1', kind: 'ticket' }, context, { + config: { mode: 'shadow', channelId: 'C0MIRROR', token: null }, + poster, + }); + + expect(result.success).toBe(true); + expect(postMessage).not.toHaveBeenCalled(); + expect(mockLinkCreate).not.toHaveBeenCalled(); + expect(logSpy.mock.calls.flat().join(' ')).toContain('would open thread'); + logSpy.mockRestore(); + }); + + it('fails the job when the ticket is gone', async () => { + mockTicketFindUnique.mockResolvedValue(null); + const { poster } = makePoster(); + + const result = await handleSlackMirror({ ticketId: 'missing', kind: 'ticket' }, context, { + config: liveConfig, + poster, + }); + + expect(result.success).toBe(false); + expect(result.error).toContain('missing'); + }); + + it('fails a reply job that carries no messageId', async () => { + mockLinkFindUnique.mockResolvedValue({ externalId: 'C0MIRROR:111.222' }); + const { poster } = makePoster(); + + const result = await handleSlackMirror({ ticketId: 'tkt-1', kind: 'reply' }, context, { + config: liveConfig, + poster, + }); + + expect(result.success).toBe(false); + expect(result.error).toContain('messageId'); + }); +}); diff --git a/packages/outpost/queue/src/handlers/ai-response.ts b/packages/outpost/queue/src/handlers/ai-response.ts index e7d44548..562dea27 100644 --- a/packages/outpost/queue/src/handlers/ai-response.ts +++ b/packages/outpost/queue/src/handlers/ai-response.ts @@ -20,7 +20,12 @@ import { prisma } from '@copilotkit/outpost/db'; import { AIPipeline } from '@copilotkit/outpost/ai'; import { AI_CONFIDENCE } from '@copilotkit/outpost/shared'; import type { PlatformTarget, TicketSource } from '@copilotkit/outpost/shared'; -import { hasAdapter, getAdapter } from '@copilotkit/outpost/shared/platforms'; +import { + hasAdapter, + getAdapter, + readSlackMirrorConfig, + isSlackMirrorEnabled, +} from '@copilotkit/outpost/shared/platforms'; import { createJob } from '../create-job.js'; import { getFeedbackCalibration } from '../feedback-calibration.js'; import { JobType } from '../types.js'; @@ -189,6 +194,10 @@ export async function handleAiResponse( // suggestedResponse holds the publishable text bots pick up), and step 6 // below escalates on suppression regardless of score. const ticketSource = ticket.source as TicketSource; + // Tracks whether the reporter actually received `aiMessage.content`. + // The Slack mirror labels its post with this, so an internal reader is + // never told the community saw a draft that was withheld or only logged. + let draftReachedReporter = false; if (pipelineResult.suppressed) { console.warn( `[AI Response] Ungrounded draft withheld for ticket ${ticketId} — ` + @@ -252,6 +261,10 @@ export async function handleAiResponse( data: { externalCommentId }, }); } + // A suppressed run posts safe replacement copy, not the + // draft stored on aiMessage — so the draft itself still + // never reached anyone. + draftReachedReporter = !pipelineResult.suppressed; console.log( `[AI Response] Posted response to ${ticket.source} for ticket ${ticketId}`, ); @@ -266,6 +279,26 @@ export async function handleAiResponse( await context.reportProgress(85); + // 5c. Mirror the AI reply into the internal Slack thread for this + // ticket. Enqueued regardless of whether the draft was delivered — an + // answer the community never saw is precisely what the team needs to + // notice — but labelled with which of those happened. + if (isSlackMirrorEnabled(readSlackMirrorConfig())) { + try { + await createJob(JobType.SLACK_MIRROR, { + ticketId: ticket.id, + kind: 'reply', + messageId: aiMessage.id, + delivered: draftReachedReporter, + }); + } catch (error) { + console.error( + `[AI Response] Failed to enqueue Slack mirror for ticket ${ticketId}:`, + error instanceof Error ? error.message : String(error), + ); + } + } + // 6. Enqueue ESCALATION when confidence is below threshold, or when the // response was withheld — nothing reached the reporter in that case, so a // human has to pick it up regardless of what the score says. diff --git a/packages/outpost/queue/src/handlers/slack-mirror.ts b/packages/outpost/queue/src/handlers/slack-mirror.ts new file mode 100644 index 00000000..2e855fc1 --- /dev/null +++ b/packages/outpost/queue/src/handlers/slack-mirror.ts @@ -0,0 +1,211 @@ +/** + * Slack ticket mirror job handler. + * + * Mirrors every ticket from GitHub and Discord into one internal Slack channel: + * the ticket opens a thread, and community follow-ups plus the AI's reply post + * as threaded replies underneath it. One Slack thread is the whole life of one + * ticket. + * + * Read-only in v1 — replying inside Slack does not post back to the source. + * Thread identity lives in TicketExternalLink (plugin `slack`, externalId + * `channelId:ts`), reusing the same table the Linear and GitHub links use. + */ + +import { WebClient } from '@slack/web-api'; +import { prisma } from '@copilotkit/outpost/db'; +import { + readSlackMirrorConfig, + isSlackMirrorEnabled, + buildPermalink, + type SlackMirrorConfig, +} from '@copilotkit/outpost/shared/platforms'; +import type { SlackMirrorPayload, JobResult, JobHandlerContext } from '../types.js'; + +/** TicketExternalLink.plugin value owned by the mirror. */ +export const SLACK_MIRROR_PLUGIN = 'slack'; + +/** Slack hard-caps a single text block; leave room for our framing. */ +const MAX_MIRROR_TEXT = 2800; + +/** Minimal Slack surface the handler needs — lets tests inject a fake. */ +export interface SlackPoster { + postMessage(args: { + channel: string; + text: string; + thread_ts?: string; + }): Promise<{ ts?: string }>; +} + +export interface SlackMirrorDeps { + prisma: typeof prisma; + config: SlackMirrorConfig; + /** Omit to build a real WebClient from the config token. */ + poster?: SlackPoster; +} + +function truncate(value: string, max: number): string { + if (value.length <= max) return value; + return `${value.slice(0, max - 1)}…`; +} + +function buildPoster(config: SlackMirrorConfig): SlackPoster { + if (!config.token) { + throw new Error( + 'SLACK_BOT_TOKEN is required to post the ticket mirror. It must be set on the ' + + 'worker service (the mirror runs there, not in the Slack bot) and carry chat:write.', + ); + } + const client = new WebClient(config.token); + return { + async postMessage(args) { + const result = await client.chat.postMessage(args); + return { ts: result.ts as string | undefined }; + }, + }; +} + +/** Thread-opening post: what the ticket is, who reported it, where it came from. */ +function formatTicketPost(ticket: { + displayId: string; + title: string; + source: string; + sourceUrl: string | null; + description: string | null; +}): string { + const header = `*[${ticket.displayId}] ${truncate(ticket.title, 200)}*`; + const origin = ticket.sourceUrl + ? `${ticket.source} · <${ticket.sourceUrl}|view original>` + : String(ticket.source); + const body = ticket.description ? truncate(ticket.description, MAX_MIRROR_TEXT) : '_no body_'; + return `${header}\n${origin}\n\n${body}`; +} + +/** + * Threaded reply post. + * + * An AI message that was withheld or shadow-logged is labelled as such. The + * mirror must not imply the reporter saw something they never saw — that is + * exactly the divergence #148 describes between what the DB records and what + * was actually published. + */ +function formatReplyPost( + message: { author: string; content: string; isAiGenerated: boolean }, + delivered: boolean | undefined, +): string { + const who = message.isAiGenerated ? `🤖 ${message.author}` : message.author; + const undelivered = + message.isAiGenerated && delivered === false + ? '\n_⚠️ not delivered to the reporter — withheld or shadow mode_' + : ''; + return `*${who}*${undelivered}\n${truncate(message.content, MAX_MIRROR_TEXT)}`; +} + +/** + * Handle a SLACK_MIRROR job. + * + * Ordering note: a `reply` whose thread does not exist yet opens the thread + * first. Jobs can land out of order, and the mirror can be switched on partway + * through a live conversation; neither should drop messages on the floor. + */ +export async function handleSlackMirror( + payload: SlackMirrorPayload, + context: JobHandlerContext, + deps?: Partial, +): Promise { + const db = deps?.prisma ?? prisma; + const config = deps?.config ?? readSlackMirrorConfig(); + + if (!isSlackMirrorEnabled(config)) { + return { success: true, data: { skipped: 'mirror-disabled' } }; + } + // isSlackMirrorEnabled guarantees this, but the compiler does not know it. + const channelId = config.channelId!; + + await context.reportProgress(10); + + const ticket = await db.ticket.findUnique({ where: { id: payload.ticketId } }); + if (!ticket) { + return { success: false, error: `Ticket ${payload.ticketId} not found` }; + } + + const existingLink = await db.ticketExternalLink.findUnique({ + where: { ticketId_plugin: { ticketId: ticket.id, plugin: SLACK_MIRROR_PLUGIN } }, + }); + + // Already mirrored — opening a second thread for the same ticket would + // split its history across two places. + if (payload.kind === 'ticket' && existingLink) { + return { success: true, data: { skipped: 'already-mirrored' } }; + } + + const isShadow = config.mode === 'shadow'; + const poster = isShadow ? null : (deps?.poster ?? buildPoster(config)); + + await context.reportProgress(40); + + // ── Open the thread when it does not exist yet ─────────────────────────── + let threadTs = existingLink ? parseThreadTs(existingLink.externalId) : null; + + if (!threadTs) { + const text = formatTicketPost(ticket); + if (isShadow) { + console.log( + `[Slack Mirror] shadow — would open thread in ${channelId} for ${ticket.displayId}:\n${text}`, + ); + } else { + const result = await poster!.postMessage({ channel: channelId, text }); + if (!result.ts) { + return { + success: false, + error: `Slack accepted the mirror post for ${ticket.displayId} but returned no ts`, + }; + } + threadTs = result.ts; + await db.ticketExternalLink.create({ + data: { + ticketId: ticket.id, + plugin: SLACK_MIRROR_PLUGIN, + externalId: `${channelId}:${threadTs}`, + externalUrl: buildPermalink(channelId, threadTs), + }, + }); + } + } + + await context.reportProgress(70); + + // ── Post the reply underneath it ───────────────────────────────────────── + if (payload.kind === 'reply') { + if (!payload.messageId) { + return { success: false, error: 'SLACK_MIRROR reply job carried no messageId' }; + } + + const message = await db.message.findUnique({ where: { id: payload.messageId } }); + if (!message) { + return { success: false, error: `Message ${payload.messageId} not found` }; + } + + const text = formatReplyPost(message, payload.delivered); + if (isShadow) { + console.log( + `[Slack Mirror] shadow — would reply in ${channelId} on ${ticket.displayId}:\n${text}`, + ); + } else { + await poster!.postMessage({ channel: channelId, text, thread_ts: threadTs! }); + } + } + + await context.reportProgress(100); + + return { + success: true, + data: { mode: config.mode, kind: payload.kind, ticketId: ticket.id }, + }; +} + +/** externalId is stored as `channelId:ts`; the ts is everything after the colon. */ +function parseThreadTs(externalId: string): string | null { + const idx = externalId.indexOf(':'); + if (idx === -1) return null; + return externalId.slice(idx + 1) || null; +} diff --git a/packages/outpost/queue/src/index.ts b/packages/outpost/queue/src/index.ts index 0113e4b2..959d8a38 100644 --- a/packages/outpost/queue/src/index.ts +++ b/packages/outpost/queue/src/index.ts @@ -11,6 +11,8 @@ export { handleHubSpotSync } from './handlers/hubspot-sync.js'; export { createTrackerSyncHandler } from './handlers/tracker-sync.js'; export { handleJobCleanup } from './handlers/job-cleanup.js'; export { handleGithubReactionPoll } from './handlers/github-reaction-poll.js'; +export { handleSlackMirror, SLACK_MIRROR_PLUGIN } from './handlers/slack-mirror.js'; +export type { SlackPoster, SlackMirrorDeps } from './handlers/slack-mirror.js'; export { getFeedbackCalibration } from './feedback-calibration.js'; export type { FeedbackCountClient } from './feedback-calibration.js'; export * from './types.js'; diff --git a/packages/outpost/queue/src/types.ts b/packages/outpost/queue/src/types.ts index fc9ce17a..dbf8f1ac 100644 --- a/packages/outpost/queue/src/types.ts +++ b/packages/outpost/queue/src/types.ts @@ -30,6 +30,8 @@ export enum JobType { JOB_CLEANUP = 'JOB_CLEANUP', /** Poll GitHub reactions on AI-authored comments for feedback signal */ GITHUB_REACTION_POLL = 'GITHUB_REACTION_POLL', + /** Mirror a ticket (or a reply on it) into the internal Slack channel */ + SLACK_MIRROR = 'SLACK_MIRROR', } // ─── Payload Shapes ───────────────────────────────────────────────────────── @@ -84,6 +86,32 @@ export type JobCleanupPayload = Record; /** No payload needed — runs against all pending-feedback AI messages. */ export type GithubReactionPollPayload = Record; +/** + * What kind of Slack mirror post this job should make. + * + * `ticket` opens the thread; `reply` posts underneath the thread the `ticket` + * job created. A `reply` that finds no thread opens one first, so an enable + * mid-conversation does not silently drop every later message. + */ +export type SlackMirrorKind = 'ticket' | 'reply'; + +export interface SlackMirrorPayload { + /** The Outpost ticket ID being mirrored */ + ticketId: string; + /** Whether this opens the thread or replies inside it */ + kind: SlackMirrorKind; + /** The Message row this post reflects; omit for the thread-opening post */ + messageId?: string; + /** + * For AI replies: whether the answer actually reached the reporter. + * + * Shadow mode and the groundedness gate both produce an AI Message row that + * was never delivered. The mirror labels those explicitly rather than + * implying the community saw them — the same failure #148 describes. + */ + delivered?: boolean; +} + /** Map from JobType to its specific payload shape */ export interface JobPayload { [JobType.AI_RESPONSE]: AiResponsePayload; @@ -96,6 +124,7 @@ export interface JobPayload { [JobType.TRACKER_SYNC]: TrackerSyncPayload; [JobType.JOB_CLEANUP]: JobCleanupPayload; [JobType.GITHUB_REACTION_POLL]: GithubReactionPollPayload; + [JobType.SLACK_MIRROR]: SlackMirrorPayload; } // ─── Job Results ──────────────────────────────────────────────────────────── diff --git a/packages/outpost/shared/src/__tests__/platforms-inbound.test.ts b/packages/outpost/shared/src/__tests__/platforms-inbound.test.ts index bd5c0698..ff1b4b99 100644 --- a/packages/outpost/shared/src/__tests__/platforms-inbound.test.ts +++ b/packages/outpost/shared/src/__tests__/platforms-inbound.test.ts @@ -609,4 +609,82 @@ describe('InboundHandler', () => { expect(createJob).toHaveBeenCalledWith('CUSTOM_AI_JOB', expect.anything()); }); }); + // ── Slack ticket mirror ────────────────────────────────────────── + + describe('Slack ticket mirror', () => { + it('enqueues a thread-opening mirror job for a new ticket', async () => { + const handler = new InboundHandler({ prisma, createJob, mirrorToSlack: true }); + + await handler.handle(makeInboundMessage()); + + expect(createJob).toHaveBeenCalledWith('SLACK_MIRROR', { + ticketId: 'ticket-1', + source: 'discord', + kind: 'ticket', + }); + }); + + it('enqueues nothing when the mirror is disabled', async () => { + const handler = new InboundHandler({ prisma, createJob, mirrorToSlack: false }); + + await handler.handle(makeInboundMessage()); + + const types = (createJob as ReturnType).mock.calls.map((c) => c[0]); + expect(types).not.toContain('SLACK_MIRROR'); + }); + + it('enqueues a threaded reply job carrying the message id', async () => { + prisma.ticket.findFirst = vi.fn().mockResolvedValue({ + id: 'ticket-1', + displayId: 'TKT-ABCDEF12', + status: 'OPEN', + sourceId: 'thread-123', + channel: 'channel-1', + source: 'DISCORD', + }); + const handler = new InboundHandler({ prisma, createJob, mirrorToSlack: true }); + + await handler.handle(makeInboundMessage({ isThreadStart: false })); + + expect(createJob).toHaveBeenCalledWith('SLACK_MIRROR', { + ticketId: 'ticket-1', + source: 'discord', + kind: 'reply', + messageId: 'msg-1', + }); + }); + + // A Slack-sourced ticket mirrored into a monitored Slack channel would + // arrive back as inbound, open a ticket, mirror again, and loop. + it('never mirrors a Slack-sourced ticket back into Slack', async () => { + const handler = new InboundHandler({ prisma, createJob, mirrorToSlack: true }); + + await handler.handle( + makeInboundMessage({ source: TicketSource.SLACK, channelId: 'C0MIRROR' }), + ); + + const types = (createJob as ReturnType).mock.calls.map((c) => c[0]); + expect(types).not.toContain('SLACK_MIRROR'); + }); + + it('still creates the ticket when enqueueing the mirror job fails', async () => { + const failing = vi.fn().mockImplementation((type: string) => { + if (type === 'SLACK_MIRROR') return Promise.reject(new Error('queue down')); + return Promise.resolve('job-1'); + }); + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const handler = new InboundHandler({ + prisma, + createJob: failing as unknown as CreateJobFn, + mirrorToSlack: true, + }); + + const result = await handler.handle(makeInboundMessage()); + + expect(result.ticketId).toBe('ticket-1'); + expect(result.isNewTicket).toBe(true); + expect(errorSpy).toHaveBeenCalled(); + errorSpy.mockRestore(); + }); + }); }); diff --git a/packages/outpost/shared/src/platforms/inbound.ts b/packages/outpost/shared/src/platforms/inbound.ts index 8d51dc6d..654c83ec 100644 --- a/packages/outpost/shared/src/platforms/inbound.ts +++ b/packages/outpost/shared/src/platforms/inbound.ts @@ -13,6 +13,7 @@ import type { InboundMessage, InboundResult, TicketRef } from './types.js'; import { generateTicketId, truncate } from '../utils.js'; import { TicketSource } from '../types.js'; +import { readSlackMirrorConfig, isSlackMirrorEnabled } from './slack-mirror-config.js'; /** * Prisma client interface — the subset of PrismaClient we actually call. @@ -52,7 +53,7 @@ export interface PrismaLike { */ export type CreateJobFn = ( type: string, - payload: { ticketId: string; threadId?: string; source: string }, + payload: { ticketId: string; threadId?: string; source: string; [key: string]: unknown }, ) => Promise; /** @@ -94,6 +95,16 @@ export interface InboundHandlerConfig { createJob: CreateJobFn; /** Job type string for AI_RESPONSE (default: 'AI_RESPONSE') */ aiResponseJobType?: string; + /** Job type string for the Slack ticket mirror (default: 'SLACK_MIRROR') */ + slackMirrorJobType?: string; + /** + * Whether to enqueue Slack mirror jobs. + * + * Defaults to the environment's mirror configuration, so a deployment with + * `SLACK_MIRROR_MODE=off` (or no channel configured) never queues work that + * the handler would only discard. Tests pass this explicitly. + */ + mirrorToSlack?: boolean; } /** @@ -109,11 +120,51 @@ export class InboundHandler { private readonly prisma: PrismaLike; private readonly createJob: CreateJobFn; private readonly aiResponseJobType: string; + private readonly slackMirrorJobType: string; + private readonly mirrorToSlack: boolean; constructor(config: InboundHandlerConfig) { this.prisma = config.prisma; this.createJob = config.createJob; this.aiResponseJobType = config.aiResponseJobType ?? 'AI_RESPONSE'; + this.slackMirrorJobType = config.slackMirrorJobType ?? 'SLACK_MIRROR'; + this.mirrorToSlack = + config.mirrorToSlack ?? isSlackMirrorEnabled(readSlackMirrorConfig()); + } + + /** + * Enqueue a Slack mirror job, swallowing any failure. + * + * The mirror is an internal convenience view. A queue hiccup while + * mirroring must never take down ticket creation for a real reporter, so + * this logs and moves on rather than propagating. + */ + private async enqueueSlackMirror( + ticketId: string, + source: TicketSource, + payload: { kind: 'ticket' | 'reply'; messageId?: string }, + ): Promise { + if (!this.mirrorToSlack) return; + + // Never mirror a Slack-sourced ticket back into Slack. If the mirror + // channel is one the Slack bot monitors, the mirror post would arrive + // as a new inbound message, open a ticket, mirror that, and loop. The + // mirror exists to bring GitHub and Discord into Slack; Slack tickets + // are already there. + if (source === TicketSource.SLACK) return; + + try { + await this.createJob(this.slackMirrorJobType, { + ticketId, + source: toPlatformTarget(source), + ...payload, + }); + } catch (err) { + console.error( + `[InboundHandler] Failed to enqueue Slack mirror (${payload.kind}) for ticket ${ticketId}:`, + err instanceof Error ? err.message : String(err), + ); + } } /** @@ -198,6 +249,11 @@ export class InboundHandler { aiJobEnqueued = true; } + // Mirror the new ticket into the internal Slack channel. The mirror's + // thread-opening post carries the ticket body, so the first Message + // does not also need a reply job. + await this.enqueueSlackMirror(ticket.id, message.source, { kind: 'ticket' }); + return { ticketId: ticket.id, displayId, @@ -269,6 +325,14 @@ export class InboundHandler { } } + // Mirror the follow-up under the ticket's existing Slack thread. Team + // replies mirror too — the thread is meant to be the whole life of the + // ticket, and a team answer is the most useful part of it. + await this.enqueueSlackMirror(ticket.id, message.source, { + kind: 'reply', + messageId: msg.id, + }); + return { ticketId: ticket.id, displayId: ticket.displayId, diff --git a/packages/outpost/shared/src/platforms/index.ts b/packages/outpost/shared/src/platforms/index.ts index e0342f52..6d926ab0 100644 --- a/packages/outpost/shared/src/platforms/index.ts +++ b/packages/outpost/shared/src/platforms/index.ts @@ -48,6 +48,11 @@ export { GitHubAdapter as PlatformGitHubAdapter, GitHubPlatformAdapter } from '. export type { GitHubAdapterConfig as PlatformGitHubAdapterConfig, GitHubOctokitLike } from './github.js'; export { SlackAdapter as PlatformSlackAdapter, SlackAdapter, buildPermalink } from './slack.js'; + +// Slack ticket mirror — flag semantics shared by the producers (inbound +// handler, AI response handler) and the consumer (SLACK_MIRROR job handler). +export { readSlackMirrorConfig, isSlackMirrorEnabled } from './slack-mirror-config.js'; +export type { SlackMirrorConfig, SlackMirrorMode } from './slack-mirror-config.js'; export type { SlackAdapterConfig as PlatformSlackAdapterConfig, SlackAdapterConfig, SlackMessageEvent } from './slack.js'; export { TeamsAdapter as PlatformTeamsAdapter, TeamsAdapter } from './teams.js'; diff --git a/packages/outpost/shared/src/platforms/slack-mirror-config.ts b/packages/outpost/shared/src/platforms/slack-mirror-config.ts new file mode 100644 index 00000000..12fb0112 --- /dev/null +++ b/packages/outpost/shared/src/platforms/slack-mirror-config.ts @@ -0,0 +1,56 @@ +/** + * Configuration for the Slack ticket mirror. + * + * The mirror posts every ticket from GitHub and Discord into one internal Slack + * channel, with follow-ups threaded underneath, so a single Slack thread is the + * whole life of one ticket. + * + * `SLACK_MIRROR_MODE` is deliberately INDEPENDENT of `SHADOW_MODE`. That flag + * protects community surfaces (Discord, GitHub) where real reporters are + * watching; the mirror targets an internal team channel, so posting there from + * staging is intended rather than a violation of the standing shadow-mode rule. + * The mirror's own three-way flag is what keeps it inert until it is configured. + */ + +/** `off` no-ops · `shadow` logs what it would post · `live` posts to Slack. */ +export type SlackMirrorMode = 'off' | 'shadow' | 'live'; + +export interface SlackMirrorConfig { + mode: SlackMirrorMode; + /** Slack channel ID (e.g. C09AB2CD3EF) — an ID, never a channel name. */ + channelId: string | null; + /** Bot token; must carry chat:write and be set on the worker service. */ + token: string | null; +} + +/** + * Read the mirror configuration from the environment. + * + * Unset or unrecognized `SLACK_MIRROR_MODE` resolves to `off`: the feature + * ships inert, and a typo fails closed rather than posting unexpectedly. + */ +export function readSlackMirrorConfig(env: NodeJS.ProcessEnv = process.env): SlackMirrorConfig { + const raw = (env.SLACK_MIRROR_MODE ?? 'off').trim().toLowerCase(); + const mode: SlackMirrorMode = raw === 'live' || raw === 'shadow' ? raw : 'off'; + + return { + mode, + channelId: env.SLACK_MIRROR_CHANNEL_ID?.trim() || null, + token: env.SLACK_BOT_TOKEN?.trim() || null, + }; +} + +/** + * Whether the mirror should do anything at all for this process. + * + * Used by the producers (inbound handler, AI response handler) so a disabled + * mirror never enqueues jobs, and by the consumer as its first check. + * + * `shadow` counts as enabled — it exists to be exercised, and its whole value + * is producing log lines showing what a live run would post. Only the channel + * ID is required for that, not the token. + */ +export function isSlackMirrorEnabled(config: SlackMirrorConfig): boolean { + if (config.mode === 'off') return false; + return config.channelId !== null; +} From b58ff8958bf93b4dd134d2ac5a38f1bcbb33ee3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nathan=20=F0=9F=94=B6=20Tarbert?= <66887028+NathanTarbert@users.noreply.github.com> Date: Mon, 3 Aug 2026 10:21:36 -0400 Subject: [PATCH 2/9] fix(slack): make the ticket-external-link row the idempotent thread identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mirror read the link, posted to Slack, then created the link. Two jobs for one ticket (the ticket job and the reply job, at SLACK_MIRROR concurrency 2) could both read "no link", both post a root message, and the loser of @@unique([ticketId, plugin]) died on P2002 — leaving a duplicate Slack thread and a dead-lettered job. Any retry after a successful post did the same. Four related defects, one cause: the link row was not treated as the identity of the thread. - parseThreadTs -> parseThreadRef, returning {channelId, ts} with both halves required. Replies now post to the channel recorded in the link instead of whatever SLACK_MIRROR_CHANNEL_ID currently says, so changing the configured channel no longer detaches every existing ticket's replies. The channel half was being written and never read. - The create is wrapped in a P2002 catch (duck-typed on err.code, so the Prisma runtime stays out of this handler) that re-reads the row and threads under the winner's ts rather than opening a rival thread. - A row whose externalId cannot be parsed is repaired in place via update. It previously re-entered the open-thread path and hit the unique constraint on every attempt, forever. The already-mirrored short-circuit now requires a PARSEABLE link, so a malformed row reaches repair exactly once. - SLACK_MIRROR concurrency 2 -> 1, plus an explicit 60s jobTimeouts entry (a reply can make two postMessage calls through WebClient's rate-limit sleeps, and the 30s default could cut that off mid-flight and duplicate work). Call-site enumeration: - parseThreadTs: removed; sole caller was handleSlackMirror's thread lookup, now calling parseThreadRef. Zero remaining references (grep). - parseThreadRef: new; called only from handleSlackMirror. - SLACK_MIRROR_PLUGIN: unchanged; readers are this handler and the tests. - handleSlackMirror signature unchanged; worker registration at apps/worker/src/index.ts:90 still holds. - JobType.SLACK_MIRROR concurrency/timeout maps: read only by Worker's scheduler in packages/outpost/queue/src/worker.ts; both keys are optional and additive. Not fixed here, still open: message-level reply dedup. A duplicate reply job can still re-post the same message; it can no longer spawn a rival thread. That needs a per-message marker and is tracked separately. Tests: 4 new cases in the handler suite (P2002 recovery, malformed-link repair, reply routes to the link's channel when config differs, duplicate reply opens no thread). Red-green verified — 3 of the 4 fail before this change. Handler suite 20/20, packages/outpost 958 tests, typecheck clean across 10 packages. --- apps/worker/src/index.ts | 9 +- .../handlers/__tests__/slack-mirror.test.ts | 110 ++++++++++++++++++ .../queue/src/handlers/slack-mirror.ts | 108 +++++++++++++---- 3 files changed, 205 insertions(+), 22 deletions(-) diff --git a/apps/worker/src/index.ts b/apps/worker/src/index.ts index ff3f395b..fc3d355e 100644 --- a/apps/worker/src/index.ts +++ b/apps/worker/src/index.ts @@ -67,12 +67,19 @@ const worker = new Worker({ [JobType.TRACKER_SYNC]: 1, [JobType.JOB_CLEANUP]: 1, [JobType.GITHUB_REACTION_POLL]: 1, - [JobType.SLACK_MIRROR]: 2, + // 1, not 2: the ticket and reply jobs for one ticket race to claim the + // same TicketExternalLink row. The handler survives the race, but serial + // processing keeps one ticket's thread in one Slack thread by construction. + [JobType.SLACK_MIRROR]: 1, }, jobTimeouts: { [JobType.AI_RESPONSE]: 120_000, // 2 minutes — AI pipeline is slow [JobType.HUBSPOT_SYNC]: 300_000, // 5 minutes — full sync can be large [JobType.ACCOUNT_SCORING]: 300_000, // 5 minutes — many accounts + // 60s, above the 30s default: a reply that has to open its thread first + // makes two chat.postMessage calls, and WebClient sleeps through Slack's + // rate-limit retries. Timing out mid-post would re-post on the retry. + [JobType.SLACK_MIRROR]: 60_000, }, }); diff --git a/packages/outpost/queue/src/handlers/__tests__/slack-mirror.test.ts b/packages/outpost/queue/src/handlers/__tests__/slack-mirror.test.ts index 87ed281c..b9199bb0 100644 --- a/packages/outpost/queue/src/handlers/__tests__/slack-mirror.test.ts +++ b/packages/outpost/queue/src/handlers/__tests__/slack-mirror.test.ts @@ -4,6 +4,7 @@ const mockTicketFindUnique = vi.fn(); const mockMessageFindUnique = vi.fn(); const mockLinkFindUnique = vi.fn(); const mockLinkCreate = vi.fn(); +const mockLinkUpdate = vi.fn(); vi.mock('@copilotkit/outpost/db', () => ({ prisma: { @@ -12,6 +13,7 @@ vi.mock('@copilotkit/outpost/db', () => ({ ticketExternalLink: { findUnique: (...a: unknown[]) => mockLinkFindUnique(...a), create: (...a: unknown[]) => mockLinkCreate(...a), + update: (...a: unknown[]) => mockLinkUpdate(...a), }, }, })); @@ -73,6 +75,7 @@ describe('handleSlackMirror', () => { mockTicketFindUnique.mockResolvedValue(TICKET); mockLinkFindUnique.mockResolvedValue(null); mockLinkCreate.mockResolvedValue({}); + mockLinkUpdate.mockResolvedValue({}); }); it('posts nothing and touches no DB when the mirror is off', async () => { @@ -227,6 +230,113 @@ describe('handleSlackMirror', () => { logSpy.mockRestore(); }); + // ── Idempotent link lifecycle ──────────────────────────────────────────── + + it('loses the create race without opening a second thread and replies on the winner ts', async () => { + // Both jobs for this ticket read "no link", so both post a root message. + // The loser's create hits @@unique([ticketId, plugin]). + mockLinkFindUnique.mockResolvedValueOnce(null).mockResolvedValueOnce({ + externalId: 'C0MIRROR:winner.0001', + }); + mockLinkCreate.mockRejectedValue( + Object.assign(new Error('Unique constraint'), { + code: 'P2002', + }), + ); + mockMessageFindUnique.mockResolvedValue({ + id: 'msg-9', + author: 'octocat (12345)', + content: 'Still broken', + isAiGenerated: false, + }); + const { poster, postMessage } = makePoster('loser.0002'); + + const result = await handleSlackMirror( + { ticketId: 'tkt-1', kind: 'reply', messageId: 'msg-9' }, + context, + { config: liveConfig, poster }, + ); + + expect(result.success).toBe(true); + expect(mockLinkCreate).toHaveBeenCalledTimes(1); + expect(mockLinkUpdate).not.toHaveBeenCalled(); + // The reply threads under the winner's ts, not the loser's orphaned post. + const reply = postMessage.mock.calls.at(-1)![0]; + expect(reply.thread_ts).toBe('winner.0001'); + }); + + it('repairs a malformed link with an update instead of a second create', async () => { + mockLinkFindUnique.mockResolvedValue({ externalId: 'no-colon-here' }); + const { poster, postMessage } = makePoster('repair.0003'); + + const result = await handleSlackMirror({ ticketId: 'tkt-1', kind: 'ticket' }, context, { + config: liveConfig, + poster, + }); + + expect(result.success).toBe(true); + expect(postMessage).toHaveBeenCalledTimes(1); + expect(mockLinkCreate).not.toHaveBeenCalled(); + expect(mockLinkUpdate).toHaveBeenCalledTimes(1); + const update = mockLinkUpdate.mock.calls[0][0]; + expect(update.where).toEqual({ + ticketId_plugin: { ticketId: 'tkt-1', plugin: SLACK_MIRROR_PLUGIN }, + }); + expect(update.data).toMatchObject({ externalId: 'C0MIRROR:repair.0003' }); + }); + + it('replies in the channel recorded on the link, not the currently configured one', async () => { + mockLinkFindUnique.mockResolvedValue({ externalId: 'C0OLDCHAN:111.222' }); + mockMessageFindUnique.mockResolvedValue({ + id: 'msg-9', + author: 'octocat (12345)', + content: 'Still broken', + isAiGenerated: false, + }); + const { poster, postMessage } = makePoster(); + + const result = await handleSlackMirror( + { ticketId: 'tkt-1', kind: 'reply', messageId: 'msg-9' }, + context, + { config: { ...liveConfig, channelId: 'C0NEWCHAN' }, poster }, + ); + + expect(result.success).toBe(true); + expect(postMessage).toHaveBeenCalledTimes(1); + expect(postMessage.mock.calls[0][0]).toMatchObject({ + channel: 'C0OLDCHAN', + thread_ts: '111.222', + }); + }); + + it('a duplicate reply job opens no thread and writes no link', async () => { + mockLinkFindUnique.mockResolvedValue({ externalId: 'C0MIRROR:111.222' }); + mockMessageFindUnique.mockResolvedValue({ + id: 'msg-9', + author: 'octocat (12345)', + content: 'Still broken', + isAiGenerated: false, + }); + const { poster, postMessage } = makePoster(); + + for (let i = 0; i < 2; i++) { + const result = await handleSlackMirror( + { ticketId: 'tkt-1', kind: 'reply', messageId: 'msg-9' }, + context, + { config: liveConfig, poster }, + ); + expect(result.success).toBe(true); + } + + // Two reply posts, but never a root post and never a link write. + expect(postMessage).toHaveBeenCalledTimes(2); + for (const [args] of postMessage.mock.calls) { + expect(args.thread_ts).toBe('111.222'); + } + expect(mockLinkCreate).not.toHaveBeenCalled(); + expect(mockLinkUpdate).not.toHaveBeenCalled(); + }); + it('fails the job when the ticket is gone', async () => { mockTicketFindUnique.mockResolvedValue(null); const { poster } = makePoster(); diff --git a/packages/outpost/queue/src/handlers/slack-mirror.ts b/packages/outpost/queue/src/handlers/slack-mirror.ts index 2e855fc1..c3711baa 100644 --- a/packages/outpost/queue/src/handlers/slack-mirror.ts +++ b/packages/outpost/queue/src/handlers/slack-mirror.ts @@ -106,6 +106,12 @@ function formatReplyPost( * Ordering note: a `reply` whose thread does not exist yet opens the thread * first. Jobs can land out of order, and the mirror can be switched on partway * through a live conversation; neither should drop messages on the floor. + * + * Idempotency: the TicketExternalLink row — not the config — is the identity of + * the thread. Two jobs for one ticket can both read "no link" and both post a + * root message; the one that loses `@@unique([ticketId, plugin])` re-reads the + * row and threads under the winner's ts rather than opening a rival thread. A + * row whose externalId cannot be parsed is repaired in place, never duplicated. */ export async function handleSlackMirror( payload: SlackMirrorPayload, @@ -128,13 +134,16 @@ export async function handleSlackMirror( return { success: false, error: `Ticket ${payload.ticketId} not found` }; } - const existingLink = await db.ticketExternalLink.findUnique({ - where: { ticketId_plugin: { ticketId: ticket.id, plugin: SLACK_MIRROR_PLUGIN } }, - }); + const linkWhere = { + ticketId_plugin: { ticketId: ticket.id, plugin: SLACK_MIRROR_PLUGIN }, + }; + const existingLink = await db.ticketExternalLink.findUnique({ where: linkWhere }); + let thread = existingLink ? parseThreadRef(existingLink.externalId) : null; // Already mirrored — opening a second thread for the same ticket would - // split its history across two places. - if (payload.kind === 'ticket' && existingLink) { + // split its history across two places. A link we cannot parse does NOT + // count as mirrored: it falls through to the repair path below. + if (payload.kind === 'ticket' && thread) { return { success: true, data: { skipped: 'already-mirrored' } }; } @@ -144,9 +153,7 @@ export async function handleSlackMirror( await context.reportProgress(40); // ── Open the thread when it does not exist yet ─────────────────────────── - let threadTs = existingLink ? parseThreadTs(existingLink.externalId) : null; - - if (!threadTs) { + if (!thread) { const text = formatTicketPost(ticket); if (isShadow) { console.log( @@ -160,15 +167,45 @@ export async function handleSlackMirror( error: `Slack accepted the mirror post for ${ticket.displayId} but returned no ts`, }; } - threadTs = result.ts; - await db.ticketExternalLink.create({ - data: { - ticketId: ticket.id, - plugin: SLACK_MIRROR_PLUGIN, - externalId: `${channelId}:${threadTs}`, - externalUrl: buildPermalink(channelId, threadTs), - }, - }); + const opened = { channelId, ts: result.ts }; + const linkData = { + externalId: `${opened.channelId}:${opened.ts}`, + externalUrl: buildPermalink(opened.channelId, opened.ts), + }; + + if (existingLink) { + // The row exists but its externalId is unusable. Repair it so the + // thread we just opened becomes the ticket's identity — a second + // create would fail the unique constraint on every attempt. + await db.ticketExternalLink.update({ where: linkWhere, data: linkData }); + thread = opened; + } else { + try { + await db.ticketExternalLink.create({ + data: { + ticketId: ticket.id, + plugin: SLACK_MIRROR_PLUGIN, + ...linkData, + }, + }); + thread = opened; + } catch (err) { + if (!isUniqueViolation(err)) throw err; + // Another job for this ticket claimed the link first. Adopt its + // thread; our root post is an orphan, but every message from + // here on lands in the one thread the row points at. + const winner = await db.ticketExternalLink.findUnique({ where: linkWhere }); + const winnerThread = winner ? parseThreadRef(winner.externalId) : null; + if (winnerThread) { + thread = winnerThread; + } else { + // The winning row is itself unparseable — repair it rather + // than retry a create that can only fail again. + await db.ticketExternalLink.update({ where: linkWhere, data: linkData }); + thread = opened; + } + } + } } } @@ -191,7 +228,14 @@ export async function handleSlackMirror( `[Slack Mirror] shadow — would reply in ${channelId} on ${ticket.displayId}:\n${text}`, ); } else { - await poster!.postMessage({ channel: channelId, text, thread_ts: threadTs! }); + // Post into the channel the link records, not the currently configured + // one: re-pointing SLACK_MIRROR_CHANNEL_ID must not orphan the replies + // of tickets whose thread already lives somewhere else. + await poster!.postMessage({ + channel: thread!.channelId, + text, + thread_ts: thread!.ts, + }); } } @@ -203,9 +247,31 @@ export async function handleSlackMirror( }; } -/** externalId is stored as `channelId:ts`; the ts is everything after the colon. */ -function parseThreadTs(externalId: string): string | null { +/** The Slack thread a ticket is mirrored into, as recorded on its link row. */ +interface SlackThreadRef { + channelId: string; + ts: string; +} + +/** + * externalId is stored as `channelId:ts`. Both halves are load-bearing — the + * channel is where replies go — so a row missing either half is unusable and + * must be repaired, not read. + */ +function parseThreadRef(externalId: string): SlackThreadRef | null { const idx = externalId.indexOf(':'); if (idx === -1) return null; - return externalId.slice(idx + 1) || null; + const channelId = externalId.slice(0, idx); + const ts = externalId.slice(idx + 1); + if (!channelId || !ts) return null; + return { channelId, ts }; +} + +/** + * Prisma's unique-constraint failure. Duck-typed on purpose: importing the + * Prisma runtime into a queue handler just to name an error class would drag + * the client into every consumer's bundle. + */ +function isUniqueViolation(err: unknown): boolean { + return typeof err === 'object' && err !== null && (err as { code?: unknown }).code === 'P2002'; } From 3091ac7397695cdfbdc59ab21b0b265868b5599e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nathan=20=F0=9F=94=B6=20Tarbert?= <66887028+NathanTarbert@users.noreply.github.com> Date: Mon, 3 Aug 2026 10:41:26 -0400 Subject: [PATCH 3/9] fix(slack): close the CR-round findings on the ticket mirror MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 1 of the CR loop returned 20 mandatory findings across 12 reviewers. This lands the rest of them (the link-idempotency lever went in as b58ff89). Code - Both producers now route through one allowlist. `isMirrorableSource` is the single place deciding what gets mirrored, and it is an ALLOWLIST (Discord, GitHub issues, GitHub discussions) rather than "anything but SLACK". The rule previously existed only in the inbound producer, so the AI-reply producer mirrored everything — a Slack-sourced ticket's AI reply opened a thread in the mirror channel, and the denylist silently pulled in TEAMS/EMAIL/WEB/MANUAL/ LINEAR tickets the feature was never specified for. - `live` with no SLACK_BOT_TOKEN now reads as DISABLED and logs why once, per process. It used to read as enabled, so every job reached buildPoster, threw, and burned five attempts into the dead-letter queue — one per ticket, forever. - Permanent Slack errors are classified and not retried: not_in_channel, channel_not_found, channel_is_archived, invalid_auth, account_inactive, missing_scope. Each failure message names the remedy. - Reply jobs are validated BEFORE anything is posted. A reply with no messageId used to reach the thread-opening post first, so every retry posted another root message to Slack; same for a messageId naming a row that no longer exists. - The undelivered label states the reason the producer recorded instead of guessing. `delivered: boolean` became `delivery: 'delivered' | 'shadow' | 'withheld' | 'post-failed' | 'no-adapter'`, set at the branch that knows. "withheld or shadow mode" was being printed for five distinct causes, which asserted a cause nobody established — the misreporting this label exists to prevent. An AI reply with no delivery status renders "unconfirmed", never delivered: unknown is not the same as fine. - `source` is now declared on SlackMirrorPayload and sent by both producers. It was riding CreateJobFn's index signature undeclared, which is how the two producers drifted into different payload shapes. - Mirror-enqueue failures log the error class and stack, so schema drift is not swallowed as "a queue hiccup". Queue (additive) - JobResult gains an optional `retryable`. A handler that sets it false is dead-lettered immediately instead of consuming every attempt. Omitting it — which every pre-existing handler does — preserves the old behavior exactly; both directions are covered by tests. Docs — each of these was a claim the diff made that the code contradicted - The loop rationale was wrong. `.env.example` and docs/deployment.md justified keeping the mirror channel unmonitored by saying the bot would read its own mirror posts and open a ticket per post. It cannot: the Slack bot drops events carrying bot_id (apps/slack-bot/src/events/message.ts). The advice stands as defense-in-depth and against duplicated context; the false mechanism is gone. - Scope is stated by naming isMirrorableSource rather than listing sources that drift. - The absolute "check SHADOW_MODE on any new outbound path" rule now carries the mirror's exception where the RULE is stated, with a gating table. - Documented that shadow needs no token, that any mode is inert without a channel ID, and — the one that would have made this ship dead — that the PRODUCERS gate on the same config, so the vars are needed on outpost-discord-bot and outpost-github-app as well as outpost-worker, not the worker alone. Tests (the reviewers found real defects in the ones this PR added) - The mirror suite was nested inside describe('restoreShadowMode') instead of describe('handleAiResponse'), inheriting an unrelated afterEach and duplicating setup. Relocated; the duplicated setup is gone. - `delivered: true` passed only on leftover mockPostResponse state from an earlier suite. It now re-stubs and asserts the post happened. - The suppressed-undelivered case never asserted a post occurred, so it survived the exact regression it targets. It asserts it now. - `toContain('missing')` matched the ticket id, not the field — it asserted nothing. Now matches the field name. - The enum test listed five members and omitted SLACK_MIRROR; only the count had been bumped. - The inbound suite pinned an env-dependent default; the env-derived fallback is now covered explicitly, including that it is OFF when unset. Call-site enumeration - isSlackMirrorEnabled: callers inbound.ts:139, ai-response.ts:296, slack-mirror.ts:124 — all three still hold; the added token requirement only narrows when it returns true. - isMirrorableSource: new; called from inbound.ts:161 and ai-response.ts:296. - CreateJobFn: `source` stays REQUIRED. Narrowing it broke assignability for every bot wrapper (discord-bot/src/events/{message-create,thread-create}.ts declare it required) — verified by typecheck, reverted, field declared on the payload instead. - JobResult.retryable: read only in worker.ts's failure dispatch; optional, so the ten existing handlers are unaffected. - SlackMirrorPayload.delivered -> delivery: producers ai-response.ts:301 and the handler's formatReplyPost were the only readers; zero remaining references to `delivered` (grep). Not fixed, still open: message-level reply dedup (a duplicate reply job re-posts the same message; it cannot spawn a rival thread). Needs a per-message marker. Verified: typecheck 10/10, build 10/10, 1,765 tests pass. Red-green confirmed on the worker's permanent-failure path. Prettier clean on every file whose baseline was clean; worker.ts was already prettier-dirty on main and was left unformatted to avoid unrelated churn. --- .env.example | 45 +++-- docs/deployment.md | 15 +- .../queue/src/__tests__/ai-response.test.ts | 176 +++++++++------- .../outpost/queue/src/__tests__/queue.test.ts | 53 +++++ .../handlers/__tests__/slack-mirror.test.ts | 190 +++++++++++++++++- .../outpost/queue/src/handlers/ai-response.ts | 31 ++- .../queue/src/handlers/slack-mirror.ts | 155 +++++++++++--- packages/outpost/queue/src/types.ts | 54 ++++- packages/outpost/queue/src/worker.ts | 15 +- .../src/__tests__/platforms-inbound.test.ts | 55 ++++- .../outpost/shared/src/platforms/inbound.ts | 28 ++- .../outpost/shared/src/platforms/index.ts | 7 +- .../src/platforms/slack-mirror-config.ts | 61 +++++- 13 files changed, 730 insertions(+), 155 deletions(-) diff --git a/.env.example b/.env.example index e6346101..cc96246d 100644 --- a/.env.example +++ b/.env.example @@ -67,23 +67,44 @@ SLACK_SIGNING_SECRET= SLACK_SOCKET_MODE=true # ─── Slack Ticket Mirror ───────────────────────────────────────────────────── -# Mirrors every GitHub + Discord ticket into one Slack channel; community -# follow-ups and the AI reply thread under it. Read-only (replying in Slack -# does NOT post back to the source). +# Mirrors tickets into one Slack channel: the ticket opens a thread, community +# follow-ups and the AI reply thread under it. Read-only (replying in Slack does +# NOT post back to the source). # -# Mode: off (default, handler no-ops) | shadow (logs the message, posts nothing) -# | live (posts). Deliberately INDEPENDENT of SHADOW_MODE — that flag protects -# community surfaces (Discord/GitHub) where real reporters watch; this posts to -# an internal team channel, so staging posting here is intended. +# Which tickets: the allowlist in isMirrorableSource() +# (packages/outpost/shared/src/platforms/slack-mirror-config.ts) — today Discord, +# GitHub issues, and GitHub discussions. Slack-sourced tickets are excluded +# (they already live in Slack); so are Teams/Email/Web/Manual/Linear. +# +# Mode: off (default, nothing is enqueued and the handler no-ops) +# | shadow (logs what it would post; posts nothing; needs NO token) +# | live (posts; requires SLACK_BOT_TOKEN, or the mirror stays disabled +# and logs why once) +# Any mode does nothing at all while SLACK_MIRROR_CHANNEL_ID is unset. +# +# Deliberately INDEPENDENT of SHADOW_MODE — that flag protects community +# surfaces (Discord/GitHub) where real reporters watch; this posts to an +# internal team channel, so staging posting here is intended. +# +# WHICH SERVICES need these vars: the handler runs in outpost-worker, but the +# PRODUCERS gate on the same config — readSlackMirrorConfig() is called from +# InboundHandler, which runs inside outpost-discord-bot and outpost-github-app. +# Set SLACK_MIRROR_MODE + SLACK_MIRROR_CHANNEL_ID on the worker AND on every +# service that creates tickets, or nothing is ever enqueued and the mirror is +# silently dead. SLACK_MIRROR_MODE=off # Channel ID, NOT a channel name. Slack: open channel -> click its name -> # bottom of the details pane -> Channel ID. Looks like C09AB2CD3EF. SLACK_MIRROR_CHANNEL_ID= -# NOTE: SLACK_BOT_TOKEN above needs chat:write AND must be set on the WORKER -# service (outpost-worker) — the mirror handler runs there, not in the Slack -# bot. The bot must also be invited to the channel or posts fail not_in_channel. -# Do NOT list this channel in MONITORED_CHANNEL_IDS: the bot would read its own -# mirror posts as inbound messages and open a ticket for each one. +# NOTE: SLACK_BOT_TOKEN above needs chat:write and must be set on the WORKER +# service (outpost-worker) — the mirror handler posts from there, not from the +# Slack bot. Invite the bot to the channel or posts fail not_in_channel (the +# handler reports that as permanent and does not retry it). +# Keep this channel OUT of MONITORED_CHANNEL_IDS. The Slack bot drops events +# carrying a bot_id (apps/slack-bot/src/events/message.ts), so its own mirror +# posts would not become tickets today — but monitoring the mirror channel +# would duplicate every ticket's context into the bot's inbound path and makes +# the loop one filter change away. Keep the two channel sets disjoint. # ─── Teams Bot ─────────────────────────────────────────────────────────────── TEAMS_APP_ID= diff --git a/docs/deployment.md b/docs/deployment.md index 79f17cf7..9e5bfde0 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -79,7 +79,7 @@ Copy `.env.example` and fill in all values. Key groups: - **Discord**: `DISCORD_TOKEN`, `DISCORD_CLIENT_ID`, `GUILD_ID`, `MONITORED_CHANNEL_IDS` - **GitHub App**: `GITHUB_APP_ID`, `GITHUB_PRIVATE_KEY`, `GITHUB_INSTALLATION_ID`, `GITHUB_WEBHOOK_SECRET`, `GITHUB_TEAM_LOGINS` (optional) - **Slack**: `SLACK_BOT_TOKEN`, `SLACK_APP_TOKEN`, `SLACK_SIGNING_SECRET`, `MONITORED_CHANNEL_IDS`, `TEAM_MEMBER_IDS` (optional) -- **Slack ticket mirror** (`outpost-worker`, not the Slack bot): `SLACK_MIRROR_MODE` (`off` | `shadow` | `live`, default `off`), `SLACK_MIRROR_CHANNEL_ID` (channel ID, not name). Also needs `SLACK_BOT_TOKEN` with `chat:write` **set on the worker service** — it is otherwise only set on `outpost-slack-bot`, and the mirror handler runs in the worker. Invite the bot to the channel or posts fail `not_in_channel`. `SLACK_MIRROR_MODE` is intentionally independent of `SHADOW_MODE`: that flag protects community surfaces, while the mirror targets an internal channel. **Keep `SLACK_MIRROR_CHANNEL_ID` out of the Slack bot's `MONITORED_CHANNEL_IDS`** — a monitored mirror channel would turn each mirror post into a new inbound ticket. Slack-sourced tickets are never mirrored for the same reason, but keeping the channels disjoint is the durable fix. +- **Slack ticket mirror**: `SLACK_MIRROR_MODE` (`off` | `shadow` | `live`, default `off`), `SLACK_MIRROR_CHANNEL_ID` (channel ID, not name). **Set both on `outpost-worker` AND on every service that creates tickets** (`outpost-discord-bot`, `outpost-github-app`): the handler runs in the worker, but the producers gate on the same config via `readSlackMirrorConfig()` inside `InboundHandler`, so vars present only on the worker mean no job is ever enqueued and the mirror is silently dead. `live` also needs `SLACK_BOT_TOKEN` with `chat:write` **on the worker** — without it the mirror reports itself disabled and logs why once, rather than dead-lettering a job per ticket. `shadow` needs no token. Any mode is inert while `SLACK_MIRROR_CHANNEL_ID` is unset. Invite the bot to the channel or posts fail `not_in_channel`, which the handler treats as permanent and does not retry. Which tickets get mirrored is decided by `isMirrorableSource()` in `packages/outpost/shared/src/platforms/slack-mirror-config.ts` (today: Discord + GitHub issues and discussions; Slack-sourced tickets are excluded because they already live in Slack). `SLACK_MIRROR_MODE` is intentionally independent of `SHADOW_MODE`: that flag protects community surfaces, while the mirror targets an internal channel — see the shadow-mode section for the documented exception. Keep `SLACK_MIRROR_CHANNEL_ID` out of the Slack bot's `MONITORED_CHANNEL_IDS`: the bot drops `bot_id` events today, so its own mirror posts do not become tickets, but monitoring the mirror channel would duplicate ticket context into the inbound path and leave the loop one filter change away. - **Teams**: `TEAMS_APP_ID`, `TEAMS_APP_PASSWORD`, `TEAMS_TENANT_ID` (optional, blank for multi-tenant), `MONITORED_CHANNEL_IDS` - **Linear sync**: `LINEAR_API_KEY`, `LINEAR_WEBHOOK_SECRET`, `LINEAR_TEAM_ID` - **Monitoring**: `SENTRY_DSN` (optional), `LOG_LEVEL` @@ -215,6 +215,19 @@ have it set on `outpost-worker`, not only on a bot. When adding any new outbound post path, check `SHADOW_MODE` before posting — otherwise staging will deliver to real users regardless of the flag. +**Documented exception — the Slack ticket mirror.** `SLACK_MIRROR_MODE` gates the mirror +instead of `SHADOW_MODE`, and the two are deliberately independent. The rule above exists to +protect community surfaces where real reporters are watching; the mirror posts to an internal +team channel, so a staging environment mirroring into it is intended rather than a leak. Any +future outbound path that is NOT a community surface may take the same exemption, but it needs +its own flag and a line in this section — the default remains `SHADOW_MODE`. + +| Path | Gated by | +| ---------------------------------------- | -------------------------------------- | +| Discord / GitHub replies (`AI_RESPONSE`) | `SHADOW_MODE` | +| Onboarding digest (`ONBOARDING_DIGEST`) | `SHADOW_MODE` | +| Slack ticket mirror (`SLACK_MIRROR`) | `SLACK_MIRROR_MODE` (internal channel) | + ### Promotion workflow ``` diff --git a/packages/outpost/queue/src/__tests__/ai-response.test.ts b/packages/outpost/queue/src/__tests__/ai-response.test.ts index 3ec12da5..f118cf98 100644 --- a/packages/outpost/queue/src/__tests__/ai-response.test.ts +++ b/packages/outpost/queue/src/__tests__/ai-response.test.ts @@ -92,6 +92,11 @@ vi.mock('@copilotkit/outpost/shared/platforms', () => ({ readSlackMirrorConfig: () => mockMirrorConfig, isSlackMirrorEnabled: (config: { mode: string; channelId: string | null }) => config.mode !== 'off' && config.channelId !== null, + // Mirrors the real allowlist in shared/src/platforms/slack-mirror-config.ts. + // That predicate's own truth table is pinned in the mirror handler's suite; + // here it only has to route this producer the way production does. + isMirrorableSource: (source: string) => + ['DISCORD', 'GITHUB_ISSUE', 'GITHUB_DISCUSSION'].includes(source), })); // Import after mocks @@ -867,41 +872,6 @@ describe('handleAiResponse', () => { }), ); }); -}); - -/** - * The shadow-mode tests above set SHADOW_MODE and hand it back in a `finally`. - * Getting the hand-back wrong does not fail those tests — it silently defines - * SHADOW_MODE for every test that runs afterwards, because assigning `undefined` - * to `process.env.X` stores the string `"undefined"`. So the restore itself is - * pinned here rather than left to trust. - */ -describe('restoreShadowMode', () => { - const beforeEachTest = process.env.SHADOW_MODE; - afterEach(() => { - restoreShadowMode(beforeEachTest); - }); - - it('unsets SHADOW_MODE entirely when it was never set', () => { - delete process.env.SHADOW_MODE; - const original = process.env.SHADOW_MODE; - process.env.SHADOW_MODE = 'true'; - - restoreShadowMode(original); - - expect('SHADOW_MODE' in process.env).toBe(false); - expect(process.env.SHADOW_MODE).toBeUndefined(); - }); - - it('puts the original value back when it was set', () => { - process.env.SHADOW_MODE = 'false'; - const original = process.env.SHADOW_MODE; - process.env.SHADOW_MODE = 'true'; - - restoreShadowMode(original); - - expect(process.env.SHADOW_MODE).toBe('false'); - }); // ── Slack ticket mirror ────────────────────────────────────────────── describe('Slack ticket mirror enqueue', () => { @@ -912,25 +882,14 @@ describe('restoreShadowMode', () => { } beforeEach(() => { - // This describe sits outside the suite that clears mocks globally, - // so job calls would otherwise accumulate across these cases. - vi.clearAllMocks(); - mockPrismaTicket.update.mockResolvedValue({}); - mockPrismaMessage.create.mockResolvedValue({ id: 'msg-ai-1' }); - mockPrismaMessage.update.mockResolvedValue({}); - mockPrismaJob.create.mockResolvedValue({ id: 'job-1' }); - mockClassifyTicket.mockResolvedValue(sampleClassification); - mockGetFeedbackCalibration.mockResolvedValue(0); - mockPrismaTicket.findUnique.mockResolvedValue(sampleTicket); - mockGenerateSupportResponse.mockResolvedValue(highConfidenceResult); - mockHasAdapter.mockReturnValue(true); - mockGetAdapter.mockReturnValue({ - platform: 'DISCORD', - postResponse: mockPostResponse, - postSystemMessage: vi.fn(), - parseInboundEvent: vi.fn(), - fetchUserInfo: vi.fn(), - }); + // This suite lives inside describe('handleAiResponse'), so the outer + // beforeEach has already cleared mocks and primed the happy path. + // mockPostResponse is re-stubbed deliberately: an earlier version of + // this suite sat OUTSIDE that beforeEach, and its delivered case + // passed only on leftover state from a previous suite. + mockPostResponse.mockResolvedValue('999888'); + mockMirrorConfig.mode = 'live'; + mockMirrorConfig.channelId = 'C0MIRROR'; }); afterEach(() => { @@ -939,59 +898,124 @@ describe('restoreShadowMode', () => { }); it('enqueues nothing while the mirror is off', async () => { + mockMirrorConfig.mode = 'off'; + mockMirrorConfig.channelId = null; + await handleAiResponse({ ticketId: 'tkt-1', source: 'discord' }, makeContext()); expect(mirrorJobs()).toHaveLength(0); }); it('marks the reply delivered when the adapter posted it', async () => { - mockMirrorConfig.mode = 'live'; - mockMirrorConfig.channelId = 'C0MIRROR'; - await handleAiResponse({ ticketId: 'tkt-1', source: 'discord' }, makeContext()); + // Pin that a post actually happened — otherwise this passes even if + // the handler stops posting entirely. + expect(mockPostResponse).toHaveBeenCalledTimes(1); const jobs = mirrorJobs(); expect(jobs).toHaveLength(1); expect(jobs[0].payload).toEqual( - expect.objectContaining({ kind: 'reply', delivered: true }), + expect.objectContaining({ + kind: 'reply', + ticketId: 'tkt-1', + delivery: 'delivered', + }), ); }); - // Shadow mode logs the draft but posts nothing, so the reporter never - // saw it. The mirror must say so rather than implying delivery. - it('marks the reply undelivered in shadow mode', async () => { - mockMirrorConfig.mode = 'live'; - mockMirrorConfig.channelId = 'C0MIRROR'; + it('reports shadow when SHADOW_MODE is on', async () => { const originalShadow = process.env.SHADOW_MODE; try { process.env.SHADOW_MODE = 'true'; await handleAiResponse({ ticketId: 'tkt-1', source: 'discord' }, makeContext()); - const jobs = mirrorJobs(); - expect(jobs).toHaveLength(1); - expect(jobs[0].payload).toEqual( - expect.objectContaining({ kind: 'reply', delivered: false }), + expect(mockPostResponse).not.toHaveBeenCalled(); + expect(mirrorJobs()[0].payload).toEqual( + expect.objectContaining({ kind: 'reply', delivery: 'shadow' }), ); } finally { restoreShadowMode(originalShadow); } }); - // A suppressed run posts safe replacement copy, not the draft stored on - // the Message the mirror renders — so the draft still reached nobody. - it('marks a suppressed reply undelivered even though a post succeeded', async () => { - mockMirrorConfig.mode = 'live'; - mockMirrorConfig.channelId = 'C0MIRROR'; + // A suppressed run posts safe replacement copy, not the draft the mirror + // renders — the draft reached nobody even though the post succeeded. + it('reports withheld for a suppressed draft even though a post succeeded', async () => { mockGenerateSupportResponse.mockResolvedValue(suppressedResult); await handleAiResponse({ ticketId: 'tkt-1', source: 'discord' }, makeContext()); - const jobs = mirrorJobs(); - expect(jobs).toHaveLength(1); - expect(jobs[0].payload).toEqual( - expect.objectContaining({ kind: 'reply', delivered: false }), + expect(mockPostResponse).toHaveBeenCalledTimes(1); + expect(mirrorJobs()[0].payload).toEqual( + expect.objectContaining({ kind: 'reply', delivery: 'withheld' }), + ); + }); + + it('reports post-failed when the adapter throws', async () => { + mockPostResponse.mockRejectedValue(new Error('discord 500')); + + await handleAiResponse({ ticketId: 'tkt-1', source: 'discord' }, makeContext()); + + expect(mirrorJobs()[0].payload).toEqual( + expect.objectContaining({ kind: 'reply', delivery: 'post-failed' }), + ); + }); + + it('reports no-adapter when the source has none registered', async () => { + mockHasAdapter.mockReturnValue(false); + + await handleAiResponse({ ticketId: 'tkt-1', source: 'discord' }, makeContext()); + + expect(mockPostResponse).not.toHaveBeenCalled(); + expect(mirrorJobs()[0].payload).toEqual( + expect.objectContaining({ kind: 'reply', delivery: 'no-adapter' }), ); }); + + // This producer had no source check at all, so a Slack-sourced ticket's + // AI reply opened a thread in the mirror channel. + it('enqueues nothing for a ticket whose source is not mirrorable', async () => { + mockPrismaTicket.findUnique.mockResolvedValue({ ...sampleTicket, source: 'SLACK' }); + + await handleAiResponse({ ticketId: 'tkt-1', source: 'discord' }, makeContext()); + + expect(mirrorJobs()).toHaveLength(0); + }); + }); +}); + +/** + * The shadow-mode tests above set SHADOW_MODE and hand it back in a `finally`. + * Getting the hand-back wrong does not fail those tests — it silently defines + * SHADOW_MODE for every test that runs afterwards, because assigning `undefined` + * to `process.env.X` stores the string `"undefined"`. So the restore itself is + * pinned here rather than left to trust. + */ +describe('restoreShadowMode', () => { + const beforeEachTest = process.env.SHADOW_MODE; + afterEach(() => { + restoreShadowMode(beforeEachTest); + }); + + it('unsets SHADOW_MODE entirely when it was never set', () => { + delete process.env.SHADOW_MODE; + const original = process.env.SHADOW_MODE; + process.env.SHADOW_MODE = 'true'; + + restoreShadowMode(original); + + expect('SHADOW_MODE' in process.env).toBe(false); + expect(process.env.SHADOW_MODE).toBeUndefined(); + }); + + it('puts the original value back when it was set', () => { + process.env.SHADOW_MODE = 'false'; + const original = process.env.SHADOW_MODE; + process.env.SHADOW_MODE = 'true'; + + restoreShadowMode(original); + + expect(process.env.SHADOW_MODE).toBe('false'); }); }); diff --git a/packages/outpost/queue/src/__tests__/queue.test.ts b/packages/outpost/queue/src/__tests__/queue.test.ts index 5546ca03..ed539798 100644 --- a/packages/outpost/queue/src/__tests__/queue.test.ts +++ b/packages/outpost/queue/src/__tests__/queue.test.ts @@ -219,6 +219,58 @@ describe('Worker', () => { ); }); + // A handler that reports retryable:false has told the worker the failure + // cannot succeed on a retry (malformed payload, missing referenced row, + // permanent API rejection like Slack's not_in_channel). Retrying those burns + // every attempt and leaves a dead-letter trail that reads like a transient + // fault. + it('dead-letters immediately when a handler reports the failure as permanent', async () => { + const jobRow = makeJobRow({ attempts: 0, maxAttempts: 5 }); // attempt would be 1 + mockPrisma.$queryRaw.mockResolvedValueOnce([jobRow]); + mockPrisma.$queryRaw.mockResolvedValue([]); + mockPrismaJob.update.mockResolvedValue({}); + + worker.on(JobType.AI_RESPONSE, async () => { + return { success: false, error: 'not_in_channel', retryable: false }; + }); + + worker.start(); + await vi.advanceTimersByTimeAsync(0); + + expect(mockPrismaJob.update).toHaveBeenCalledWith( + expect.objectContaining({ + where: { id: 'job-1' }, + data: expect.objectContaining({ + status: 'DEAD_LETTER', + error: 'not_in_channel', + }), + }), + ); + }); + + // The flag is opt-in: every pre-existing handler omits it and must keep + // retrying exactly as before. + it('still retries a failure that does not set retryable', async () => { + const jobRow = makeJobRow({ attempts: 0, maxAttempts: 5 }); + mockPrisma.$queryRaw.mockResolvedValueOnce([jobRow]); + mockPrisma.$queryRaw.mockResolvedValue([]); + mockPrismaJob.update.mockResolvedValue({}); + + worker.on(JobType.AI_RESPONSE, async () => { + return { success: false, error: 'transient' }; + }); + + worker.start(); + await vi.advanceTimersByTimeAsync(0); + + expect(mockPrismaJob.update).toHaveBeenCalledWith( + expect.objectContaining({ + where: { id: 'job-1' }, + data: expect.objectContaining({ status: 'PENDING', attempts: 1 }), + }), + ); + }); + it('retries failed jobs with backoff when attempts remain', async () => { const jobRow = makeJobRow({ attempts: 1, maxAttempts: 5 }); // attempt will be 2 mockPrisma.$queryRaw.mockResolvedValueOnce([jobRow]); @@ -537,6 +589,7 @@ describe('JobType enum', () => { expect(JobType.SLA_CHECK).toBe('SLA_CHECK'); expect(JobType.ESCALATION).toBe('ESCALATION'); expect(JobType.ONBOARDING_DIGEST).toBe('ONBOARDING_DIGEST'); + expect(JobType.SLACK_MIRROR).toBe('SLACK_MIRROR'); }); it('has exactly 11 job types', () => { diff --git a/packages/outpost/queue/src/handlers/__tests__/slack-mirror.test.ts b/packages/outpost/queue/src/handlers/__tests__/slack-mirror.test.ts index b9199bb0..79e19d7e 100644 --- a/packages/outpost/queue/src/handlers/__tests__/slack-mirror.test.ts +++ b/packages/outpost/queue/src/handlers/__tests__/slack-mirror.test.ts @@ -19,7 +19,12 @@ vi.mock('@copilotkit/outpost/db', () => ({ })); import { handleSlackMirror, SLACK_MIRROR_PLUGIN } from '../slack-mirror.js'; -import { readSlackMirrorConfig, isSlackMirrorEnabled } from '@copilotkit/outpost/shared/platforms'; +import { + readSlackMirrorConfig, + isSlackMirrorEnabled, + isMirrorableSource, + resetSlackMirrorWarnings, +} from '@copilotkit/outpost/shared/platforms'; import type { SlackMirrorConfig } from '@copilotkit/outpost/shared/platforms'; const context = { reportProgress: vi.fn().mockResolvedValue(undefined), jobId: 'job-1' }; @@ -176,7 +181,17 @@ describe('handleSlackMirror', () => { expect(mockLinkCreate).toHaveBeenCalledTimes(1); }); - it('marks an undelivered AI reply instead of implying the reporter saw it', async () => { + /** + * Each undelivered cause must render its OWN reason. The handler used to + * print "withheld or shadow mode" for every one of them, naming a cause that + * was not established for four of the five. + */ + it.each([ + ['shadow', 'SHADOW_MODE was on'], + ['withheld', 'groundedness gate withheld'], + ['post-failed', 'posting to the source platform failed'], + ['no-adapter', 'no delivery was attempted'], + ] as const)('renders the %s reason on an AI reply', async (delivery, expected) => { mockLinkFindUnique.mockResolvedValue({ externalId: 'C0MIRROR:111.222' }); mockMessageFindUnique.mockResolvedValue({ id: 'msg-ai', @@ -187,12 +202,14 @@ describe('handleSlackMirror', () => { const { poster, postMessage } = makePoster(); await handleSlackMirror( - { ticketId: 'tkt-1', kind: 'reply', messageId: 'msg-ai', delivered: false }, + { ticketId: 'tkt-1', kind: 'reply', messageId: 'msg-ai', delivery }, context, { config: liveConfig, poster }, ); - expect(postMessage.mock.calls[0][0].text).toContain('not delivered to the reporter'); + const text = postMessage.mock.calls[0][0].text; + expect(text).toContain(expected); + expect(text).toContain('not sent'); }); it('does not mark a delivered AI reply', async () => { @@ -206,12 +223,56 @@ describe('handleSlackMirror', () => { const { poster, postMessage } = makePoster(); await handleSlackMirror( - { ticketId: 'tkt-1', kind: 'reply', messageId: 'msg-ai', delivered: true }, + { ticketId: 'tkt-1', kind: 'reply', messageId: 'msg-ai', delivery: 'delivered' }, context, { config: liveConfig, poster }, ); - expect(postMessage.mock.calls[0][0].text).not.toContain('not delivered'); + const text = postMessage.mock.calls[0][0].text; + expect(text).not.toContain('not sent'); + expect(text).not.toContain('unconfirmed'); + }); + + // Unknown is not "fine": an AI reply whose fate the payload never recorded + // must not read as delivered. + it('renders an AI reply with no delivery status as unconfirmed', async () => { + mockLinkFindUnique.mockResolvedValue({ externalId: 'C0MIRROR:111.222' }); + mockMessageFindUnique.mockResolvedValue({ + id: 'msg-ai', + author: 'outpost-ai', + content: 'Here is the fix', + isAiGenerated: true, + }); + const { poster, postMessage } = makePoster(); + + await handleSlackMirror( + { ticketId: 'tkt-1', kind: 'reply', messageId: 'msg-ai' }, + context, + { config: liveConfig, poster }, + ); + + expect(postMessage.mock.calls[0][0].text).toContain('delivery unconfirmed'); + }); + + // A human reply is delivered by definition — no label belongs on it. + it('puts no delivery note on a community reply', async () => { + mockLinkFindUnique.mockResolvedValue({ externalId: 'C0MIRROR:111.222' }); + mockMessageFindUnique.mockResolvedValue({ + id: 'msg-9', + author: 'octocat (12345)', + content: 'Still broken', + isAiGenerated: false, + }); + const { poster, postMessage } = makePoster(); + + await handleSlackMirror({ ticketId: 'tkt-1', kind: 'reply', messageId: 'msg-9' }, context, { + config: liveConfig, + poster, + }); + + const text = postMessage.mock.calls[0][0].text; + expect(text).not.toContain('not sent'); + expect(text).not.toContain('unconfirmed'); }); it('shadow mode posts nothing and records no link', async () => { @@ -341,13 +402,14 @@ describe('handleSlackMirror', () => { mockTicketFindUnique.mockResolvedValue(null); const { poster } = makePoster(); - const result = await handleSlackMirror({ ticketId: 'missing', kind: 'ticket' }, context, { + const result = await handleSlackMirror({ ticketId: 'tkt-gone', kind: 'ticket' }, context, { config: liveConfig, poster, }); expect(result.success).toBe(false); - expect(result.error).toContain('missing'); + expect(result.error).toContain('tkt-gone'); + expect(result.error).toContain('not found'); }); it('fails a reply job that carries no messageId', async () => { @@ -360,6 +422,118 @@ describe('handleSlackMirror', () => { }); expect(result.success).toBe(false); + // Assert the FIELD name. The original assertion here was + // toContain('missing'), which matched the ticket id rather than the + // message — it passed for the wrong reason. expect(result.error).toContain('messageId'); + expect(result.retryable).toBe(false); + }); + // ── Validation owes no side effects ────────────────────────────────────── + // A reply job used to reach the thread-opening post BEFORE discovering it + // had no messageId, so every retry posted another root message to Slack. + + it('posts nothing when a reply job carries no messageId', async () => { + mockLinkFindUnique.mockResolvedValue(null); + const { poster, postMessage } = makePoster(); + + const result = await handleSlackMirror({ ticketId: 'tkt-1', kind: 'reply' }, context, { + config: liveConfig, + poster, + }); + + expect(result.success).toBe(false); + expect(result.retryable).toBe(false); + expect(postMessage).not.toHaveBeenCalled(); + expect(mockLinkCreate).not.toHaveBeenCalled(); }); + + it('posts nothing when a reply job names a message that does not exist', async () => { + mockLinkFindUnique.mockResolvedValue(null); + mockMessageFindUnique.mockResolvedValue(null); + const { poster, postMessage } = makePoster(); + + const result = await handleSlackMirror( + { ticketId: 'tkt-1', kind: 'reply', messageId: 'msg-gone' }, + context, + { config: liveConfig, poster }, + ); + + expect(result.success).toBe(false); + expect(result.retryable).toBe(false); + expect(result.error).toContain('msg-gone'); + expect(postMessage).not.toHaveBeenCalled(); + }); + + // ── Permanent Slack errors must not burn retries ───────────────────────── + + it('reports not_in_channel as permanent, with the remedy', async () => { + mockLinkFindUnique.mockResolvedValue(null); + const postMessage = vi.fn().mockRejectedValue( + Object.assign(new Error('An API error occurred'), { + data: { error: 'not_in_channel' }, + }), + ); + + const result = await handleSlackMirror({ ticketId: 'tkt-1', kind: 'ticket' }, context, { + config: liveConfig, + poster: { postMessage }, + }); + + expect(result.success).toBe(false); + expect(result.retryable).toBe(false); + expect(result.error).toContain('not_in_channel'); + expect(result.error).toContain('invite'); + expect(mockLinkCreate).not.toHaveBeenCalled(); + }); + + it('lets a transient Slack error retry', async () => { + mockLinkFindUnique.mockResolvedValue(null); + const postMessage = vi.fn().mockRejectedValue(new Error('ratelimited')); + + await expect( + handleSlackMirror({ ticketId: 'tkt-1', kind: 'ticket' }, context, { + config: liveConfig, + poster: { postMessage }, + }), + ).rejects.toThrow('ratelimited'); + }); +}); + +describe('isSlackMirrorEnabled — live requires a token', () => { + beforeEach(() => { + resetSlackMirrorWarnings(); + }); + + // live + no token used to read as ENABLED, so producers enqueued jobs that + // could only throw in buildPoster and dead-letter, one per ticket, forever. + it('is disabled when live with no token, and says so once', () => { + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + expect(isSlackMirrorEnabled({ mode: 'live', channelId: 'C1', token: null })).toBe(false); + expect(isSlackMirrorEnabled({ mode: 'live', channelId: 'C1', token: null })).toBe(false); + + expect(errorSpy).toHaveBeenCalledTimes(1); + expect(errorSpy.mock.calls[0].join(' ')).toContain('SLACK_BOT_TOKEN'); + errorSpy.mockRestore(); + }); + + it('is enabled when live with a token', () => { + expect(isSlackMirrorEnabled({ mode: 'live', channelId: 'C1', token: 'xoxb-1' })).toBe(true); + }); +}); + +describe('isMirrorableSource', () => { + it('covers GitHub and Discord only', () => { + expect(isMirrorableSource('DISCORD')).toBe(true); + expect(isMirrorableSource('GITHUB_ISSUE')).toBe(true); + expect(isMirrorableSource('GITHUB_DISCUSSION')).toBe(true); + }); + + // The old denylist ("everything except SLACK") silently mirrored these. + it.each(['SLACK', 'TEAMS', 'EMAIL', 'WEB', 'MANUAL', 'LINEAR', 'ORCA'])( + 'excludes %s', + (source) => { + expect(isMirrorableSource(source)).toBe(false); + }, + ); }); diff --git a/packages/outpost/queue/src/handlers/ai-response.ts b/packages/outpost/queue/src/handlers/ai-response.ts index 562dea27..f275ff12 100644 --- a/packages/outpost/queue/src/handlers/ai-response.ts +++ b/packages/outpost/queue/src/handlers/ai-response.ts @@ -25,11 +25,17 @@ import { getAdapter, readSlackMirrorConfig, isSlackMirrorEnabled, + isMirrorableSource, } from '@copilotkit/outpost/shared/platforms'; import { createJob } from '../create-job.js'; import { getFeedbackCalibration } from '../feedback-calibration.js'; import { JobType } from '../types.js'; -import type { AiResponsePayload, JobResult, JobHandlerContext } from '../types.js'; +import type { + AiResponsePayload, + JobResult, + JobHandlerContext, + SlackMirrorDelivery, +} from '../types.js'; /** * Map from TicketSource enum values (stored in DB) to PlatformTarget @@ -194,10 +200,12 @@ export async function handleAiResponse( // suggestedResponse holds the publishable text bots pick up), and step 6 // below escalates on suppression regardless of score. const ticketSource = ticket.source as TicketSource; - // Tracks whether the reporter actually received `aiMessage.content`. - // The Slack mirror labels its post with this, so an internal reader is - // never told the community saw a draft that was withheld or only logged. - let draftReachedReporter = false; + // What became of `aiMessage.content`, recorded at the branch that knows. + // The Slack mirror renders this verbatim, so an internal reader is never + // told the community saw a draft that was withheld, only logged, or lost + // to a failed post. Starts as the no-adapter case: if no branch below + // claims it, nothing was ever attempted. + let delivery: SlackMirrorDelivery = 'no-adapter'; if (pipelineResult.suppressed) { console.warn( `[AI Response] Ungrounded draft withheld for ticket ${ticketId} — ` + @@ -207,6 +215,9 @@ export async function handleAiResponse( } if (process.env.SHADOW_MODE === 'true') { + // Shadow mode is a fact about this run, independent of whether the + // shadow Message row below persists — claim it before the try. + delivery = 'shadow'; try { await prisma.message.create({ data: { @@ -263,12 +274,13 @@ export async function handleAiResponse( } // A suppressed run posts safe replacement copy, not the // draft stored on aiMessage — so the draft itself still - // never reached anyone. - draftReachedReporter = !pipelineResult.suppressed; + // never reached anyone, even though the post succeeded. + delivery = pipelineResult.suppressed ? 'withheld' : 'delivered'; console.log( `[AI Response] Posted response to ${ticket.source} for ticket ${ticketId}`, ); } catch (error) { + delivery = 'post-failed'; console.error( `[AI Response] Failed to post response to ${ticket.source} for ticket ${ticketId}:`, error instanceof Error ? error.message : String(error), @@ -283,13 +295,14 @@ export async function handleAiResponse( // ticket. Enqueued regardless of whether the draft was delivered — an // answer the community never saw is precisely what the team needs to // notice — but labelled with which of those happened. - if (isSlackMirrorEnabled(readSlackMirrorConfig())) { + if (isMirrorableSource(ticket.source) && isSlackMirrorEnabled(readSlackMirrorConfig())) { try { await createJob(JobType.SLACK_MIRROR, { ticketId: ticket.id, + source: payload.source, kind: 'reply', messageId: aiMessage.id, - delivered: draftReachedReporter, + delivery, }); } catch (error) { console.error( diff --git a/packages/outpost/queue/src/handlers/slack-mirror.ts b/packages/outpost/queue/src/handlers/slack-mirror.ts index c3711baa..bb6aa10c 100644 --- a/packages/outpost/queue/src/handlers/slack-mirror.ts +++ b/packages/outpost/queue/src/handlers/slack-mirror.ts @@ -19,7 +19,12 @@ import { buildPermalink, type SlackMirrorConfig, } from '@copilotkit/outpost/shared/platforms'; -import type { SlackMirrorPayload, JobResult, JobHandlerContext } from '../types.js'; +import type { + SlackMirrorPayload, + SlackMirrorDelivery, + JobResult, + JobHandlerContext, +} from '../types.js'; /** TicketExternalLink.plugin value owned by the mirror. */ export const SLACK_MIRROR_PLUGIN = 'slack'; @@ -64,6 +69,50 @@ function buildPoster(config: SlackMirrorConfig): SlackPoster { }; } +/** + * Slack errors that no retry can fix. + * + * `not_in_channel` needs a human to invite the bot; `channel_not_found` and + * `invalid_auth` need a config change. Retrying them five times and + * dead-lettering hides an operator task behind a fault that reads as transient, + * so each is reported once with the remedy attached. + */ +const PERMANENT_SLACK_ERRORS: Record = { + not_in_channel: + 'invite the mirror bot to SLACK_MIRROR_CHANNEL_ID (Slack: channel → Integrations → Add apps)', + channel_not_found: + 'SLACK_MIRROR_CHANNEL_ID does not name a channel this bot can see — check the ID, not the name', + channel_is_archived: + 'the mirror channel is archived — point SLACK_MIRROR_CHANNEL_ID at a live channel', + invalid_auth: 'SLACK_BOT_TOKEN is invalid or revoked — reissue it', + account_inactive: 'the Slack bot account is deactivated — reinstall the app', + missing_scope: 'the bot token lacks chat:write — add the scope and reinstall', + is_archived: 'the mirror channel is archived — point SLACK_MIRROR_CHANNEL_ID at a live channel', +}; + +/** + * Map a thrown Slack error to a permanent-failure result, or null when it looks + * transient (rate limits, 5xx, network) and a retry is worth having. + */ +function classifySlackError(err: unknown, ticketLabel: string): JobResult | null { + // @slack/web-api puts the API's error string on `err.data.error`, and often + // on `err.message` too. Duck-type both rather than importing its error class. + const data = (err as { data?: { error?: unknown } })?.data; + const fromData = typeof data?.error === 'string' ? data.error : undefined; + const message = err instanceof Error ? err.message : String(err); + const code = fromData ?? Object.keys(PERMANENT_SLACK_ERRORS).find((k) => message.includes(k)); + + if (!code) return null; + const remedy = PERMANENT_SLACK_ERRORS[code]; + if (!remedy) return null; + + return { + success: false, + error: `Slack rejected the mirror post for ${ticketLabel}: ${code}. To fix: ${remedy}`, + retryable: false, + }; +} + /** Thread-opening post: what the ticket is, who reported it, where it came from. */ function formatTicketPost(ticket: { displayId: string; @@ -80,24 +129,43 @@ function formatTicketPost(ticket: { return `${header}\n${origin}\n\n${body}`; } +/** + * What each delivery outcome says to an internal reader. + * + * The mirror states the reason the producer recorded and nothing more. It used + * to print "withheld or shadow mode" for every undelivered case, which named a + * cause for failures that had a different one — the same misreporting #148 + * describes between what the DB holds and what was published. + */ +const DELIVERY_NOTES: Record = { + delivered: '', + shadow: '\n_⚠️ not sent — SHADOW_MODE was on, so this was logged only_', + withheld: + '\n_⚠️ not sent — the groundedness gate withheld this draft; the reporter got the safe replacement_', + 'post-failed': '\n_⚠️ not sent — posting to the source platform failed_', + 'no-adapter': '\n_⚠️ not sent — no delivery was attempted for this source_', +}; + +/** An AI reply whose fate the payload did not record. Unknown is not "fine". */ +const DELIVERY_UNKNOWN = '\n_⚠️ delivery unconfirmed — this reply carried no delivery status_'; + /** * Threaded reply post. * - * An AI message that was withheld or shadow-logged is labelled as such. The - * mirror must not imply the reporter saw something they never saw — that is - * exactly the divergence #148 describes between what the DB records and what - * was actually published. + * Community and team replies carry no delivery status: the platform delivered + * them by definition. For an AI reply the status is mandatory in practice — its + * absence renders as unconfirmed, never as delivered. */ function formatReplyPost( message: { author: string; content: string; isAiGenerated: boolean }, - delivered: boolean | undefined, + delivery: SlackMirrorDelivery | undefined, ): string { const who = message.isAiGenerated ? `🤖 ${message.author}` : message.author; - const undelivered = - message.isAiGenerated && delivered === false - ? '\n_⚠️ not delivered to the reporter — withheld or shadow mode_' - : ''; - return `*${who}*${undelivered}\n${truncate(message.content, MAX_MIRROR_TEXT)}`; + let note = ''; + if (message.isAiGenerated) { + note = delivery ? DELIVERY_NOTES[delivery] : DELIVERY_UNKNOWN; + } + return `*${who}*${note}\n${truncate(message.content, MAX_MIRROR_TEXT)}`; } /** @@ -147,6 +215,34 @@ export async function handleSlackMirror( return { success: true, data: { skipped: 'already-mirrored' } }; } + // ── Validate the job before anything is posted ─────────────────────────── + // A reply job used to reach the thread-opening post first and only then + // discover it had no messageId — posting a root message to Slack on every + // one of its retries. Validation owes no side effects. + let replyMessage: { + id: string; + author: string; + content: string; + isAiGenerated: boolean; + } | null = null; + if (payload.kind === 'reply') { + if (!payload.messageId) { + return { + success: false, + error: 'SLACK_MIRROR reply job is missing required field `messageId`', + retryable: false, + }; + } + replyMessage = await db.message.findUnique({ where: { id: payload.messageId } }); + if (!replyMessage) { + return { + success: false, + error: `SLACK_MIRROR reply job references Message ${payload.messageId}, which does not exist`, + retryable: false, + }; + } + } + const isShadow = config.mode === 'shadow'; const poster = isShadow ? null : (deps?.poster ?? buildPoster(config)); @@ -160,7 +256,14 @@ export async function handleSlackMirror( `[Slack Mirror] shadow — would open thread in ${channelId} for ${ticket.displayId}:\n${text}`, ); } else { - const result = await poster!.postMessage({ channel: channelId, text }); + let result: { ts?: string }; + try { + result = await poster!.postMessage({ channel: channelId, text }); + } catch (err) { + const permanent = classifySlackError(err, ticket.displayId); + if (permanent) return permanent; + throw err; + } if (!result.ts) { return { success: false, @@ -213,16 +316,8 @@ export async function handleSlackMirror( // ── Post the reply underneath it ───────────────────────────────────────── if (payload.kind === 'reply') { - if (!payload.messageId) { - return { success: false, error: 'SLACK_MIRROR reply job carried no messageId' }; - } - - const message = await db.message.findUnique({ where: { id: payload.messageId } }); - if (!message) { - return { success: false, error: `Message ${payload.messageId} not found` }; - } - - const text = formatReplyPost(message, payload.delivered); + const message = replyMessage!; + const text = formatReplyPost(message, payload.delivery); if (isShadow) { console.log( `[Slack Mirror] shadow — would reply in ${channelId} on ${ticket.displayId}:\n${text}`, @@ -231,11 +326,17 @@ export async function handleSlackMirror( // Post into the channel the link records, not the currently configured // one: re-pointing SLACK_MIRROR_CHANNEL_ID must not orphan the replies // of tickets whose thread already lives somewhere else. - await poster!.postMessage({ - channel: thread!.channelId, - text, - thread_ts: thread!.ts, - }); + try { + await poster!.postMessage({ + channel: thread!.channelId, + text, + thread_ts: thread!.ts, + }); + } catch (err) { + const permanent = classifySlackError(err, ticket.displayId); + if (permanent) return permanent; + throw err; + } } } diff --git a/packages/outpost/queue/src/types.ts b/packages/outpost/queue/src/types.ts index dbf8f1ac..f3c1a624 100644 --- a/packages/outpost/queue/src/types.ts +++ b/packages/outpost/queue/src/types.ts @@ -95,6 +95,28 @@ export type GithubReactionPollPayload = Record; */ export type SlackMirrorKind = 'ticket' | 'reply'; +/** + * What actually happened to an AI reply, from the producer that knows. + * + * A boolean was not enough. `delivered: false` covered five distinct causes — + * shadow mode, a suppressed draft, no adapter, adapter misconfigured, and the + * post throwing — and the mirror rendered one guess ("withheld or shadow mode") + * for all of them, asserting a cause nobody established. That is the same class + * of misreporting the mirror's delivery label exists to prevent, so the reason + * travels with the payload instead of being inferred. + */ +export type SlackMirrorDelivery = + /** Posted to the source platform; the reporter can see it. */ + | 'delivered' + /** SHADOW_MODE was on: logged to the DB, never posted. */ + | 'shadow' + /** The groundedness gate withheld the draft; safe replacement copy went out instead. */ + | 'withheld' + /** postResponse threw — a delivery failure, not a deliberate hold. */ + | 'post-failed' + /** No adapter for this source, or adapter construction failed. */ + | 'no-adapter'; + export interface SlackMirrorPayload { /** The Outpost ticket ID being mirrored */ ticketId: string; @@ -103,13 +125,23 @@ export interface SlackMirrorPayload { /** The Message row this post reflects; omit for the thread-opening post */ messageId?: string; /** - * For AI replies: whether the answer actually reached the reporter. + * Platform the ticket came from, as a PlatformTarget string. + * + * Declared rather than left to ride the CreateJobFn index signature: both + * producers send it, and an undeclared field that only type-checks by + * accident is how the two of them drifted into different payload shapes. + * The handler does not read it — it re-reads the ticket — but it makes a + * queued job legible on its own. + */ + source?: string; + /** + * For AI replies: what became of the answer. * - * Shadow mode and the groundedness gate both produce an AI Message row that - * was never delivered. The mirror labels those explicitly rather than - * implying the community saw them — the same failure #148 describes. + * Omitted on community/team replies (the platform delivered those by + * definition). Absent on an AI reply means UNKNOWN, which the mirror renders + * as unconfirmed — never as delivered. */ - delivered?: boolean; + delivery?: SlackMirrorDelivery; } /** Map from JobType to its specific payload shape */ @@ -133,6 +165,18 @@ export interface JobResult { success: boolean; data?: Record; error?: string; + /** + * Whether a failure is worth retrying. Omit for the historical behavior + * (retry until `maxAttempts`, then dead-letter). + * + * Set `false` only for failures that CANNOT succeed on a retry — a malformed + * payload, a missing row it references, a permanent API rejection like + * Slack's `not_in_channel`. Those previously consumed every attempt and + * landed in the dead-letter queue with a misleading trail suggesting a + * transient fault. Additive by design: every existing handler omits it and + * behaves exactly as before. + */ + retryable?: boolean; } // ─── Options ──────────────────────────────────────────────────────────────── diff --git a/packages/outpost/queue/src/worker.ts b/packages/outpost/queue/src/worker.ts index 6c7738ad..02081387 100644 --- a/packages/outpost/queue/src/worker.ts +++ b/packages/outpost/queue/src/worker.ts @@ -349,7 +349,20 @@ export class Worker { }, }); } else { - await this.handleFailure(job.id, attempt, job.maxAttempts, result.error ?? 'Unknown error'); + // A handler that reports `retryable: false` has told us the + // failure cannot succeed on a retry (malformed payload, + // missing referenced row, permanent API rejection). Retrying + // it burns every attempt and leaves a dead-letter trail that + // reads like a transient fault. Dead-letter it immediately by + // presenting the attempt as the final one. + await this.handleFailure( + job.id, + // Presenting the attempt as the last one is what makes + // handleFailure dead-letter instead of scheduling a retry. + result.retryable === false ? job.maxAttempts : attempt, + job.maxAttempts, + result.error ?? 'Unknown error', + ); } } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); diff --git a/packages/outpost/shared/src/__tests__/platforms-inbound.test.ts b/packages/outpost/shared/src/__tests__/platforms-inbound.test.ts index ff1b4b99..f6f869c9 100644 --- a/packages/outpost/shared/src/__tests__/platforms-inbound.test.ts +++ b/packages/outpost/shared/src/__tests__/platforms-inbound.test.ts @@ -654,19 +654,62 @@ describe('InboundHandler', () => { }); }); - // A Slack-sourced ticket mirrored into a monitored Slack channel would - // arrive back as inbound, open a ticket, mirror again, and loop. - it('never mirrors a Slack-sourced ticket back into Slack', async () => { + // The mirror is an ALLOWLIST of GitHub + Discord. A denylist ("anything + // but SLACK") silently mirrored TEAMS/EMAIL/WEB/MANUAL/LINEAR tickets the + // feature was never specified for. + it.each([ + TicketSource.SLACK, + TicketSource.TEAMS, + TicketSource.EMAIL, + TicketSource.WEB, + TicketSource.MANUAL, + TicketSource.LINEAR, + ])('does not mirror a %s-sourced ticket', async (source) => { const handler = new InboundHandler({ prisma, createJob, mirrorToSlack: true }); - await handler.handle( - makeInboundMessage({ source: TicketSource.SLACK, channelId: 'C0MIRROR' }), - ); + await handler.handle(makeInboundMessage({ source })); const types = (createJob as ReturnType).mock.calls.map((c) => c[0]); expect(types).not.toContain('SLACK_MIRROR'); }); + it.each([TicketSource.GITHUB_ISSUE, TicketSource.GITHUB_DISCUSSION])( + 'mirrors a %s-sourced ticket', + async (source) => { + const handler = new InboundHandler({ prisma, createJob, mirrorToSlack: true }); + + await handler.handle(makeInboundMessage({ source })); + + const types = (createJob as ReturnType).mock.calls.map((c) => c[0]); + expect(types).toContain('SLACK_MIRROR'); + }, + ); + + // Production does not pass mirrorToSlack — it falls back to the env. With + // the env unset the fallback must be OFF, so a stray SLACK_MIRROR_MODE in + // a developer's shell cannot silently enqueue jobs (and cannot perturb the + // call-count assertions in every other test in this file). + it('defaults to disabled when the environment configures no mirror', async () => { + const originalMode = process.env.SLACK_MIRROR_MODE; + const originalChannel = process.env.SLACK_MIRROR_CHANNEL_ID; + try { + delete process.env.SLACK_MIRROR_MODE; + delete process.env.SLACK_MIRROR_CHANNEL_ID; + + const handler = new InboundHandler({ prisma, createJob }); + await handler.handle(makeInboundMessage()); + + const types = (createJob as ReturnType).mock.calls.map((c) => c[0]); + expect(types).not.toContain('SLACK_MIRROR'); + } finally { + if (originalMode !== undefined) process.env.SLACK_MIRROR_MODE = originalMode; + else delete process.env.SLACK_MIRROR_MODE; + if (originalChannel !== undefined) + process.env.SLACK_MIRROR_CHANNEL_ID = originalChannel; + else delete process.env.SLACK_MIRROR_CHANNEL_ID; + } + }); + it('still creates the ticket when enqueueing the mirror job fails', async () => { const failing = vi.fn().mockImplementation((type: string) => { if (type === 'SLACK_MIRROR') return Promise.reject(new Error('queue down')); diff --git a/packages/outpost/shared/src/platforms/inbound.ts b/packages/outpost/shared/src/platforms/inbound.ts index 654c83ec..5029a9ab 100644 --- a/packages/outpost/shared/src/platforms/inbound.ts +++ b/packages/outpost/shared/src/platforms/inbound.ts @@ -13,7 +13,11 @@ import type { InboundMessage, InboundResult, TicketRef } from './types.js'; import { generateTicketId, truncate } from '../utils.js'; import { TicketSource } from '../types.js'; -import { readSlackMirrorConfig, isSlackMirrorEnabled } from './slack-mirror-config.js'; +import { + readSlackMirrorConfig, + isSlackMirrorEnabled, + isMirrorableSource, +} from './slack-mirror-config.js'; /** * Prisma client interface — the subset of PrismaClient we actually call. @@ -53,6 +57,9 @@ export interface PrismaLike { */ export type CreateJobFn = ( type: string, + // Every bot's wrapper declares `source` as required, so it stays required + // here — narrowing it would break assignability for all of them. The index + // signature is what lets a job type add its own fields. payload: { ticketId: string; threadId?: string; source: string; [key: string]: unknown }, ) => Promise; @@ -146,12 +153,12 @@ export class InboundHandler { ): Promise { if (!this.mirrorToSlack) return; - // Never mirror a Slack-sourced ticket back into Slack. If the mirror - // channel is one the Slack bot monitors, the mirror post would arrive - // as a new inbound message, open a ticket, mirror that, and loop. The - // mirror exists to bring GitHub and Discord into Slack; Slack tickets - // are already there. - if (source === TicketSource.SLACK) return; + // The mirror covers GitHub and Discord. `isMirrorableSource` is the ONE + // place that rule lives; the AI-reply producer routes through the same + // predicate, which is what stops the two producers from disagreeing + // about whether a ticket is mirrorable. Slack-sourced tickets are + // excluded because they already live in Slack. + if (!isMirrorableSource(source)) return; try { await this.createJob(this.slackMirrorJobType, { @@ -160,9 +167,14 @@ export class InboundHandler { ...payload, }); } catch (err) { + // The mirror is an internal convenience view; a queue failure here + // must never take down ticket creation for a real reporter. Log the + // error class and stack so schema drift is not mistaken for a + // transient queue hiccup. console.error( `[InboundHandler] Failed to enqueue Slack mirror (${payload.kind}) for ticket ${ticketId}:`, - err instanceof Error ? err.message : String(err), + err instanceof Error ? `${err.name}: ${err.message}` : String(err), + err instanceof Error ? err.stack : undefined, ); } } diff --git a/packages/outpost/shared/src/platforms/index.ts b/packages/outpost/shared/src/platforms/index.ts index 6d926ab0..80960afa 100644 --- a/packages/outpost/shared/src/platforms/index.ts +++ b/packages/outpost/shared/src/platforms/index.ts @@ -51,7 +51,12 @@ export { SlackAdapter as PlatformSlackAdapter, SlackAdapter, buildPermalink } fr // Slack ticket mirror — flag semantics shared by the producers (inbound // handler, AI response handler) and the consumer (SLACK_MIRROR job handler). -export { readSlackMirrorConfig, isSlackMirrorEnabled } from './slack-mirror-config.js'; +export { + readSlackMirrorConfig, + isSlackMirrorEnabled, + isMirrorableSource, + resetSlackMirrorWarnings, +} from './slack-mirror-config.js'; export type { SlackMirrorConfig, SlackMirrorMode } from './slack-mirror-config.js'; export type { SlackAdapterConfig as PlatformSlackAdapterConfig, SlackAdapterConfig, SlackMessageEvent } from './slack.js'; diff --git a/packages/outpost/shared/src/platforms/slack-mirror-config.ts b/packages/outpost/shared/src/platforms/slack-mirror-config.ts index 12fb0112..5b4b7ed4 100644 --- a/packages/outpost/shared/src/platforms/slack-mirror-config.ts +++ b/packages/outpost/shared/src/platforms/slack-mirror-config.ts @@ -49,8 +49,67 @@ export function readSlackMirrorConfig(env: NodeJS.ProcessEnv = process.env): Sla * `shadow` counts as enabled — it exists to be exercised, and its whole value * is producing log lines showing what a live run would post. Only the channel * ID is required for that, not the token. + * + * `live` additionally requires a token. Without one, every job would reach the + * poster, throw, and burn its retries into the dead-letter queue — one per + * ticket, indefinitely. A misconfigured `live` therefore reads as DISABLED + * (nothing is enqueued, nothing dies) and says so once, loudly, because the + * operator's intent was to post and silence would hide that it never did. */ export function isSlackMirrorEnabled(config: SlackMirrorConfig): boolean { if (config.mode === 'off') return false; - return config.channelId !== null; + if (config.channelId === null) return false; + if (config.mode === 'live' && config.token === null) { + warnLiveWithoutToken(); + return false; + } + return true; +} + +/** Latch so the misconfiguration is reported once per process, not per job. */ +let liveWithoutTokenWarned = false; + +function warnLiveWithoutToken(): void { + if (liveWithoutTokenWarned) return; + liveWithoutTokenWarned = true; + console.error( + '[Slack Mirror] SLACK_MIRROR_MODE=live but SLACK_BOT_TOKEN is unset — the mirror is ' + + 'DISABLED. Set SLACK_BOT_TOKEN (needs chat:write) on this service; the mirror handler ' + + 'runs in outpost-worker, so the worker needs it too, not only outpost-slack-bot.', + ); +} + +/** Test seam: reset the once-per-process warning latch. */ +export function resetSlackMirrorWarnings(): void { + liveWithoutTokenWarned = false; +} + +/** + * Ticket sources the mirror covers. + * + * An ALLOWLIST, deliberately. The mirror exists to bring GitHub and Discord + * tickets into Slack; a denylist ("everything except SLACK") silently pulled in + * TEAMS, EMAIL, WEB, MANUAL, and LINEAR tickets the feature was never specified + * for. Slack-sourced tickets are excluded because they already live in Slack. + * + * Values are the string forms of `TicketSource` (shared/src/types.ts). This + * module deliberately does not import that enum: it is consumed by both the + * queue package and the platform producers, and staying string-keyed keeps it + * free of a cycle through the platform barrel. + */ +const MIRRORABLE_SOURCES: ReadonlySet = new Set([ + 'DISCORD', + 'GITHUB_ISSUE', + 'GITHUB_DISCUSSION', +]); + +/** + * Whether a ticket from this source should be mirrored. + * + * Both producers MUST route through this. The rule previously lived in the + * inbound producer only, so the AI-reply producer mirrored everything — which + * is how a Slack-sourced ticket ended up opening a thread in the mirror channel. + */ +export function isMirrorableSource(source: string): boolean { + return MIRRORABLE_SOURCES.has(source); } From af58ed6017b01073915b6e5cf48070cc292d4879 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nathan=20=F0=9F=94=B6=20Tarbert?= <66887028+NathanTarbert@users.noreply.github.com> Date: Mon, 3 Aug 2026 10:52:33 -0400 Subject: [PATCH 4/9] fix(slack): close round-2 CR findings, including three I introduced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Confirmation round (8 reviewers) returned a new bucket (a). Several items were defects in the round-1 fixes themselves, which is what the round is for. Introduced by the previous commit, now fixed - The token requirement in `isSlackMirrorEnabled` made the PRODUCERS treat a tokenless `live` as disabled. Since the producers run in the bots and only enqueue, that left the mirror silently dead while the docs said the token belonged on the worker. Split the predicate: `isSlackMirrorEnabled` (mode + channel) gates the producers, `canSlackMirrorPost` additionally requires a token and gates the consumer, which reports a permanent failure naming the variable. The bot token no longer has to be spread to services that never post. - `retryable: false` dead-lettered by presenting the attempt as `maxAttempts`, writing a fabricated attempt count — an exhausted-retry trail for a job that ran once. `handleFailure` now takes an explicit `permanent` flag and records the true count. - The mirror payload forwarded the AI job's optional `source` hint, so it could record `source: undefined` while the inbound producer sent a resolved value. Sends the resolved source now, and the type's comment no longer overclaims. Also fixed - An unrecognized `delivery` value indexed to `undefined` and rendered the literal string "undefined" into Slack. Unknown values fall back to "unconfirmed". - An unknown `kind` fell through both branches: it posted a root message and returned success. Rejected up front as permanent. - "Accepted but no ts" was retryable even though the post had landed, so every retry posted another root message. Now permanent, and says the message was posted but cannot be tracked. - Ticket bodies and replies were interpolated into Slack mrkdwn unescaped, so a reporter on a public tracker could inject links and mentions into an internal channel. Escapes &, <, > per Slack's formatting rules. - Slack error classification preferred a substring scan that was order-dependent; now prefers the API's own code with a word-boundary fallback, and covers token_expired, token_revoked, not_authed, msg_too_long. - An unrecognized SLACK_MIRROR_MODE failed closed silently, indistinguishable from a deliberate `off`. It logs now. - WebClient's default retry policy can sleep for minutes, outliving the 60s job timeout and leaving a post in flight after the worker gave up. Capped to 2 retries with a 2s ceiling. - Corrected comments that overclaimed: the idempotency guarantee covers THREAD identity only (a duplicate reply still re-posts), the text cap is a readability budget rather than a Slack hard limit, and the shadow log now says shadow does not persist thread identity so repeated opens are expected. Tests - Ambient-environment sensitivity, reproduced by a reviewer: 3 tests in the inbound suite failed with SLACK_MIRROR_MODE exported, and 7 in the AI suite failed with SHADOW_MODE=true inherited. Both suites now neutralize and restore the ambient values. - The permanent-failure test now pins `attempts: 1` — the assertion whose absence hid the fabricated count. - New coverage: no-ts branch, unknown kind, live-without-token, mrkdwn escaping, unrecognized delivery, the split producer/consumer predicates, empty channel id. - Fixture corrections: `source: 'GITHUB'` was not a TicketSource value, and the mirror fixture encoded live-with-null-token, a config production rejects. Call-site enumeration - isSlackMirrorEnabled: inbound.ts:139, ai-response.ts:296, slack-mirror.ts:124 — all three still hold; the predicate only widened (token no longer required). - canSlackMirrorPost: new; sole caller slack-mirror.ts, after the enabled check. - resetSlackMirrorWarnings: removed with the warn latch it served; zero remaining references (grep), test import updated. - handleFailure: sole caller is worker.ts's failure dispatch, both arms updated; the new parameter defaults to false so the throw path is unchanged. Still open, tracked: message-level reply dedup (needs a per-message marker); truncation splits surrogate pairs; a new WebClient per job. Verified: typecheck 10/10, 1,774 tests pass, prettier clean on files whose baseline was clean. --- apps/worker/src/index.ts | 1 + .../queue/src/__tests__/ai-response.test.ts | 24 ++- .../outpost/queue/src/__tests__/queue.test.ts | 3 + .../handlers/__tests__/slack-mirror.test.ts | 160 ++++++++++++++++-- .../outpost/queue/src/handlers/ai-response.ts | 5 +- .../queue/src/handlers/slack-mirror.ts | 117 +++++++++++-- packages/outpost/queue/src/types.ts | 5 +- packages/outpost/queue/src/worker.ts | 17 +- .../src/__tests__/platforms-inbound.test.ts | 22 ++- .../outpost/shared/src/platforms/index.ts | 2 +- .../src/platforms/slack-mirror-config.ts | 57 ++++--- 11 files changed, 344 insertions(+), 69 deletions(-) diff --git a/apps/worker/src/index.ts b/apps/worker/src/index.ts index fc3d355e..5e0bc8a2 100644 --- a/apps/worker/src/index.ts +++ b/apps/worker/src/index.ts @@ -15,6 +15,7 @@ * - TRACKER_SYNC: Push changes to external trackers * - JOB_CLEANUP: Periodic cleanup of old jobs and sync events * - GITHUB_REACTION_POLL: Poll GitHub reactions on AI comments (no webhook exists) + * - SLACK_MIRROR: mirror a ticket or reply into the internal Slack channel */ import http from 'node:http'; diff --git a/packages/outpost/queue/src/__tests__/ai-response.test.ts b/packages/outpost/queue/src/__tests__/ai-response.test.ts index f118cf98..49246b42 100644 --- a/packages/outpost/queue/src/__tests__/ai-response.test.ts +++ b/packages/outpost/queue/src/__tests__/ai-response.test.ts @@ -5,7 +5,22 @@ * classification, message persistence, and escalation triggering. * All external dependencies (Prisma, AIPipeline, etc.) are mocked. */ -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach, beforeAll, afterAll } from 'vitest'; + +// Seven tests in this file assert the non-shadow path. An inherited +// SHADOW_MODE=true flips the handler and fails them, so the ambient value is +// cleared for the whole suite and restored afterwards. +const AMBIENT_SHADOW = { value: undefined as string | undefined }; + +beforeAll(() => { + AMBIENT_SHADOW.value = process.env.SHADOW_MODE; + delete process.env.SHADOW_MODE; +}); + +afterAll(() => { + if (AMBIENT_SHADOW.value !== undefined) process.env.SHADOW_MODE = AMBIENT_SHADOW.value; + else delete process.env.SHADOW_MODE; +}); import type { JobHandlerContext } from '../types.js'; // ─── Mock Setup ───────────────────────────────────────────────────────────── @@ -69,7 +84,9 @@ const mockGetAdapter = vi.fn().mockReturnValue({ const mockMirrorConfig: { mode: string; channelId: string | null; token: string | null } = { mode: 'off', channelId: null, - token: null, + // A real token: the mirror suite below runs in `live`, and a null token is a + // config the production predicate treats as unable to post. + token: 'xoxb-test', }; vi.mock('@copilotkit/outpost/shared', () => ({ @@ -919,6 +936,9 @@ describe('handleAiResponse', () => { kind: 'reply', ticketId: 'tkt-1', delivery: 'delivered', + // Pin the RESOLVED source; the handler used to forward the + // AI job's optional hint, yielding source: undefined. + source: 'discord', }), ); }); diff --git a/packages/outpost/queue/src/__tests__/queue.test.ts b/packages/outpost/queue/src/__tests__/queue.test.ts index ed539798..553a0f85 100644 --- a/packages/outpost/queue/src/__tests__/queue.test.ts +++ b/packages/outpost/queue/src/__tests__/queue.test.ts @@ -242,6 +242,9 @@ describe('Worker', () => { where: { id: 'job-1' }, data: expect.objectContaining({ status: 'DEAD_LETTER', + // The TRUE attempt count. Writing maxAttempts here would + // fabricate an exhausted-retry trail for a job that ran once. + attempts: 1, error: 'not_in_channel', }), }), diff --git a/packages/outpost/queue/src/handlers/__tests__/slack-mirror.test.ts b/packages/outpost/queue/src/handlers/__tests__/slack-mirror.test.ts index 79e19d7e..681fa588 100644 --- a/packages/outpost/queue/src/handlers/__tests__/slack-mirror.test.ts +++ b/packages/outpost/queue/src/handlers/__tests__/slack-mirror.test.ts @@ -22,8 +22,8 @@ import { handleSlackMirror, SLACK_MIRROR_PLUGIN } from '../slack-mirror.js'; import { readSlackMirrorConfig, isSlackMirrorEnabled, + canSlackMirrorPost, isMirrorableSource, - resetSlackMirrorWarnings, } from '@copilotkit/outpost/shared/platforms'; import type { SlackMirrorConfig } from '@copilotkit/outpost/shared/platforms'; @@ -34,7 +34,7 @@ const TICKET = { displayId: 'OUT-101', title: 'Sidebar crashes on mount', description: 'Repro: render CopilotSidebar with no props.', - source: 'GITHUB', + source: 'GITHUB_ISSUE', sourceUrl: 'https://github.com/CopilotKit/CopilotKit/issues/42', }; @@ -499,26 +499,28 @@ describe('handleSlackMirror', () => { }); }); -describe('isSlackMirrorEnabled — live requires a token', () => { - beforeEach(() => { - resetSlackMirrorWarnings(); +describe('canSlackMirrorPost — the consumer needs a token, the producers do not', () => { + // The producer gate must NOT require a token: the bots only enqueue, and + // demanding a Slack token there would either spread it to services that + // never post or leave the mirror silently dead. + it('stays enabled for the producers when live has no token', () => { + expect(isSlackMirrorEnabled({ mode: 'live', channelId: 'C1', token: null })).toBe(true); }); - // live + no token used to read as ENABLED, so producers enqueued jobs that - // could only throw in buildPoster and dead-letter, one per ticket, forever. - it('is disabled when live with no token, and says so once', () => { - const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + it('cannot post when live has no token', () => { + expect(canSlackMirrorPost({ mode: 'live', channelId: 'C1', token: null })).toBe(false); + }); - expect(isSlackMirrorEnabled({ mode: 'live', channelId: 'C1', token: null })).toBe(false); - expect(isSlackMirrorEnabled({ mode: 'live', channelId: 'C1', token: null })).toBe(false); + it('can post in shadow with no token, because shadow posts nothing', () => { + expect(canSlackMirrorPost({ mode: 'shadow', channelId: 'C1', token: null })).toBe(true); + }); - expect(errorSpy).toHaveBeenCalledTimes(1); - expect(errorSpy.mock.calls[0].join(' ')).toContain('SLACK_BOT_TOKEN'); - errorSpy.mockRestore(); + it('can post when live has a token', () => { + expect(canSlackMirrorPost({ mode: 'live', channelId: 'C1', token: 'xoxb-1' })).toBe(true); }); - it('is enabled when live with a token', () => { - expect(isSlackMirrorEnabled({ mode: 'live', channelId: 'C1', token: 'xoxb-1' })).toBe(true); + it('treats an empty channel id as unset', () => { + expect(isSlackMirrorEnabled({ mode: 'live', channelId: '', token: 'x' })).toBe(false); }); }); @@ -537,3 +539,129 @@ describe('isMirrorableSource', () => { }, ); }); + +describe('handleSlackMirror — permanent vs retryable', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockTicketFindUnique.mockResolvedValue(TICKET); + mockLinkFindUnique.mockResolvedValue(null); + mockLinkCreate.mockResolvedValue({}); + mockLinkUpdate.mockResolvedValue({}); + }); + + // The post landed but Slack gave us no ts. Retrying would post again. + it('does not retry when Slack accepts the post but returns no ts', async () => { + const postMessage = vi.fn().mockResolvedValue({}); + + const result = await handleSlackMirror({ ticketId: 'tkt-1', kind: 'ticket' }, context, { + config: liveConfig, + poster: { postMessage }, + }); + + expect(result.success).toBe(false); + expect(result.retryable).toBe(false); + expect(mockLinkCreate).not.toHaveBeenCalled(); + }); + + // A deleted ticket IS retryable on purpose: the producers enqueue outside the + // ticket's transaction, so a worker can legitimately arrive first. + it('leaves a missing ticket retryable', async () => { + mockTicketFindUnique.mockResolvedValue(null); + const { poster } = makePoster(); + + const result = await handleSlackMirror({ ticketId: 'tkt-gone', kind: 'ticket' }, context, { + config: liveConfig, + poster, + }); + + expect(result.success).toBe(false); + expect(result.retryable).toBeUndefined(); + }); + + it('rejects an unknown kind without posting', async () => { + const { poster, postMessage } = makePoster(); + + const result = await handleSlackMirror( + { ticketId: 'tkt-1', kind: 'bogus' as 'ticket' }, + context, + { config: liveConfig, poster }, + ); + + expect(result.success).toBe(false); + expect(result.retryable).toBe(false); + expect(postMessage).not.toHaveBeenCalled(); + }); + + it('reports live-without-token as a permanent misconfiguration', async () => { + const { poster, postMessage } = makePoster(); + + const result = await handleSlackMirror({ ticketId: 'tkt-1', kind: 'ticket' }, context, { + config: { mode: 'live', channelId: 'C0MIRROR', token: null }, + poster, + }); + + expect(result.success).toBe(false); + expect(result.retryable).toBe(false); + expect(result.error).toContain('SLACK_BOT_TOKEN'); + expect(postMessage).not.toHaveBeenCalled(); + }); +}); + +describe('handleSlackMirror — untrusted text', () => { + beforeEach(() => { + vi.clearAllMocks(); + mockTicketFindUnique.mockResolvedValue({ + ...TICKET, + title: 'Crash