Skip to content

Commit 640a98d

Browse files
authored
Slack: sync direct messages, mentions, and saved items (#346)
* feat(twister): add Connector.hiddenChannels Declare that a connector's channels are internal detail rather than a user choice. Channels are still reported and mirrored so links keep their channel attribution and compose targets keep working, but no channel picker is shown and enabling or disabling a channel is not offered. Use it for connectors whose sync scope is decided by rules rather than by channel selection. * feat(slack): sync direct messages, mentions, and saved items only Channels are no longer selected or synced individually, and the connector declares hidden channels so no picker is shown. What arrives in Plot is: - Every direct message and group direct message. - Any thread containing a message that mentions you — directly, through a user group you belong to, or through @channel, @here or @everyone — wherever in the workspace it happens. - Any thread you save for later. Once a thread qualifies it keeps syncing. Unsaving it in Slack, or completing the to-do in Plot, changes state rather than unsubscribing, so a thread can never sit in Plot showing content that no longer matches Slack. Direct conversations are now one ongoing thread each, keyed on the person for a one-to-one and on the conversation for a group, instead of a new thread per message. Slack's own reply threads inside them become note replies. Channel threads keep Slack's threading unchanged. Replies written in Plot post to the message being answered, or top-level in a direct conversation, rather than nesting under an old anchor message. Removes the per-connection setting for grouping related messages into conversations, along with the auto-threading it gated: grouping is now structural and needs no configuration. Connections created under the previous model migrate on upgrade — per-channel webhooks and their state are retired, and the channel-token registry is renamed to describe what it actually holds.
1 parent be8543e commit 640a98d

10 files changed

Lines changed: 4611 additions & 511 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@plotday/twister": minor
3+
---
4+
5+
Added: `Connector.hiddenChannels` — declare that a connector's channels are internal detail rather than a user choice. Channels are still reported and mirrored so links keep their channel attribution, but no channel picker is shown and enable/disable is not offered. Use it for connectors whose sync scope is decided by rules rather than channel selection.

‎connectors/slack/src/slack-api.test.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,4 +52,18 @@ describe("transformSlackThread", () => {
5252
});
5353
expect(link.notes?.[0]?.author).toEqual(link.author);
5454
});
55+
56+
it("does not ask the platform to re-group the messages", () => {
57+
// Outside a direct conversation, Slack's own reply threads are a real,
58+
// user-visible grouping. Folding consecutive messages on top of that
59+
// would merge separate conversations that happen to run back to back.
60+
const link = transformSlackThread(
61+
[{ type: "message", ts: "3000.0001", user: "U1", text: "Standalone" }],
62+
"C123"
63+
);
64+
65+
expect(
66+
(link as unknown as { autoThread?: unknown }).autoThread
67+
).toBeUndefined();
68+
});
5569
});

‎connectors/slack/src/slack-api.ts‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type {
33
Action,
44
NewLinkWithNotes,
55
NewActor,
6+
NewContact,
67
NewReactions,
78
} from "@plotday/twister/plot";
89
import { slackNameToUnicode } from "./slack-emoji";
@@ -297,6 +298,22 @@ export class SlackApi {
297298
};
298299
}
299300

