diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b515a102d5..eff05048881 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ Docs: https://docs.openclaw.ai - Telegram/Streaming: preserve archived draft preview mapping after flush and clean superseded reasoning preview bubbles so multi-message preview finals no longer cross-edit or orphan stale messages under send/rotation races. (#23202) Thanks @obviyus. - Slack/Slash commands: preserve the Bolt app receiver when registering external select options handlers so monitor startup does not crash on runtimes that require bound `app.options` calls. (#23209) Thanks @0xgaia. +- Slack/Telegram slash sessions: await session metadata persistence before dispatch so first-turn native slash runs do not race session-origin metadata updates. (#23065) thanks @hydro13. - Agents/Ollama: preserve unsafe integer tool-call arguments as exact strings during NDJSON parsing, preventing large numeric IDs from being rounded before tool execution. (#23170) Thanks @BestJoester. - Cron/Gateway: keep `cron.list` and `cron.status` responsive during startup catch-up by avoiding a long-held cron lock while missed jobs execute. (#23106) Thanks @jayleekr. - Gateway/Config reload: compare array-valued config paths structurally during diffing so unchanged `memory.qmd.paths` and `memory.qmd.scope.rules` no longer trigger false restart-required reloads. (#23185) Thanks @rex05ai. diff --git a/src/slack/monitor/slash.test-harness.ts b/src/slack/monitor/slash.test-harness.ts index 9935b347897..39dec929b44 100644 --- a/src/slack/monitor/slash.test-harness.ts +++ b/src/slack/monitor/slash.test-harness.ts @@ -8,6 +8,8 @@ const mocks = vi.hoisted(() => ({ finalizeInboundContextMock: vi.fn(), resolveConversationLabelMock: vi.fn(), createReplyPrefixOptionsMock: vi.fn(), + recordSessionMetaFromInboundMock: vi.fn(), + resolveStorePathMock: vi.fn(), })); vi.mock("../../auto-reply/reply/provider-dispatcher.js", () => ({ @@ -35,6 +37,12 @@ vi.mock("../../channels/reply-prefix.js", () => ({ createReplyPrefixOptions: (...args: unknown[]) => mocks.createReplyPrefixOptionsMock(...args), })); +vi.mock("../../config/sessions.js", () => ({ + recordSessionMetaFromInbound: (...args: unknown[]) => + mocks.recordSessionMetaFromInboundMock(...args), + resolveStorePath: (...args: unknown[]) => mocks.resolveStorePathMock(...args), +})); + type SlashHarnessMocks = { dispatchMock: ReturnType; readAllowFromStoreMock: ReturnType; @@ -43,6 +51,8 @@ type SlashHarnessMocks = { finalizeInboundContextMock: ReturnType; resolveConversationLabelMock: ReturnType; createReplyPrefixOptionsMock: ReturnType; + recordSessionMetaFromInboundMock: ReturnType; + resolveStorePathMock: ReturnType; }; export function getSlackSlashMocks(): SlashHarnessMocks { @@ -61,4 +71,6 @@ export function resetSlackSlashMocks() { mocks.finalizeInboundContextMock.mockReset().mockImplementation((ctx: unknown) => ctx); mocks.resolveConversationLabelMock.mockReset().mockReturnValue(undefined); mocks.createReplyPrefixOptionsMock.mockReset().mockReturnValue({ onModelSelected: () => {} }); + mocks.recordSessionMetaFromInboundMock.mockReset().mockResolvedValue(undefined); + mocks.resolveStorePathMock.mockReset().mockReturnValue("/tmp/openclaw-sessions.json"); } diff --git a/src/slack/monitor/slash.test.ts b/src/slack/monitor/slash.test.ts index 53fa613b94d..8b2aee9e946 100644 --- a/src/slack/monitor/slash.test.ts +++ b/src/slack/monitor/slash.test.ts @@ -210,6 +210,14 @@ function findFirstActionsBlock(payload: { blocks?: Array<{ type: string }> }) { | undefined; } +function createDeferred() { + let resolve!: (value: T | PromiseLike) => void; + const promise = new Promise((res) => { + resolve = res; + }); + return { promise, resolve }; +} + function createArgMenusHarness() { const commands = new Map Promise>(); const actions = new Map Promise>(); @@ -859,3 +867,47 @@ describe("slack slash commands access groups", () => { expectUnauthorizedResponse(respond); }); }); + +describe("slack slash command session metadata", () => { + const { recordSessionMetaFromInboundMock } = getSlackSlashMocks(); + + it("calls recordSessionMetaFromInbound after dispatching a slash command", async () => { + const harness = createPolicyHarness({ groupPolicy: "open" }); + await registerAndRunPolicySlash({ harness }); + + expect(dispatchMock).toHaveBeenCalledTimes(1); + expect(recordSessionMetaFromInboundMock).toHaveBeenCalledTimes(1); + const call = recordSessionMetaFromInboundMock.mock.calls[0]?.[0] as { + sessionKey?: string; + ctx?: { OriginatingChannel?: string }; + }; + expect(call.ctx?.OriginatingChannel).toBe("slack"); + expect(call.sessionKey).toBeDefined(); + }); + + it("awaits session metadata persistence before dispatch", async () => { + const deferred = createDeferred(); + recordSessionMetaFromInboundMock.mockReset().mockReturnValue(deferred.promise); + + const harness = createPolicyHarness({ groupPolicy: "open" }); + await registerCommands(harness.ctx, harness.account); + + const runPromise = runSlashHandler({ + commands: harness.commands, + command: { + channel_id: harness.channelId, + channel_name: harness.channelName, + }, + }); + + await vi.waitFor(() => { + expect(recordSessionMetaFromInboundMock).toHaveBeenCalledTimes(1); + }); + expect(dispatchMock).not.toHaveBeenCalled(); + + deferred.resolve(); + await runPromise; + + expect(dispatchMock).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/slack/monitor/slash.ts b/src/slack/monitor/slash.ts index 27af729dbf0..4b98b0bbcc6 100644 --- a/src/slack/monitor/slash.ts +++ b/src/slack/monitor/slash.ts @@ -539,9 +539,14 @@ export async function registerSlackMonitorSlashCommands(params: { import("../../auto-reply/reply/inbound-context.js"), import("../../auto-reply/reply/provider-dispatcher.js"), ]); - const [{ resolveConversationLabel }, { createReplyPrefixOptions }] = await Promise.all([ + const [ + { resolveConversationLabel }, + { createReplyPrefixOptions }, + { recordSessionMetaFromInbound, resolveStorePath }, + ] = await Promise.all([ import("../../channels/conversation-label.js"), import("../../channels/reply-prefix.js"), + import("../../config/sessions.js"), ]); const route = resolveAgentRoute({ @@ -605,6 +610,19 @@ export async function registerSlackMonitorSlashCommands(params: { OriginatingTo: `user:${command.user_id}`, }); + const storePath = resolveStorePath(cfg.session?.store, { + agentId: route.agentId, + }); + try { + await recordSessionMetaFromInbound({ + storePath, + sessionKey: ctxPayload.SessionKey ?? route.sessionKey, + ctx: ctxPayload, + }); + } catch (err) { + runtime.error?.(danger(`slack slash: failed updating session meta: ${String(err)}`)); + } + const { onModelSelected, ...prefixOptions } = createReplyPrefixOptions({ cfg, agentId: route.agentId, diff --git a/src/telegram/bot-native-commands.session-meta.test.ts b/src/telegram/bot-native-commands.session-meta.test.ts new file mode 100644 index 00000000000..5f7e2b55022 --- /dev/null +++ b/src/telegram/bot-native-commands.session-meta.test.ts @@ -0,0 +1,173 @@ +import { describe, expect, it, vi } from "vitest"; +import type { OpenClawConfig } from "../config/config.js"; +import type { TelegramAccountConfig } from "../config/types.js"; +import type { RuntimeEnv } from "../runtime.js"; +import { registerTelegramNativeCommands } from "./bot-native-commands.js"; + +// All mocks scoped to this file only — does not affect bot-native-commands.test.ts + +const sessionMocks = vi.hoisted(() => ({ + recordSessionMetaFromInbound: vi.fn(), + resolveStorePath: vi.fn(), +})); +const replyMocks = vi.hoisted(() => ({ + dispatchReplyWithBufferedBlockDispatcher: vi.fn(async () => undefined), +})); + +vi.mock("../config/sessions.js", () => ({ + recordSessionMetaFromInbound: sessionMocks.recordSessionMetaFromInbound, + resolveStorePath: sessionMocks.resolveStorePath, +})); +vi.mock("../pairing/pairing-store.js", () => ({ + readChannelAllowFromStore: vi.fn(async () => []), +})); +vi.mock("../auto-reply/reply/inbound-context.js", () => ({ + finalizeInboundContext: vi.fn((ctx: unknown) => ctx), +})); +vi.mock("../auto-reply/reply/provider-dispatcher.js", () => ({ + dispatchReplyWithBufferedBlockDispatcher: replyMocks.dispatchReplyWithBufferedBlockDispatcher, +})); +vi.mock("../channels/reply-prefix.js", () => ({ + createReplyPrefixOptions: vi.fn(() => ({ onModelSelected: () => {} })), +})); +vi.mock("../auto-reply/skill-commands.js", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, listSkillCommandsForAgents: vi.fn(() => []) }; +}); +vi.mock("../plugins/commands.js", () => ({ + getPluginCommandSpecs: vi.fn(() => []), + matchPluginCommand: vi.fn(() => null), + executePluginCommand: vi.fn(async () => ({ text: "ok" })), +})); +vi.mock("./bot/delivery.js", () => ({ + deliverReplies: vi.fn(async () => ({ delivered: true })), +})); + +const buildParams = (cfg: OpenClawConfig, accountId = "default") => ({ + bot: { + api: { + setMyCommands: vi.fn().mockResolvedValue(undefined), + sendMessage: vi.fn().mockResolvedValue(undefined), + }, + command: vi.fn(), + } as unknown as Parameters[0]["bot"], + cfg, + runtime: {} as unknown as RuntimeEnv, + accountId, + telegramCfg: {} as TelegramAccountConfig, + allowFrom: [], + groupAllowFrom: [], + replyToMode: "off" as const, + textLimit: 4096, + useAccessGroups: false, + nativeEnabled: true, + nativeSkillsEnabled: true, + nativeDisabledExplicit: false, + resolveGroupPolicy: () => ({ allowlistEnabled: false, allowed: true }), + resolveTelegramGroupConfig: () => ({ + groupConfig: undefined, + topicConfig: undefined, + }), + shouldSkipUpdate: () => false, + opts: { token: "token" }, +}); + +function createDeferred() { + let resolve!: (value: T | PromiseLike) => void; + const promise = new Promise((res) => { + resolve = res; + }); + return { promise, resolve }; +} + +describe("registerTelegramNativeCommands — session metadata", () => { + it("calls recordSessionMetaFromInbound after a native slash command", async () => { + sessionMocks.recordSessionMetaFromInbound.mockReset().mockResolvedValue(undefined); + sessionMocks.resolveStorePath.mockReset().mockReturnValue("/tmp/openclaw-sessions.json"); + + const commandHandlers = new Map Promise>(); + const cfg: OpenClawConfig = {}; + + registerTelegramNativeCommands({ + ...buildParams(cfg), + allowFrom: ["*"], + bot: { + api: { + setMyCommands: vi.fn().mockResolvedValue(undefined), + sendMessage: vi.fn().mockResolvedValue(undefined), + }, + command: vi.fn((name: string, cb: (ctx: unknown) => Promise) => { + commandHandlers.set(name, cb); + }), + } as unknown as Parameters[0]["bot"], + }); + + const handler = commandHandlers.get("status"); + expect(handler).toBeTruthy(); + await handler?.({ + match: "", + message: { + message_id: 1, + date: Math.floor(Date.now() / 1000), + chat: { id: 100, type: "private" }, + from: { id: 200, username: "bob" }, + }, + }); + + expect(sessionMocks.recordSessionMetaFromInbound).toHaveBeenCalledTimes(1); + const call = ( + sessionMocks.recordSessionMetaFromInbound.mock.calls as unknown as Array< + [{ sessionKey?: string; ctx?: { OriginatingChannel?: string } }] + > + )[0]?.[0]; + expect(call?.ctx?.OriginatingChannel).toBe("telegram"); + expect(call?.sessionKey).toBeDefined(); + }); + + it("awaits session metadata persistence before dispatch", async () => { + const deferred = createDeferred(); + sessionMocks.recordSessionMetaFromInbound.mockReset().mockReturnValue(deferred.promise); + sessionMocks.resolveStorePath.mockReset().mockReturnValue("/tmp/openclaw-sessions.json"); + replyMocks.dispatchReplyWithBufferedBlockDispatcher.mockReset().mockResolvedValue(undefined); + + const commandHandlers = new Map Promise>(); + const cfg: OpenClawConfig = {}; + + registerTelegramNativeCommands({ + ...buildParams(cfg), + allowFrom: ["*"], + bot: { + api: { + setMyCommands: vi.fn().mockResolvedValue(undefined), + sendMessage: vi.fn().mockResolvedValue(undefined), + }, + command: vi.fn((name: string, cb: (ctx: unknown) => Promise) => { + commandHandlers.set(name, cb); + }), + } as unknown as Parameters[0]["bot"], + }); + + const handler = commandHandlers.get("status"); + expect(handler).toBeTruthy(); + + const runPromise = handler?.({ + match: "", + message: { + message_id: 1, + date: Math.floor(Date.now() / 1000), + chat: { id: 100, type: "private" }, + from: { id: 200, username: "bob" }, + }, + }); + + await vi.waitFor(() => { + expect(sessionMocks.recordSessionMetaFromInbound).toHaveBeenCalledTimes(1); + }); + expect(replyMocks.dispatchReplyWithBufferedBlockDispatcher).not.toHaveBeenCalled(); + + deferred.resolve(); + await runPromise; + + expect(replyMocks.dispatchReplyWithBufferedBlockDispatcher).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/telegram/bot-native-commands.ts b/src/telegram/bot-native-commands.ts index 424139c84d7..8bb4d4a9517 100644 --- a/src/telegram/bot-native-commands.ts +++ b/src/telegram/bot-native-commands.ts @@ -17,6 +17,7 @@ import { createReplyPrefixOptions } from "../channels/reply-prefix.js"; import type { OpenClawConfig } from "../config/config.js"; import type { ChannelGroupPolicy } from "../config/group-policy.js"; import { resolveMarkdownTableMode } from "../config/markdown-tables.js"; +import { recordSessionMetaFromInbound, resolveStorePath } from "../config/sessions.js"; import { normalizeTelegramCommandName, resolveTelegramCustomCommands, @@ -594,6 +595,19 @@ export const registerTelegramNativeCommands = ({ OriginatingTo: `telegram:${chatId}`, }); + const storePath = resolveStorePath(cfg.session?.store, { + agentId: route.agentId, + }); + try { + await recordSessionMetaFromInbound({ + storePath, + sessionKey: ctxPayload.SessionKey ?? route.sessionKey, + ctx: ctxPayload, + }); + } catch (err) { + runtime.error?.(danger(`telegram slash: failed updating session meta: ${String(err)}`)); + } + const disableBlockStreaming = typeof telegramCfg.blockStreaming === "boolean" ? !telegramCfg.blockStreaming