fix: restore route threads from thread-scoped session keys only

teconomix · teconomix · commit 498de0fecf9e · 2026-03-12T21:26:46.000Z
Do not recover route thread ids from the normalised session store in
non-inbound reply paths. Store normalisation can fold origin.threadId
back into lastThreadId/deliveryContext, which resurrects stale thread
routing after delivery was intentionally cleared.

Instead, restore thread context only from:
- ctx.MessageThreadId (active inbound turn), or
- the active thread-scoped session key (:thread: / :topic:)

Also updates dispatch tests to verify that stale origin/store thread
metadata cannot override a non-thread session key, while a thread-scoped
session key still restores the correct route thread.
diff --git a/src/auto-reply/reply/dispatch-from-config.test.ts b/src/auto-reply/reply/dispatch-from-config.test.ts
@@ -113,11 +113,15 @@ vi.mock("../../logging/diagnostic.js", () => ({
   logMessageProcessed: diagnosticMocks.logMessageProcessed,
   logSessionStateChange: diagnosticMocks.logSessionStateChange,
 }));
-vi.mock("../../config/sessions.js", () => ({
-  loadSessionStore: sessionStoreMocks.loadSessionStore,
-  resolveStorePath: sessionStoreMocks.resolveStorePath,
-  resolveSessionStoreEntry: sessionStoreMocks.resolveSessionStoreEntry,
-}));
+vi.mock("../../config/sessions.js", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("../../config/sessions.js")>();
+  return {
+    ...actual,
+    loadSessionStore: sessionStoreMocks.loadSessionStore,
+    resolveStorePath: sessionStoreMocks.resolveStorePath,
+    resolveSessionStoreEntry: sessionStoreMocks.resolveSessionStoreEntry,
+  };
+});
 
 vi.mock("../../plugins/hook-runner-global.js", () => ({
   getGlobalHookRunner: () => hookMocks.runner,
@@ -315,17 +319,19 @@ describe("dispatchReplyFromConfig", () => {
     );
   });
 
-  it("falls back to session deliveryContext threadId when current ctx has no MessageThreadId", async () => {
+  it("falls back to thread-scoped session key when current ctx has no MessageThreadId", async () => {
     setNoAbort();
     mocks.routeReply.mockClear();
     sessionStoreMocks.currentEntry = {
       deliveryContext: {
         channel: "mattermost",
         to: "channel:CHAN1",
         accountId: "default",
-        threadId: "post-root",
       },
-      lastThreadId: "post-root",
+      origin: {
+        threadId: "stale-origin-root",
+      },
+      lastThreadId: "stale-origin-root",
     };
     const cfg = emptyConfig;
     const dispatcher = createDispatcher();
@@ -355,17 +361,16 @@ describe("dispatchReplyFromConfig", () => {
   it("does not resurrect a cleared route thread from origin metadata", async () => {
     setNoAbort();
     mocks.routeReply.mockClear();
-    // Simulate the real store: lastThreadId is normalised from origin.threadId when
-    // deliveryContext.threadId is absent. Using lastThreadId as a fallback would
-    // incorrectly revive stale thread routing.
+    // Simulate the real store: lastThreadId and deliveryContext.threadId may be normalised from
+    // origin.threadId on read, but a non-thread session key must still route to channel root.
     sessionStoreMocks.currentEntry = {
       deliveryContext: {
         channel: "mattermost",
         to: "channel:CHAN1",
         accountId: "default",
-        // threadId deliberately absent — the route was cleared
+        threadId: "stale-root",
       },
-      lastThreadId: "stale-root", // normalised from origin.threadId by loadSessionStore
+      lastThreadId: "stale-root",
       origin: {
         threadId: "stale-root",
       },
diff --git a/src/auto-reply/reply/dispatch-from-config.ts b/src/auto-reply/reply/dispatch-from-config.ts
@@ -2,6 +2,7 @@ import { resolveSessionAgentId } from "../../agents/agent-scope.js";
 import type { OpenClawConfig } from "../../config/config.js";
 import {
   loadSessionStore,
+  parseSessionThreadInfo,
   resolveSessionStoreEntry,
   resolveStorePath,
   type SessionEntry,
@@ -172,10 +173,12 @@ export async function dispatchReplyFromConfig(params: {
 
   const sessionStoreEntry = resolveSessionStoreLookup(ctx, cfg);
   const acpDispatchSessionKey = sessionStoreEntry.sessionKey ?? sessionKey;
-  // Only use the active delivery context's threadId. `lastThreadId` is not safe here because
-  // `loadSessionStore` normalises it from `origin.threadId`, which can outlive an intentionally
-  // cleared delivery route and would resurrect stale thread routing.
-  const routeThreadId = ctx.MessageThreadId ?? sessionStoreEntry.entry?.deliveryContext?.threadId;
+  // Restore route thread context only from the active turn or the thread-scoped session key.
+  // Do not read thread ids from the normalised session store here: `origin.threadId` can be
+  // folded back into lastThreadId/deliveryContext during store normalisation and resurrect a
+  // stale route after thread delivery was intentionally cleared.
+  const routeThreadId =
+    ctx.MessageThreadId ?? parseSessionThreadInfo(acpDispatchSessionKey).threadId;
   const inboundAudio = isInboundAudioContext(ctx);
   const sessionTtsAuto = normalizeTtsAutoMode(sessionStoreEntry.entry?.ttsAuto);
   const hookRunner = getGlobalHookRunner();
