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

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 (🧵 / :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.
This commit is contained in:
teconomix 2026-03-12 21:26:46 +00:00 committed by Muhammed Mukhthar CM
parent 0511029071
commit bf084f21cd
2 changed files with 25 additions and 17 deletions

View File

@ -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,7 +319,7 @@ 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 = {
@ -323,9 +327,11 @@ describe("dispatchReplyFromConfig", () => {
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",
},

View File

@ -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();