fix: slack connector getParents should return parents for threads

connectors/src/connectors/slack/index.ts

@@ -24,8 +24,13 @@ import {
   getSlackClient,
 } from "@connectors/connectors/slack/lib/slack_client";
 import {
-  internalIdFromSlackChannelId,
+  isSlackChannelInternalId,
+  isSlackNonThreadedMessagesInternalId,
+  isSlackThreadInternalId,
   slackChannelIdFromInternalId,
+  slackChannelIdFromSlackNonThreadedMessagesInternalId,
+  slackChannelInternalIdFromSlackChannelId,
+  slackThreadIdentifierFromSlackThreadInternalId,
 } from "@connectors/connectors/slack/lib/utils";
 import { launchSlackSyncWorkflow } from "@connectors/connectors/slack/temporal/client.js";
 import {
@@ -381,7 +386,7 @@ export class SlackConnectorManager extends BaseConnectorManager<SlackConfigurati
 
       const resources: ContentNode[] = slackChannels.map((ch) => ({
         provider: "slack",
-        internalId: internalIdFromSlackChannelId(ch.slackChannelId),
+        internalId: slackChannelInternalIdFromSlackChannelId(ch.slackChannelId),
         parentInternalId: null,
         type: "channel",
         title: `#${ch.slackChannelName}`,
@@ -596,7 +601,7 @@ export class SlackConnectorManager extends BaseConnectorManager<SlackConfigurati
 
     const contentNodes: ContentNode[] = channels.map((ch) => ({
       provider: "slack",
-      internalId: internalIdFromSlackChannelId(ch.slackChannelId),
+      internalId: slackChannelInternalIdFromSlackChannelId(ch.slackChannelId),
       parentInternalId: null,
       type: "channel",
       title: `#${ch.slackChannelName}`,
@@ -617,9 +622,32 @@ export class SlackConnectorManager extends BaseConnectorManager<SlackConfigurati
   }: {
     internalId: string;
   }): Promise<Result<string[], Error>> {
-    // We only ever return permissions at the slack channel level so the parents are always the
-    // internalId itself.
-    return new Ok([internalId]);
+    // If the internal ID is a Slack channel ID, it has no other parent
+    if (isSlackChannelInternalId(internalId)) {
+      return new Ok([internalId]);
+    }
+    // If it is a slack thread, or a slack "non-threaded" message document, it also
+    // needs the channel internal ID as parent
+    else if (isSlackThreadInternalId(internalId)) {
+      const { channelId } =
+        slackThreadIdentifierFromSlackThreadInternalId(internalId);
+      return new Ok([
+        internalId,
+        slackChannelInternalIdFromSlackChannelId(channelId),
+      ]);
+    } else if (isSlackNonThreadedMessagesInternalId(internalId)) {
+      const channelId =
+        slackChannelIdFromSlackNonThreadedMessagesInternalId(internalId);
+      return new Ok([
+        internalId,
+        slackChannelInternalIdFromSlackChannelId(channelId),
+      ]);
+    }
+    // This in theory shouldn't happen
+    else {
+      logger.warn({ internalId }, "Unknown internal ID for Slack connector");
+      return new Ok([internalId]);
+    }
   }
 
   async setConfigurationKey({

connectors/src/connectors/slack/lib/utils.ts

@@ -51,10 +51,81 @@ export const timeAgoFrom = (millisSinceEpoch: number) => {
   return seconds + "s";
 };
 
-export function internalIdFromSlackChannelId(channel: string) {
+export type SlackChannelInternalId = string;
+export type SlackThreadInternalId = string;
+export type SlackNonThreadedMessagesInternalId = string;
+
+export function isSlackChannelInternalId(
+  internalId: string
+): internalId is SlackChannelInternalId {
+  return internalId.startsWith("slack-channel-");
+}
+
+export function isSlackThreadInternalId(
+  internalId: string
+): internalId is SlackThreadInternalId {
+  return internalId.startsWith("slack-") && internalId.includes("-thread-");
+}
+
+export function isSlackNonThreadedMessagesInternalId(
+  internalId: string
+): internalId is SlackNonThreadedMessagesInternalId {
+  return internalId.startsWith("slack-") && internalId.includes("-messages-");
+}
+
+export function slackChannelInternalIdFromSlackChannelId(
+  channel: string
+): SlackChannelInternalId {
   return `slack-channel-${_.last(channel.split("slack-channel-"))!}`;
 }
 
-export function slackChannelIdFromInternalId(nodeId: string) {
+export function slackChannelIdFromInternalId(nodeId: SlackChannelInternalId) {
   return _.last(nodeId.split("slack-channel-"))!;
 }
+
+export type SlackThreadIdentifier = {
+  channelId: string;
+  threadTs: string;
+};
+
+export function slackThreadInternalIdFromSlackThreadIdentifier({
+  channelId,
+  threadTs,
+}: SlackThreadIdentifier): SlackThreadInternalId {
+  return `slack-${channelId}-thread-${threadTs}`;
+}
+
+export function slackThreadIdentifierFromSlackThreadInternalId(
+  internalId: SlackThreadInternalId
+): SlackThreadIdentifier {
+  const parts = internalId.split("-thread-");
+  const channelId = _.last(parts[0]!.split("slack-"))!;
+  const threadTs = parts[1];
+  return {
+    channelId,
+    threadTs: threadTs!,
+  };
+}
+
+export type SlackNonThreadedMessagesIdentifier = {
+  channelId: string;
+  startDate: Date;
+  endDate: Date;
+};
+
+export function slackNonThreadedMessagesInternalIdFromSlackNonThreadedMessagesIdentifier({
+  channelId,
+  startDate,
+  endDate,
+}: SlackNonThreadedMessagesIdentifier): SlackNonThreadedMessagesInternalId {
+  const startDateStr = `${startDate.getFullYear()}-${startDate.getMonth()}-${startDate.getDate()}`;
+  const endDateStr = `${endDate.getFullYear()}-${endDate.getMonth()}-${endDate.getDate()}`;
+  return `slack-${channelId}-messages-${startDateStr}-${endDateStr}`;
+}
+
+export function slackChannelIdFromSlackNonThreadedMessagesInternalId(
+  internalId: SlackNonThreadedMessagesInternalId
+): string {
+  const parts = internalId.split("-messages-");
+  return _.last(parts[0]!.split("slack-"))!;
+}

connectors/src/connectors/slack/temporal/activities.ts

@@ -27,7 +27,9 @@ import { getRepliesFromThread } from "@connectors/connectors/slack/lib/thread";
 import {
   getWeekEnd,
   getWeekStart,
-  internalIdFromSlackChannelId,
+  slackChannelInternalIdFromSlackChannelId,
+  slackNonThreadedMessagesInternalIdFromSlackNonThreadedMessagesIdentifier,
+  slackThreadInternalIdFromSlackThreadIdentifier,
 } from "@connectors/connectors/slack/lib/utils";
 import { dataSourceConfigFromConnector } from "@connectors/lib/api/data_source_config";
 import { cacheGet, cacheSet } from "@connectors/lib/cache";
@@ -248,10 +250,10 @@ export async function syncChannel(
   if (!messagesCursor) {
     await upsertDataSourceFolder({
       dataSourceConfig,
-      folderId: internalIdFromSlackChannelId(channelId),
+      folderId: slackChannelInternalIdFromSlackChannelId(channelId),
       title: `#${channel.name}`,
       parentId: null,
-      parents: [internalIdFromSlackChannelId(channelId)],
+      parents: [slackChannelInternalIdFromSlackChannelId(channelId)],
       mimeType: "application/vnd.dust.slack.channel",
     });
   }
@@ -561,9 +563,12 @@ export async function syncNonThreaded(
 
   const startDate = new Date(startTsMs);
   const endDate = new Date(endTsMs);
-  const startDateStr = `${startDate.getFullYear()}-${startDate.getMonth()}-${startDate.getDate()}`;
-  const endDateStr = `${endDate.getFullYear()}-${endDate.getMonth()}-${endDate.getDate()}`;
-  const documentId = `slack-${channelId}-messages-${startDateStr}-${endDateStr}`;
+  const documentId =
+    slackNonThreadedMessagesInternalIdFromSlackNonThreadedMessagesIdentifier({
+      channelId,
+      startDate,
+      endDate,
+    });
   const firstMessage = messages[0];
   let sourceUrl: string | undefined = undefined;
 
@@ -621,8 +626,8 @@ export async function syncNonThreaded(
     documentUrl: sourceUrl,
     timestampMs: updatedAt,
     tags,
-    parentId: internalIdFromSlackChannelId(channelId),
-    parents: [documentId, internalIdFromSlackChannelId(channelId)],
+    parentId: slackChannelInternalIdFromSlackChannelId(channelId),
+    parents: [documentId, slackChannelInternalIdFromSlackChannelId(channelId)],
     upsertContext: {
       sync_type: isBatchSync ? "batch" : "incremental",
     },
@@ -752,7 +757,10 @@ export async function syncThread(
     "syncThread.getRepliesFromThread.done"
   );
 
-  const documentId = `slack-${channelId}-thread-${threadTs}`;
+  const documentId = slackThreadInternalIdFromSlackThreadIdentifier({
+    channelId,
+    threadTs,
+  });
 
   const botUserId = await getBotUserIdMemoized(connectorId);
   allMessages = allMessages.filter((m) => m.user !== botUserId);
@@ -832,8 +840,8 @@ export async function syncThread(
     documentUrl: sourceUrl,
     timestampMs: updatedAt,
     tags,
-    parentId: internalIdFromSlackChannelId(channelId),
-    parents: [documentId, internalIdFromSlackChannelId(channelId)],
+    parentId: slackChannelInternalIdFromSlackChannelId(channelId),
+    parents: [documentId, slackChannelInternalIdFromSlackChannelId(channelId)],
     upsertContext: {
       sync_type: isBatchSync ? "batch" : "incremental",
     },
@@ -1191,7 +1199,7 @@ export async function deleteChannel(channelId: string, connectorId: ModelId) {
 
   await deleteDataSourceFolder({
     dataSourceConfig,
-    folderId: internalIdFromSlackChannelId(channelId),
+    folderId: slackChannelInternalIdFromSlackChannelId(channelId),
   });
 
   logger.info(