From 5e73f3344807a0b16d22073142d573afde121881 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sun, 22 Feb 2026 14:39:00 -0500 Subject: [PATCH] fix(slack): keep thread session fork/history context after first turn (#23843) * Slack thread sessions: keep forking and history context after first turn * Update CHANGELOG.md --- CHANGELOG.md | 1 + .../reply/get-reply-run.media-only.test.ts | 14 ++++ src/auto-reply/reply/get-reply-run.ts | 11 ++- src/auto-reply/reply/session.test.ts | 75 +++++++++++++++++++ src/auto-reply/reply/session.ts | 6 +- src/config/sessions/types.ts | 2 + .../monitor/message-handler/prepare.test.ts | 23 ++++-- src/slack/monitor/message-handler/prepare.ts | 2 +- 8 files changed, 120 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddb6c4431e8..f579d47b16f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Slack/Threading: sessions: keep parent-session forking and thread-history context active beyond first turn by removing first-turn-only gates in session init, thread-history fetch, and reply prompt context injection. (#23843, #23090) Thanks @vincentkoc and @Taskle. - Slack/Threading: respect `replyToMode` when Slack auto-populates top-level `thread_ts`, and ignore inline `replyToId` directive tags when `replyToMode` is `off` so thread forcing stays disabled unless explicitly configured. (#23839, #23320, #23513) Thanks @vincentkoc and @dorukardahan. - Slack/Extension: forward `message read` `threadId` to `readMessages` and use delivery-context `threadId` as outbound `thread_ts` fallback so extension replies/reads stay in the correct Slack thread. (#22216, #22485, #23836) Thanks @vincentkoc, @lan17 and @dorukardahan. - Channels/Group policy: fail closed when `groupPolicy: "allowlist"` is set without explicit `groups`, honor account-level `groupPolicy` overrides, and enforce `groupPolicy: "disabled"` as a hard group block. (#22215) Thanks @etereo. diff --git a/src/auto-reply/reply/get-reply-run.media-only.test.ts b/src/auto-reply/reply/get-reply-run.media-only.test.ts index f7edf2aa31f..0fde3c4686a 100644 --- a/src/auto-reply/reply/get-reply-run.media-only.test.ts +++ b/src/auto-reply/reply/get-reply-run.media-only.test.ts @@ -169,6 +169,20 @@ describe("runPreparedReply media-only handling", () => { expect(call?.followupRun.prompt).toContain("[User sent media without caption]"); }); + it("keeps thread history context on follow-up turns", async () => { + const result = await runPreparedReply( + baseParams({ + isNewSession: false, + }), + ); + expect(result).toEqual({ text: "ok" }); + + const call = vi.mocked(runReplyAgent).mock.calls[0]?.[0]; + expect(call).toBeTruthy(); + expect(call?.followupRun.prompt).toContain("[Thread history - for context]"); + expect(call?.followupRun.prompt).toContain("Earlier message in this thread"); + }); + it("returns the empty-body reply when there is no text and no media", async () => { const result = await runPreparedReply( baseParams({ diff --git a/src/auto-reply/reply/get-reply-run.ts b/src/auto-reply/reply/get-reply-run.ts index bcbaf72f563..e12342efcdc 100644 --- a/src/auto-reply/reply/get-reply-run.ts +++ b/src/auto-reply/reply/get-reply-run.ts @@ -260,12 +260,11 @@ export async function runPreparedReply( prefixedBodyBase = appendUntrustedContext(prefixedBodyBase, sessionCtx.UntrustedContext); const threadStarterBody = ctx.ThreadStarterBody?.trim(); const threadHistoryBody = ctx.ThreadHistoryBody?.trim(); - const threadContextNote = - isNewSession && threadHistoryBody - ? `[Thread history - for context]\n${threadHistoryBody}` - : isNewSession && threadStarterBody - ? `[Thread starter - for context]\n${threadStarterBody}` - : undefined; + const threadContextNote = threadHistoryBody + ? `[Thread history - for context]\n${threadHistoryBody}` + : threadStarterBody + ? `[Thread starter - for context]\n${threadStarterBody}` + : undefined; const skillResult = await ensureSkillSnapshot({ sessionEntry, sessionStore, diff --git a/src/auto-reply/reply/session.test.ts b/src/auto-reply/reply/session.test.ts index 5ac167fd667..aa442adb4eb 100644 --- a/src/auto-reply/reply/session.test.ts +++ b/src/auto-reply/reply/session.test.ts @@ -126,6 +126,81 @@ describe("initSessionState thread forking", () => { warn.mockRestore(); }); + it("forks from parent when thread session key already exists but was not forked yet", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const root = await makeCaseDir("openclaw-thread-session-existing-"); + const sessionsDir = path.join(root, "sessions"); + await fs.mkdir(sessionsDir); + + const parentSessionId = "parent-session"; + const parentSessionFile = path.join(sessionsDir, "parent.jsonl"); + const header = { + type: "session", + version: 3, + id: parentSessionId, + timestamp: new Date().toISOString(), + cwd: process.cwd(), + }; + const message = { + type: "message", + id: "m1", + parentId: null, + timestamp: new Date().toISOString(), + message: { role: "user", content: "Parent prompt" }, + }; + await fs.writeFile( + parentSessionFile, + `${JSON.stringify(header)}\n${JSON.stringify(message)}\n`, + "utf-8", + ); + + const storePath = path.join(root, "sessions.json"); + const parentSessionKey = "agent:main:slack:channel:c1"; + const threadSessionKey = "agent:main:slack:channel:c1:thread:123"; + await saveSessionStore(storePath, { + [parentSessionKey]: { + sessionId: parentSessionId, + sessionFile: parentSessionFile, + updatedAt: Date.now(), + }, + [threadSessionKey]: { + sessionId: "preseed-thread-session", + updatedAt: Date.now(), + }, + }); + + const cfg = { + session: { store: storePath }, + } as OpenClawConfig; + + const first = await initSessionState({ + ctx: { + Body: "Thread reply", + SessionKey: threadSessionKey, + ParentSessionKey: parentSessionKey, + }, + cfg, + commandAuthorized: true, + }); + + expect(first.sessionEntry.sessionId).not.toBe("preseed-thread-session"); + expect(first.sessionEntry.forkedFromParent).toBe(true); + + const second = await initSessionState({ + ctx: { + Body: "Thread reply 2", + SessionKey: threadSessionKey, + ParentSessionKey: parentSessionKey, + }, + cfg, + commandAuthorized: true, + }); + + expect(second.sessionEntry.sessionId).toBe(first.sessionEntry.sessionId); + expect(second.sessionEntry.forkedFromParent).toBe(true); + warn.mockRestore(); + }); + it("records topic-specific session files when MessageThreadId is present", async () => { const root = await makeCaseDir("openclaw-topic-session-"); const storePath = path.join(root, "sessions.json"); diff --git a/src/auto-reply/reply/session.ts b/src/auto-reply/reply/session.ts index e9bf4b26083..f644f47aa34 100644 --- a/src/auto-reply/reply/session.ts +++ b/src/auto-reply/reply/session.ts @@ -336,11 +336,12 @@ export async function initSessionState(params: { sessionEntry.displayName = threadLabel; } const parentSessionKey = ctx.ParentSessionKey?.trim(); + const alreadyForked = sessionEntry.forkedFromParent === true; if ( - isNewSession && parentSessionKey && parentSessionKey !== sessionKey && - sessionStore[parentSessionKey] + sessionStore[parentSessionKey] && + !alreadyForked ) { log.warn( `forking from parent session: parentKey=${parentSessionKey} → sessionKey=${sessionKey} ` + @@ -355,6 +356,7 @@ export async function initSessionState(params: { sessionId = forked.sessionId; sessionEntry.sessionId = forked.sessionId; sessionEntry.sessionFile = forked.sessionFile; + sessionEntry.forkedFromParent = true; log.warn(`forked session created: file=${forked.sessionFile}`); } } diff --git a/src/config/sessions/types.ts b/src/config/sessions/types.ts index 10d1d3bc54b..25091cd065e 100644 --- a/src/config/sessions/types.ts +++ b/src/config/sessions/types.ts @@ -35,6 +35,8 @@ export type SessionEntry = { sessionFile?: string; /** Parent session key that spawned this session (used for sandbox session-tool scoping). */ spawnedBy?: string; + /** True after a thread/topic session has been forked from its parent transcript once. */ + forkedFromParent?: boolean; /** Subagent spawn depth (0 = main, 1 = sub-agent, 2 = sub-sub-agent). */ spawnDepth?: number; systemSent?: boolean; diff --git a/src/slack/monitor/message-handler/prepare.test.ts b/src/slack/monitor/message-handler/prepare.test.ts index 7123ea7f92f..a4feb2e4b2e 100644 --- a/src/slack/monitor/message-handler/prepare.test.ts +++ b/src/slack/monitor/message-handler/prepare.test.ts @@ -324,7 +324,7 @@ describe("slack prepareSlackMessage inbound contract", () => { expect(replies).toHaveBeenCalledTimes(2); }); - it("does not mark first thread turn when thread session already exists in store", async () => { + it("keeps loading thread history when thread session already exists in store", async () => { const { storePath } = makeTmpStorePath(); const cfg = { session: { store: storePath }, @@ -346,9 +346,19 @@ describe("slack prepareSlackMessage inbound contract", () => { JSON.stringify({ [threadKeys.sessionKey]: { updatedAt: Date.now() } }, null, 2), ); - const replies = vi.fn().mockResolvedValue({ - messages: [{ text: "starter", user: "U2", ts: "200.000" }], - }); + const replies = vi + .fn() + .mockResolvedValueOnce({ + messages: [{ text: "starter", user: "U2", ts: "200.000" }], + }) + .mockResolvedValueOnce({ + messages: [ + { text: "starter", user: "U2", ts: "200.000" }, + { text: "assistant follow-up", bot_id: "B1", ts: "200.500" }, + { text: "user follow-up", user: "U1", ts: "200.800" }, + { text: "current message", user: "U1", ts: "201.000" }, + ], + }); const slackCtx = createThreadSlackCtx({ cfg, replies }); slackCtx.resolveUserName = async () => ({ name: "Alice" }); slackCtx.resolveChannelName = async () => ({ name: "general", type: "channel" }); @@ -361,7 +371,10 @@ describe("slack prepareSlackMessage inbound contract", () => { expect(prepared).toBeTruthy(); expect(prepared!.ctxPayload.IsFirstThreadTurn).toBeUndefined(); - expect(prepared!.ctxPayload.ThreadHistoryBody).toBeUndefined(); + expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("assistant follow-up"); + expect(prepared!.ctxPayload.ThreadHistoryBody).toContain("user follow-up"); + expect(prepared!.ctxPayload.ThreadHistoryBody).not.toContain("current message"); + expect(replies).toHaveBeenCalledTimes(2); }); it("includes thread_ts and parent_user_id metadata in thread replies", async () => { diff --git a/src/slack/monitor/message-handler/prepare.ts b/src/slack/monitor/message-handler/prepare.ts index 7f6100469f5..c94ba9bbb70 100644 --- a/src/slack/monitor/message-handler/prepare.ts +++ b/src/slack/monitor/message-handler/prepare.ts @@ -521,7 +521,7 @@ export async function prepareSlackMessage(params: { storePath, sessionKey, // Thread-specific session key }); - if (threadInitialHistoryLimit > 0 && !threadSessionPreviousTimestamp) { + if (threadInitialHistoryLimit > 0) { const threadHistory = await resolveSlackThreadHistory({ channelId: message.channel, threadTs,