fix(slack): canonicalize inherited thread replies

Peter Steinberger · Peter Steinberger · commit adb1979ca3a7 · 2026-06-30T16:19:41.000-07:00
diff --git a/extensions/slack/src/channel.test.ts b/extensions/slack/src/channel.test.ts
@@ -475,77 +475,6 @@ describe("slackPlugin status", () => {
     });
   });
 
-  it("prefers the current Slack thread session when reply is inherited (replyToIsExplicit=false)", async () => {
-    const resolveRoute = slackPlugin.messaging?.resolveOutboundSessionRoute;
-    if (!resolveRoute) {
-      throw new Error("slack messaging.resolveOutboundSessionRoute unavailable");
-    }
-
-    const route = await resolveRoute({
-      cfg: {} as OpenClawConfig,
-      agentId: "main",
-      target: "channel:C1",
-      currentSessionKey: "agent:main:slack:channel:C1:thread:1712345678.123456",
-      replyToId: "1712345688.654321",
-      replyToIsExplicit: false,
-    });
-
-    // Inherited child reply timestamp should not create a new session;
-    // it should route into the existing thread session.
-    expectRecordFields(route, "Slack route (inherited reply)", {
-      sessionKey: "agent:main:slack:channel:c1:thread:1712345678.123456",
-      baseSessionKey: "agent:main:slack:channel:c1",
-      threadId: "1712345678.123456",
-    });
-  });
-
-  it("keeps explicit reply-to precedence when replyToIsExplicit=true", async () => {
-    const resolveRoute = slackPlugin.messaging?.resolveOutboundSessionRoute;
-    if (!resolveRoute) {
-      throw new Error("slack messaging.resolveOutboundSessionRoute unavailable");
-    }
-
-    const route = await resolveRoute({
-      cfg: {} as OpenClawConfig,
-      agentId: "main",
-      target: "channel:C1",
-      currentSessionKey: "agent:main:slack:channel:C1:thread:1712345678.123456",
-      replyToId: "1712345688.654321",
-      replyToIsExplicit: true,
-    });
-
-    // Explicit reply targets should keep the current reply-first behavior.
-    expectRecordFields(route, "Slack route (explicit reply)", {
-      sessionKey: "agent:main:slack:channel:c1:thread:1712345688.654321",
-      baseSessionKey: "agent:main:slack:channel:c1",
-      threadId: "1712345688.654321",
-    });
-  });
-
-  it("preserves reply-first routing when replyToIsExplicit is omitted (backward compat)", async () => {
-    const resolveRoute = slackPlugin.messaging?.resolveOutboundSessionRoute;
-    if (!resolveRoute) {
-      throw new Error("slack messaging.resolveOutboundSessionRoute unavailable");
-    }
-
-    const route = await resolveRoute({
-      cfg: {} as OpenClawConfig,
-      agentId: "main",
-      target: "channel:C1",
-      currentSessionKey: "agent:main:slack:channel:C1:thread:1712345678.123456",
-      replyToId: "1712345688.654321",
-      // replyToIsExplicit intentionally omitted
-    });
-
-    // Callers that haven't opted into the tri-state signal (e.g. gateway
-    // send with an explicit replyToId) must keep reply-first routing.
-    expectRecordFields(route, "Slack route (omitted explicitness)", {
-      sessionKey: "agent:main:slack:channel:c1:thread:1712345688.654321",
-      baseSessionKey: "agent:main:slack:channel:c1",
-      threadId: "1712345688.654321",
-    });
-  });
-
   it("canonicalizes bare Slack IM channel targets to direct user session routes", async () => {
     const resolveRoute = slackPlugin.messaging?.resolveOutboundSessionRoute;
     if (!resolveRoute) {
@@ -945,6 +874,33 @@ describe("slackPlugin outbound", () => {
     });
   });
 
+  it.each([
+    {
+      name: "inherited",
+      replyToIsExplicit: false,
+      expectedReplyToId: "1712345678.123456",
+    },
+    { name: "explicit", replyToIsExplicit: true, expectedReplyToId: "1712345688.654321" },
+    { name: "unknown", replyToIsExplicit: undefined, expectedReplyToId: "1712345688.654321" },
+  ])(
+    "routes $name child replies to $expectedReplyToId",
+    ({ replyToIsExplicit, expectedReplyToId }) => {
+      const resolveReplyTransport = slackPlugin.threading?.resolveReplyTransport;
+      if (!resolveReplyTransport) {
+        throw new Error("slack threading.resolveReplyTransport unavailable");
+      }
+
+      expect(
+        resolveReplyTransport({
+          cfg,
+          replyToId: "1712345688.654321",
+          threadId: "1712345678.123456",
+          replyToIsExplicit,
+        }),
+      ).toEqual({ replyToId: expectedReplyToId, threadId: null });
+    },
+  );
+
   it("ignores explicit reply targets for off-mode final delivery", () => {
     const resolveReplyTransport = slackPlugin.threading?.resolveReplyTransport;
     if (!resolveReplyTransport) {
diff --git a/extensions/slack/src/channel.ts b/extensions/slack/src/channel.ts
@@ -313,7 +313,6 @@ async function resolveSlackOutboundSessionRoute(params: {
   accountId?: string | null;
   target: string;
   replyToId?: string | null;
-  replyToIsExplicit?: boolean;
   threadId?: string | number | null;
   currentSessionKey?: string | null;
 }) {
@@ -375,16 +374,6 @@ async function resolveSlackOutboundSessionRoute(params: {
     replyToId: params.replyToId,
     threadId: params.threadId,
     currentSessionKey: params.currentSessionKey,
-    // Inherited Slack child-reply timestamps should not create new
-    // delivery-mirror sessions; prefer the recovered thread session
-    // when the reply target was explicitly marked as inherited.
-    // Omitted (undefined) explicitness preserves the old reply-first
-    // default for callers that haven't opted into the tri-state signal
-    // (e.g. gateway send with an explicit request replyToId).
-    precedence:
-      params.replyToIsExplicit === false
-        ? ["currentSession", "threadId", "replyToId"]
-        : ["replyToId", "threadId", "currentSession"],
     canRecoverCurrentThread: () =>
       shouldRecoverSlackThreadFromCurrentSession({
         cfg: params.cfg,
@@ -829,10 +818,14 @@ export const slackPlugin: ChannelPlugin<ResolvedSlackAccount, SlackProbe> = crea
               toolContext,
             }),
           ),
-    resolveReplyTransport: ({ threadId, replyToId, replyDelivery }) => {
+    resolveReplyTransport: ({ threadId, replyToId, replyToIsExplicit, replyDelivery }) => {
+      const allowedReplyToId = replyDelivery?.replyToMode === "off" ? undefined : replyToId;
+      // Slack's thread_ts identifies the root. Only known inherited replies may let
+      // that root replace a child timestamp; explicit and unknown callers stay reply-first.
+      const preferThreadId = replyToIsExplicit === false;
       const resolvedReplyToId = resolveSlackThreadTsValue({
-        replyToId: replyDelivery?.replyToMode === "off" ? undefined : replyToId,
-        threadId,
+        replyToId: preferThreadId ? threadId : allowedReplyToId,
+        threadId: preferThreadId ? allowedReplyToId : threadId,
       });
       return {
         replyToId:
diff --git a/src/channels/plugins/types.core.ts b/src/channels/plugins/types.core.ts
@@ -649,8 +649,6 @@ export type ChannelMessagingAdapter = {
       source: "normalized" | "directory";
     };
     replyToId?: string | null;
-    /** True when replyToId came from an explicit action-payload replyTo field. */
-    replyToIsExplicit?: boolean;
     threadId?: string | number | null;
   }) => ChannelOutboundSessionRoute | Promise<ChannelOutboundSessionRoute | null> | null;
 };
diff --git a/src/infra/outbound/message-action-runner.threading.test.ts b/src/infra/outbound/message-action-runner.threading.test.ts
@@ -123,14 +123,12 @@ describe("message action threading helpers", () => {
         threadId: threadId ?? null,
       }),
       resolveOutboundSessionRoute,
-      replyToIsExplicit: true,
       ensureOutboundSessionEntry,
     });
 
     expect(resolveOutboundSessionRoute).toHaveBeenCalledOnce();
     expect(firstMockArg(resolveOutboundSessionRoute)).toMatchObject({
       replyToId: "root-42",
-      replyToIsExplicit: true,
       threadId: "root-42",
     });
   });
diff --git a/src/infra/outbound/message-action-threading.ts b/src/infra/outbound/message-action-threading.ts
@@ -211,7 +211,6 @@ export async function prepareOutboundMirrorRoute(params: {
           currentSessionKey: params.currentSessionKey,
           resolvedTarget: params.resolvedTarget,
           replyToId,
-          replyToIsExplicit: params.replyToIsExplicit,
           threadId: resolvedThreadId,
         })
       : null;
diff --git a/src/infra/outbound/outbound-session.ts b/src/infra/outbound/outbound-session.ts
@@ -39,8 +39,6 @@ export type ResolveOutboundSessionRouteParams = {
   currentSessionKey?: string;
   resolvedTarget?: ResolvedMessagingTarget;
   replyToId?: string | null;
-  /** True when replyToId came from an explicit action-payload replyTo field. */
-  replyToIsExplicit?: boolean;
   threadId?: string | number | null;
 };
 
