mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
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
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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}`);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user