From 78108efc0ca6b3c3ffa410e632a2840eb41c3231 Mon Sep 17 00:00:00 2001 From: cryppadotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 6 Nov 2026 15:50:27 +0000 Subject: [PATCH] fix(slack): retain DM thread tasks after completion Co-Authored-By: Paperclip --- doc/connections/SLACK-AUTOMATIC-SETUP.md | 8 ++ maps/agent-access-map/chat-channels.md | 7 +- maps/feature-map/chat-channels.md | 12 ++- .../chat-channels.integration.test.ts | 81 ++++++++++++++++++- .../chat-channels/inbound/authorization.ts | 1 + .../chat-channels/inbound/envelope.ts | 4 +- .../chat-channels/inbound/surfaces.ts | 6 +- .../services/chat-channels/slack/messages.ts | 6 +- 8 files changed, 114 insertions(+), 11 deletions(-) diff --git a/doc/connections/SLACK-AUTOMATIC-SETUP.md b/doc/connections/SLACK-AUTOMATIC-SETUP.md index 8ac45b0b234..d22e53b30aa 100644 --- a/doc/connections/SLACK-AUTOMATIC-SETUP.md +++ b/doc/connections/SLACK-AUTOMATIC-SETUP.md @@ -102,6 +102,14 @@ subscribes to the Slack thread, and starts the agent; later replies in that thre continue the same task without another mention. Normal account and channel access rules still apply, and repeated Slack deliveries do not create duplicate tasks. +A native Slack thread keeps the same task in channels and DMs, including after +the task is Done or Cancelled. A new reply reopens that task with its saved +comments. DM access settings still apply to threads inside DMs. Top-level DMs +keep their linear lifecycle and can start a new task after completion. +For a thread that was split before this fix, later replies continue its latest +saved task. Earlier tasks remain separate; this change does not merge history +or copy messages from the surrounding DM into a new thread. + Agents can ask interactive questions in Slack through Paperclip's saved `ask_user_questions` interactions. Native runners use `request_human_input`; legacy adapters use the issue-interactions API documented in the injected diff --git a/maps/agent-access-map/chat-channels.md b/maps/agent-access-map/chat-channels.md index 745d4de01b3..d9459d5727d 100644 --- a/maps/agent-access-map/chat-channels.md +++ b/maps/agent-access-map/chat-channels.md @@ -654,7 +654,12 @@ retrieval or telephone playback is established by the UI fixtures. Slack's **Require at-mention** policy applies to channel messages and channel thread replies, including queued work. DMs are exempt from mention checks; endpoint availability, **Allow direct messages**, and sender authorization still -apply. Top-level DM replies retain their linear conversation destination. Source: +apply. Top-level DM replies retain their linear conversation destination. Native +DM threads retain their task across completion while keeping the `direct_message` +resource type and the DM reach gate. Duplicate delivery does not add another +comment or wake. These cases have PostgreSQL fixture coverage in the +[chat integration suite](../../server/src/__tests__/chat-channels.integration.test.ts). +Source: [admission](../../server/src/services/chat-channels.ts) and [queued run policy](../../server/src/services/durable-chat-wakeup.ts). Live Slack verification of this exemption remains separate from provider-fixture coverage. diff --git a/maps/feature-map/chat-channels.md b/maps/feature-map/chat-channels.md index 977f0fb0300..f33f55a0b36 100644 --- a/maps/feature-map/chat-channels.md +++ b/maps/feature-map/chat-channels.md @@ -101,9 +101,15 @@ cover the database and prompt contracts; they do not prove live provider deliver For Slack, enable **Require at-mention** and **Allow direct messages**. Send a plain DM and follow up without a mention: both should reach the agent, with -top-level replies to top-level DMs. Channel messages and thread replies still -require a mention when that setting is enabled. Disable direct messages and -confirm a plain DM is rejected. Use a private channel for threaded conversations. +top-level replies to top-level DMs. Channel messages and channel thread replies +still require a mention when that setting is enabled. Finish a task inside a DM +thread, then reply in that same thread. The reply must reopen the same task and +retain its earlier comments. A different thread root must use a different task. +Disable direct messages and confirm both plain and threaded DMs are rejected. +The [completion regression](../../server/src/__tests__/chat-channels.integration.test.ts) +covers Done, Cancelled, completed bindings, duplicate delivery, and bare-DM and +channel-thread controls with PostgreSQL and provider fixtures. This is not live +Slack verification. For Slack acknowledgement latency, mention the bot with and without an attachment. Eyes should appear after authorized task/comment persistence, independently of diff --git a/server/src/__tests__/chat-channels.integration.test.ts b/server/src/__tests__/chat-channels.integration.test.ts index 644e2696c00..e397253549c 100644 --- a/server/src/__tests__/chat-channels.integration.test.ts +++ b/server/src/__tests__/chat-channels.integration.test.ts @@ -30531,6 +30531,82 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { await service.shutdown(); }); + it.each([ + { isDM: true, threaded: true, status: "done", state: "active" }, + { isDM: true, threaded: true, status: "cancelled", state: "active" }, + { isDM: true, threaded: true, status: "in_review", state: "completed" }, + { isDM: false, threaded: true, status: "done", state: "completed" }, + { isDM: true, threaded: false, status: "done", state: "completed" }, + ] as const)( + "preserves Slack thread history across completion ($isDM, $threaded, $status, $state)", + async ({ isDM, threaded, status, state }) => { + const fixture = await seedCompany(); + const { callbacks, endpoint, service, wakeup } = await configuredSlackEndpoint(fixture, { linkedBoardUser: true }); + const channelId = isDM ? "D-HISTORY" : "C-HISTORY"; + const thread = makeThread({ + channelId, + id: `slack:${channelId}:${threaded ? "1000.1" : ""}`, + isDM, + }).thread; + const send = (id: string, text: string) => deliverMessage({ + callbacks, endpointId: endpoint.id, thread, + message: makeMessage({ id, text, mentioned: !isDM, userId: "UBOARD" }), + trigger: isDM ? "direct_message" : "mention", + }); + await send("1000.1", "Keep this context for my next request"); + const [original] = await readChatConversationsByEndpointId(endpoint.id); + await db.insert(issueComments).values({ + companyId: fixture.companyId, issueId: original.issueId, + authorType: "agent", authorAgentId: fixture.assignedAgentId, + body: "I kept the original request.", + }); + await db.update(issues).set({ status }).where(eq(issues.id, original.issueId)); + await db.update(chatConversations).set({ state }).where(eq(chatConversations.id, original.id)); + + await send("1000.2", "Is it done?"); + await send("1000.2", "Is it done?"); + const conversations = await readChatConversationsByEndpointId(endpoint.id); + expect(conversations).toHaveLength(threaded ? 1 : 2); + const generation = threaded ? original.sessionGeneration : original.sessionGeneration + 1; + const continued = conversations.find(row => row.sessionGeneration === generation)!; + expect(continued.issueId === original.issueId).toBe(threaded); + expect(continued.state).toBe("active"); + expect((await issueService(db).getById(continued.issueId))?.status).toBe(status === "in_review" ? "in_review" : "todo"); + const comments = await readIssueCommentsByIssueId(continued.issueId) + .orderBy(asc(issueComments.createdAt), asc(issueComments.id)); + expect(comments.map(row => row.body)).toEqual(threaded + ? ["Keep this context for my next request", "I kept the original request.", "Is it done?"] + : ["Is it done?"]); + expect(wakeup).toHaveBeenCalledTimes(2); + const resources = await db.select().from(chatEndpointResources) + .where(eq(chatEndpointResources.endpointId, endpoint.id)); + expect(resources.find(row => row.providerResourceId === channelId)?.type).toBe(isDM ? "direct_message" : "channel"); + if (isDM) { + await service.update(endpoint.id, { allowDirectMessages: false }); + await send("1000.3", "This must be rejected"); + expect(wakeup).toHaveBeenCalledTimes(2); + expect(await readIssueCommentsByIssueId(continued.issueId)).toHaveLength(threaded ? 3 : 1); + } + }, + ); + + it("keeps separate Slack DM thread roots on separate tasks", async () => { + const fixture = await seedCompany(); + const { callbacks, endpoint } = await configuredSlackEndpoint(fixture); + for (const root of ["1000.1", "1000.2"]) { + await deliverMessage({ + callbacks, endpointId: endpoint.id, + thread: makeThread({ channelId: "D-ROOTS", id: `slack:D-ROOTS:${root}`, isDM: true }).thread, + message: makeMessage({ id: root, text: "A separate request" }), + trigger: "direct_message", + }); + } + const conversations = await readChatConversationsByEndpointId(endpoint.id); + expect(conversations).toHaveLength(2); + expect(new Set(conversations.map(row => row.issueId)).size).toBe(2); + expect(conversations.map(row => row.sessionGeneration)).toEqual([1, 1]); + }); + it("deduplicates inbound events, keeps one task per thread, and enables newly discovered Slack channels", async () => { const fixture = await seedCompany(); const { callbacks, endpoint, runtime, service, wakeup } = @@ -36290,10 +36366,11 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { await resumed.service.shutdown(); }); - it("returns two answered Slack turns to Idle in the same thread", async () => { + it.each([false, true])("returns two answered Slack turns to Idle in the same thread (DM=%s)", async (isDM) => { const fixture = await seedCompany(); const { callbacks, endpoint, service } = await configuredSlackEndpoint(fixture); - const thread = makeThread({ channelId: "C-IDLE", id: "slack:C-IDLE:4049.1", name: "idle" }); + const channelId = isDM ? "D-IDLE" : "C-IDLE"; + const thread = makeThread({ channelId, id: `slack:${channelId}:4049.1`, name: "idle", isDM }); let issueId: string | undefined; let conversationId: string | undefined; for (const [index, text] of ["@maya are you there?", "what is your name?"].entries()) { diff --git a/server/src/services/chat-channels/inbound/authorization.ts b/server/src/services/chat-channels/inbound/authorization.ts index cb3f330a3d3..2817378aeb8 100644 --- a/server/src/services/chat-channels/inbound/authorization.ts +++ b/server/src/services/chat-channels/inbound/authorization.ts @@ -373,6 +373,7 @@ export function createChatInboundAuthority(db: Db, options: InboundAuthorityOpti const type = providerResourceType( endpoint.provider, chatSurfaceKind(endpoint.provider, thread), + thread.isDM, ); const providerResourceId = canonicalProviderResourceId( endpoint.provider, diff --git a/server/src/services/chat-channels/inbound/envelope.ts b/server/src/services/chat-channels/inbound/envelope.ts index 8bc459e5c73..4fd2fc32a6a 100644 --- a/server/src/services/chat-channels/inbound/envelope.ts +++ b/server/src/services/chat-channels/inbound/envelope.ts @@ -263,7 +263,7 @@ export function createInboundEnvelope(db: Db, options: InboundEnvelopeOptions) { handle: message.author.userName, }, resource: { - type: providerResourceType(endpoint.provider, surfaceKind), + type: providerResourceType(endpoint.provider, surfaceKind, thread.isDM), providerResourceId: canonicalProviderResourceId( endpoint.provider, thread, @@ -340,7 +340,7 @@ export function createInboundEnvelope(db: Db, options: InboundEnvelopeOptions) { } : {}), resource: { - type: providerResourceType(endpoint.provider, surfaceKind), + type: providerResourceType(endpoint.provider, surfaceKind, thread.isDM), providerResourceId: canonicalProviderResourceId( endpoint.provider, thread, diff --git a/server/src/services/chat-channels/inbound/surfaces.ts b/server/src/services/chat-channels/inbound/surfaces.ts index 7f89d794c6b..22e8392004b 100644 --- a/server/src/services/chat-channels/inbound/surfaces.ts +++ b/server/src/services/chat-channels/inbound/surfaces.ts @@ -1,4 +1,4 @@ -import { slackThreadChannelId } from "../slack/messages.js"; +import { slackThreadChannelId, slackThreadRootMessageId } from "../slack/messages.js"; import type { EndpointRow } from "../runtime-types.js"; @@ -30,8 +30,9 @@ type ChatSurfaceKind = "direct_message" | "linear_group" | "native_thread"; function providerResourceType( provider: ChatProvider, surfaceKind: ChatSurfaceKind, + isDirectMessage: boolean, ): string { - if (surfaceKind === "direct_message") return "direct_message"; + if (isDirectMessage) return "direct_message"; if (provider === "github") return "repository"; if (provider === "imessage-photon") return "group_chat"; if (provider === "discord") return "channel"; @@ -116,6 +117,7 @@ function chatSurfaceKind( provider: ChatProvider, thread: Thread, ): ChatSurfaceKind { + if (provider === "slack" && slackThreadRootMessageId(thread.id)) return "native_thread"; if (thread.isDM) return "direct_message"; if (provider === "imessage-photon") return "linear_group"; if (provider === "telegram") { diff --git a/server/src/services/chat-channels/slack/messages.ts b/server/src/services/chat-channels/slack/messages.ts index e492cf1863f..36196eab18f 100644 --- a/server/src/services/chat-channels/slack/messages.ts +++ b/server/src/services/chat-channels/slack/messages.ts @@ -91,8 +91,12 @@ export function slackThreadChannelId(threadId: string): string | null { return channelId || null; } +export function slackThreadRootMessageId(threadId: string): string | null { + return /^slack:[^:]+:([^:]+)$/.exec(threadId)?.[1] ?? null; +} + export function slackOrphanFollowUp(threadId: string, messageId: string): boolean { - const rootMessageId = /^slack:[^:]+:(.+)$/.exec(threadId)?.[1]; + const rootMessageId = slackThreadRootMessageId(threadId); return Boolean(rootMessageId) && rootMessageId !== messageId; }