diff --git a/doc/connections/SLACK-AUTOMATIC-SETUP.md b/doc/connections/SLACK-AUTOMATIC-SETUP.md index d22e53b30aa..f15ee5f91b5 100644 --- a/doc/connections/SLACK-AUTOMATIC-SETUP.md +++ b/doc/connections/SLACK-AUTOMATIC-SETUP.md @@ -104,8 +104,10 @@ 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. +comments. DM access settings and task controls still apply to threads inside DMs. +An explicit `/new` or `/close` starts the next task generation only after its +control response is delivered. Task completion alone does not release the binding. +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. diff --git a/maps/agent-access-map/chat-channels.md b/maps/agent-access-map/chat-channels.md index d9459d5727d..745d4de01b3 100644 --- a/maps/agent-access-map/chat-channels.md +++ b/maps/agent-access-map/chat-channels.md @@ -654,12 +654,7 @@ 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. 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: +apply. Top-level DM replies retain their linear conversation destination. 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 f33f55a0b36..679eb9265e7 100644 --- a/maps/feature-map/chat-channels.md +++ b/maps/feature-map/chat-channels.md @@ -101,15 +101,9 @@ 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 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. +top-level replies to top-level DMs. Channel messages and channel thread replies still +require a mention when that setting is enabled. Disable direct messages and +confirm a plain DM is rejected. Threads also work in DMs. 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 e397253549c..fdf64434fef 100644 --- a/server/src/__tests__/chat-channels.integration.test.ts +++ b/server/src/__tests__/chat-channels.integration.test.ts @@ -58500,7 +58500,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { ]); }); - it("binds Slack DM slash controls to the native root returned for the latest task", async () => { + it.each(["new", "close"] as const)("binds Slack DM slash controls to the native root returned for the latest task (%s)", async (control) => { const fixture = await seedCompany(); const { callbacks, endpoint, runtime, service } = await configuredSlackEndpoint(fixture); @@ -58564,7 +58564,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { threadId: starterThreadId, text: expect.stringMatching(/— todo$/), }); - await invoke("close"); + await invoke(control); await expect(service.listConversations(endpoint.id)).resolves.toEqual([ expect.objectContaining({ externalThreadId: starterThreadId, @@ -58578,9 +58578,21 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { state: "completed", }), ]); - expect( - await readIssuesByCompanyId(fixture.companyId), - ).toHaveLength(1); + expect(await readIssuesByCompanyId(fixture.companyId)).toHaveLength(1); + const providerTime = Date.now(); + const providerTimestamp = `${Math.floor(providerTime / 1_000)}.${String((providerTime % 1_000) * 1_000).padStart(6, "0")}`; + await deliverMessage({ + callbacks, endpointId: endpoint.id, + thread: makeThread({ channelId: "D09SLASHTASK", id: starterThreadId, isDM: true }).thread, + message: makeMessage({ id: providerTimestamp, raw: { ts: providerTimestamp }, text: "An explicitly requested new task", userId: "U-DM-SLASH-TASK" }), + trigger: "direct_message", + }); + const conversations = await service.listConversations(endpoint.id); + expect(conversations).toHaveLength(2); + expect(conversations.find(row => row.state === "active")).toMatchObject({ + externalThreadId: starterThreadId, sessionGeneration: 2, + }); + expect(await readIssuesByCompanyId(fixture.companyId)).toHaveLength(2); }); it("acknowledges a Slack retry while one durable worker owns the unrecorded command root", async () => { diff --git a/server/src/services/chat-channels.ts b/server/src/services/chat-channels.ts index e6cdc79894a..c41fe0af97e 100644 --- a/server/src/services/chat-channels.ts +++ b/server/src/services/chat-channels.ts @@ -1749,13 +1749,17 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) { let { conversation: existingConversation, issue: existingIssue, latestConversation, isLinear } = await resolveInboundConversationBinding(db, { endpoint, thread, - isLinear: () => endpoint.provider === "speko" ? SPEKO_CONVERSATION_IS_LINEAR : surfaceKind !== "native_thread", + isLinear: () => endpoint.provider === "speko" + ? SPEKO_CONVERSATION_IS_LINEAR + : surfaceKind !== "native_thread" || (endpoint.provider === "slack" && thread.isDM), lookupConversation: endpoint.provider === "microsoft-teams" ? () => conversationForThread(endpoint.id, thread.id) : undefined, conversationIsComplete: endpoint.provider === "imessage-photon" ? (conversationId) => photonConversationBindingIsComplete(conversationId, hasCommittedTaskControlCompletion) - : undefined, + : endpoint.provider === "slack" && thread.isDM && surfaceKind === "native_thread" + ? async (conversationId, state) => state === "completed" && hasCommittedTaskControlCompletion(conversationId) + : undefined, }); const telegramControls = endpoint.provider === "telegram" ? telegramInboundControls(message.text, surfaceKind) diff --git a/server/src/services/chat-channels/inbound/conversation-binding.ts b/server/src/services/chat-channels/inbound/conversation-binding.ts index f177aabd8e1..c79f87aae65 100644 --- a/server/src/services/chat-channels/inbound/conversation-binding.ts +++ b/server/src/services/chat-channels/inbound/conversation-binding.ts @@ -9,7 +9,7 @@ type ConversationBindingInput = { thread: { id: string; channelId: string }; isLinear: () => boolean; lookupConversation?: () => Promise; - conversationIsComplete?: (conversationId: string) => Promise; + conversationIsComplete?: (conversationId: string, state: ConversationRow["state"]) => Promise; }; export async function resolveInboundConversationBinding( @@ -54,7 +54,7 @@ export async function resolveInboundConversationBinding( isLinear && existingConversation && (conversationIsComplete - ? await conversationIsComplete(existingConversation.id) + ? await conversationIsComplete(existingConversation.id, existingConversation.state) : existingConversation.state === "completed" || existingIssue?.status === "done" || existingIssue?.status === "cancelled")