mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 01:50:13 +01:00
fix(slack): retain DM thread tasks after completion
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
eb5c24b61c
commit
78108efc0c
8 files changed
+114
-11
No files matched your search
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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()) {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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") {
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user