301+
/**
302+
* Ids of the user groups `userId` belongs to. Requires the `usergroups:read`
303+
* scope; callers must check the grant before calling, since a missing scope
304+
* surfaces as a permanent error that would flag the whole connection.
305+
*/
306+
public async getUserGroupsForUser(userId: string): Promise<string[]> {
307+
const data = await this.call("usergroups.list", { include_users: "true" });
308+
const groups = (data.usergroups ?? []) as Array<{
309+
id: string;
310+
users?: string[];
311+
}>;
312+
return groups
313+
.filter((group) => group.users?.includes(userId))
314+
.map((group) => group.id);
315+
}
316+
300317
public async getConversationHistory(
301318
channelId: string,
302319
cursor?: string,
@@ -569,7 +586,7 @@ export function slackUserInfoFromUser(user: SlackUser): SlackUserInfo {
569586
}
570587

571588
/**
572-
* Converts a Slack user ID to a NewActor. Always returns an actor: with no
589+
* Converts a Slack user ID to a NewContact. Always returns a contact: with no
573590
* resolvable name or email it falls back to the raw user id as the name.
574591
*
575592
* That fallback is a healing placeholder, not poison. `users.info` being
@@ -579,8 +596,14 @@ export function slackUserInfoFromUser(user: SlackUser): SlackUserInfo {
579596
* crediting the connection itself. The API treats a contact whose name equals
580597
* its own account id as unresolved and replaces it with the real name as soon
581598
* as `users.info` succeeds, so the id never sticks.
599+
*
600+
* Declared return type is `NewContact` (the contact-shaped half of the
601+
* `NewActor` union), not `NewActor` — every branch below already returns a
602+
* contact shape, so widening to the precise type lets callers that need a
603+
* guaranteed `name`/`email` (e.g. deriving a DM title or `accessContacts`)
604+
* use this directly instead of duplicating the fallback logic.
582605
*/
583-
function slackUserToNewActor(userId: string, info?: SlackUserInfo): NewActor {
606+
export function slackUserToNewActor(userId: string, info?: SlackUserInfo): NewContact {
584607
const source = {
585608
accountId: userId,
586609
...(info?.handle ? { descriptor: `@${info.handle}` } : {}),
@@ -735,14 +758,6 @@ export function transformSlackThread(
735758
notes: [],
736759
preview: firstText || null,
737760
...(initialSync ? { unread: false, archived: false } : {}),
738-
// Opt into sequential auto-threading, keyed on the channel. A no-op
739-
// unless the connection enabled it. 1:1 DMs (channel id "D…") read as one
740-
// continuous conversation, so fold every message into a single running
741-
// thread; channels and group DMs use the LLM continuation check.
742-
autoThread: {
743-
key: channelId,
744-
mode: channelId.startsWith("D") ? "fold" : "sequential",
745-
},
746761
};
747762

748763
// Create Notes for all messages (including first). Reactions live
Lines changed: 239 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,239 @@
1+
import { describe, expect, it } from "vitest";
2+
3+
import { assembleSlackDmLink } from "./slack-dm";
4+
import type { SlackMessage, SlackUserInfo } from "./slack-api";
5+
6+
function msg(ts: string, user: string, text: string, threadTs?: string): SlackMessage {
7+
return { type: "message", ts, user, text, ...(threadTs ? { thread_ts: threadTs } : {}) };
8+
}
9+
10+
describe("assembleSlackDmLink", () => {
11+
it("keys a 1:1 DM on the counterparty", () => {
12+
const link = assembleSlackDmLink({
13+
channelId: "D111",
14+
counterpartyUserId: "U222",
15+
messages: [msg("100.1", "U222", "hi")],
16+
initialSync: false,
17+
});
18+
19+
expect(link?.source).toBe("slack:person:U222");
20+
expect(link?.sources).toEqual(["slack:person:U222", "slack:chat:D111"]);
21+
expect(link?.type).toBe("dm");
22+
expect(link?.channelId).toBe("D111");
23+
expect(link?.meta?.direct).toBe(true);
24+
});
25+
26+
it("keys a group DM on the conversation", () => {
27+
const link = assembleSlackDmLink({
28+
channelId: "G333",
29+
counterpartyUserId: null,
30+
messages: [msg("100.1", "U222", "hi all")],
31+
initialSync: false,
32+
});
33+
34+
expect(link?.source).toBe("slack:chat:G333");
35+
expect(link?.sources).toEqual(["slack:chat:G333"]);
36+
});
37+
38+
it("creates one note per message, oldest first, keyed on ts", () => {
39+
const link = assembleSlackDmLink({
40+
channelId: "D111",
41+
counterpartyUserId: "U222",
42+
messages: [msg("102.0", "U222", "third"), msg("100.0", "U222", "first")],
43+
initialSync: false,
44+
});
45+
46+
expect(link?.notes?.map((n) => n.key)).toEqual(["100.0", "102.0"]);
47+
expect(link?.notes?.map((n) => n.content)).toEqual(["first", "third"]);
48+
});
49+
50+
it("records the newest message as meta.threadTs so a to-do star has something to target", () => {
51+
// A DM is one permanent, ever-growing link with no single message of its
52+
// own, so onThreadToDo needs a message to anchor a star on. Without this,
53+
// starring a synced (as opposed to composed-from-Plot) DM is a silent
54+
// no-op — onThreadToDo bails out whenever meta.threadTs is missing.
55+
const link = assembleSlackDmLink({
56+
channelId: "D111",
57+
counterpartyUserId: "U222",
58+
messages: [msg("100.0", "U222", "first"), msg("102.0", "U222", "third")],
59+
initialSync: false,
60+
});
61+
62+
expect(link?.meta?.threadTs).toBe("102.0");
63+
});
64+
65+
it("marks a threaded reply with reNote pointing at its parent", () => {
66+
const link = assembleSlackDmLink({
67+
channelId: "D111",
68+
counterpartyUserId: "U222",
69+
messages: [msg("100.0", "U222", "parent"), msg("101.0", "U222", "reply", "100.0")],
70+
initialSync: false,
71+
});
72+
73+
expect(link?.notes?.[0]?.reNote).toBeUndefined();
74+
expect(link?.notes?.[1]?.reNote).toEqual({ key: "100.0" });
75+
});
76+
77+
it("does not mark a thread parent as its own reply", () => {
78+
const link = assembleSlackDmLink({
79+
channelId: "D111",
80+
counterpartyUserId: "U222",
81+
messages: [msg("100.0", "U222", "parent", "100.0")],
82+
initialSync: false,
83+
});
84+
85+
expect(link?.notes?.[0]?.reNote).toBeUndefined();
86+
});
87+
88+
it("suppresses unread and archived state on initial sync", () => {
89+
const link = assembleSlackDmLink({
90+
channelId: "D111",
91+
counterpartyUserId: "U222",
92+
messages: [msg("100.0", "U222", "hi")],
93+
initialSync: true,
94+
});
95+
96+
expect(link?.unread).toBe(false);
97+
expect(link?.archived).toBe(false);
98+
});
99+
100+
it("returns null when there are no messages", () => {
101+
expect(
102+
assembleSlackDmLink({
103+
channelId: "D111",
104+
counterpartyUserId: "U222",
105+
messages: [],
106+
initialSync: false,
107+
})
108+
).toBeNull();
109+
});
110+
111+
it("derives title from counterparty's name when userInfos is provided", () => {
112+
const userInfos = new Map<string, SlackUserInfo>([
113+
["U222", { name: "Alice", email: "alice@example.com", handle: "alice" }],
114+
]);
115+
116+
const link = assembleSlackDmLink({
117+
channelId: "D111",
118+
counterpartyUserId: "U222",
119+
messages: [msg("100.0", "U222", "hello")],
120+
initialSync: false,
121+
userInfos,
122+
});
123+
124+
expect(link?.title).toBe("Alice");
125+
expect(link?.accessContacts?.[0]?.name).toBe("Alice");
126+
expect(link?.accessContacts?.[0]?.email).toBe("alice@example.com");
127+
});
128+
129+
it("sets a real author on every note", () => {
130+
const link = assembleSlackDmLink({
131+
channelId: "D111",
132+
counterpartyUserId: "U222",
133+
messages: [msg("100.0", "U222", "hi")],
134+
initialSync: false,
135+
});
136+
137+
expect(link?.notes?.[0]?.author).toBeDefined();
138+
expect(
139+
link?.notes?.[0]?.author && "source" in link.notes[0].author
140+
? link.notes[0].author.source?.accountId
141+
: undefined
142+
).toBe("U222");
143+
});
144+
145+
it("omits unread/archived (does not hardcode them) when initialSync is false", () => {
146+
const link = assembleSlackDmLink({
147+
channelId: "D111",
148+
counterpartyUserId: "U222",
149+
messages: [msg("100.0", "U222", "hi")],
150+
initialSync: false,
151+
});
152+
153+
expect(link).not.toHaveProperty("unread");
154+
expect(link).not.toHaveProperty("archived");
155+
});
156+
157+
it("still carries a counterparty contact and no clobbered title when userInfos hasn't resolved", () => {
158+
const link = assembleSlackDmLink({
159+
channelId: "D111",
160+
counterpartyUserId: "U222",
161+
messages: [msg("100.0", "U222", "hi")],
162+
initialSync: false,
163+
// No userInfos at all — a cold or rate-limited sync.
164+
});
165+
166+
// CRITICAL 1: accessContacts must not vanish just because users.info
167+
// hasn't resolved — it's the filing signal for link creation, which
168+
// only happens once.
169+
expect(link?.accessContacts).toBeDefined();
170+
expect(link?.accessContacts?.[0]?.source?.accountId).toBe("U222");
171+
172+
// CRITICAL 2: title must be omitted (preserving any existing stored
173+
// title) rather than clobbered with a placeholder — neither the raw
174+
// Slack user id nor a generic constant string.
175+
expect(link).not.toHaveProperty("title");
176+
});
177+
178+
it("carries file/image attachments as fileRef actions", () => {
179+
const message: SlackMessage = {
180+
type: "message",
181+
ts: "100.0",
182+
user: "U222",
183+
text: "check this out",
184+
files: [
185+
{
186+
id: "F123",
187+
name: "screenshot.png",
188+
mimetype: "image/png",
189+
size: 4096,
190+
original_w: 800,
191+
original_h: 600,
192+
},
193+
],
194+
};
195+
196+
const link = assembleSlackDmLink({
197+
channelId: "D111",
198+
counterpartyUserId: "U222",
199+
messages: [message],
200+
initialSync: false,
201+
});
202+
203+
expect(link?.notes?.[0]?.actions).toEqual([
204+
{
205+
type: "fileRef",
206+
ref: "F123",
207+
fileName: "screenshot.png",
208+
fileSize: 4096,
209+
mimeType: "image/png",
210+
imageWidth: 800,
211+
imageHeight: 600,
212+
},
213+
]);
214+
});
215+
216+
it("sets checkForTasks on every note", () => {
217+
const link = assembleSlackDmLink({
218+
channelId: "D111",
219+
counterpartyUserId: "U222",
220+
messages: [msg("100.0", "U222", "hi")],
221+
initialSync: false,
222+
});
223+
224+
expect(link?.notes?.[0]?.checkForTasks).toBe(true);
225+
});
226+
227+
it("sets a canonical Slack sourceUrl anchored on the latest message", () => {
228+
const link = assembleSlackDmLink({
229+
channelId: "D111",
230+
counterpartyUserId: "U222",
231+
messages: [msg("100.0", "U222", "first"), msg("102.0", "U222", "second")],
232+
initialSync: false,
233+
});
234+
235+
expect(link?.sourceUrl).toBe(
236+
"https://slack.com/app_redirect?channel=D111&message_ts=102.0"
237+
);
238+
});
239+
});

0 commit comments

Comments
 (0)