diff --git a/config/max-lines-baseline.txt b/config/max-lines-baseline.txt index 2c4dc1de403f..f8ad43e5c525 100644 --- a/config/max-lines-baseline.txt +++ b/config/max-lines-baseline.txt @@ -495,7 +495,6 @@ src/agents/tools/web-fetch.ts src/agents/transcript-redact.test.ts src/agents/workspace.ts src/agents/worktrees/service.ts -src/auto-reply/command-control.test.ts src/auto-reply/inbound.test.ts src/auto-reply/reply/abort.test.ts src/auto-reply/reply/agent-runner-memory.test.ts @@ -504,7 +503,6 @@ src/auto-reply/reply/agent-runner-payloads.test.ts src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts src/auto-reply/reply/commands-acp.test.ts -src/auto-reply/reply/commands-approve.test.ts src/auto-reply/reply/commands-models.ts src/auto-reply/reply/commands-status.test.ts src/auto-reply/reply/directive-handling.model.test.ts diff --git a/src/auto-reply/command-control.test.ts b/src/auto-reply/command-control.test.ts index 16f119610680..91d4d9e2c119 100644 --- a/src/auto-reply/command-control.test.ts +++ b/src/auto-reply/command-control.test.ts @@ -158,37 +158,37 @@ describe("resolveCommandAuthorization", () => { expect(auth.isAuthorizedSender).toBe(true); }); - it("uses explicit owner allowlist when allowFrom is wildcard", () => { + it.each([ + { + name: "uses explicit owner allowlist when allowFrom is wildcard", + channelConfig: { allowFrom: ["*"] }, + }, + { + name: "uses explicit owner allowlist when allowFrom is empty", + channelConfig: {}, + }, + ])("$name", ({ channelConfig }) => { const cfg = { commands: { ownerAllowFrom: ["whatsapp:+15551234567"] }, - channels: { whatsapp: { allowFrom: ["*"] } }, + channels: { whatsapp: channelConfig }, } as OpenClawConfig; + const resolveSender = (senderId: string) => + resolveCommandAuthorization({ + ctx: { + Provider: "whatsapp", + Surface: "whatsapp", + From: `whatsapp:${senderId}`, + SenderE164: senderId, + } as MsgContext, + cfg, + commandAuthorized: true, + }); - const ownerCtx = { - Provider: "whatsapp", - Surface: "whatsapp", - From: "whatsapp:+15551234567", - SenderE164: "+15551234567", - } as MsgContext; - const ownerAuth = resolveCommandAuthorization({ - ctx: ownerCtx, - cfg, - commandAuthorized: true, - }); + const ownerAuth = resolveSender("+15551234567"); expect(ownerAuth.senderIsOwner).toBe(true); expect(ownerAuth.isAuthorizedSender).toBe(true); - const otherCtx = { - Provider: "whatsapp", - Surface: "whatsapp", - From: "whatsapp:+19995551234", - SenderE164: "+19995551234", - } as MsgContext; - const otherAuth = resolveCommandAuthorization({ - ctx: otherCtx, - cfg, - commandAuthorized: true, - }); + const otherAuth = resolveSender("+19995551234"); expect(otherAuth.senderIsOwner).toBe(false); expect(otherAuth.isAuthorizedSender).toBe(false); }); @@ -278,39 +278,6 @@ describe("resolveCommandAuthorization", () => { expect(auth.isAuthorizedSender).toBe(false); }); - it("uses explicit owner allowlist when allowFrom is empty", () => { - const cfg = { - commands: { ownerAllowFrom: ["whatsapp:+15551234567"] }, - channels: { whatsapp: {} }, - } as OpenClawConfig; - - const ownerAuth = resolveCommandAuthorization({ - ctx: { - Provider: "whatsapp", - Surface: "whatsapp", - From: "whatsapp:+15551234567", - SenderE164: "+15551234567", - } as MsgContext, - cfg, - commandAuthorized: true, - }); - expect(ownerAuth.senderIsOwner).toBe(true); - expect(ownerAuth.isAuthorizedSender).toBe(true); - - const otherAuth = resolveCommandAuthorization({ - ctx: { - Provider: "whatsapp", - Surface: "whatsapp", - From: "whatsapp:+19995551234", - SenderE164: "+19995551234", - } as MsgContext, - cfg, - commandAuthorized: true, - }); - expect(otherAuth.senderIsOwner).toBe(false); - expect(otherAuth.isAuthorizedSender).toBe(false); - }); - it("uses context owner candidates for command authorization without granting owner status", () => { setActivePluginRegistry( createTestRegistry([ @@ -389,86 +356,52 @@ describe("resolveCommandAuthorization", () => { expect(auth.isAuthorizedSender).toBe(true); }); - it("does not apply channel-prefixed owner wildcards to webchat command contexts", () => { - const cfg = { - commands: { ownerAllowFrom: ["discord:*"] }, - } as OpenClawConfig; - + it.each([ + { + name: "does not apply channel-prefixed owner wildcards to webchat command contexts", + owner: "discord:*", + provider: "webchat", + expectedProvider: undefined, + expectedOwner: false, + }, + { + name: "does not apply channel-prefixed owner identities to webchat command contexts", + owner: "discord:123456789012345678", + provider: "webchat", + expectedProvider: undefined, + expectedOwner: false, + }, + { + name: "applies channel-prefixed owner identities to matching providers", + owner: "discord:123456789012345678", + provider: "discord", + expectedProvider: "discord", + expectedOwner: true, + }, + { + name: "does not apply channel-prefixed owner wildcards to mismatched providers", + owner: "telegram:*", + provider: "discord", + expectedProvider: "discord", + expectedOwner: false, + }, + ] as const)("$name", ({ owner, provider, expectedProvider, expectedOwner }) => { + const webchat = provider === "webchat"; const auth = resolveCommandAuthorization({ ctx: { - Provider: "webchat", - Surface: "webchat", - OriginatingChannel: "webchat", + Provider: provider, + Surface: provider, + ...(webchat + ? { OriginatingChannel: "webchat", GatewayClientScopes: ["operator.write"] } + : { From: "discord:123456789012345678" }), SenderId: "123456789012345678", - GatewayClientScopes: ["operator.write"], } as MsgContext, - cfg, + cfg: { commands: { ownerAllowFrom: [owner] } } as OpenClawConfig, commandAuthorized: true, }); - expect(auth.providerId).toBeUndefined(); - expect(auth.senderIsOwner).toBe(false); - }); - - it("does not apply channel-prefixed owner identities to webchat command contexts", () => { - const cfg = { - commands: { ownerAllowFrom: ["discord:123456789012345678"] }, - } as OpenClawConfig; - - const auth = resolveCommandAuthorization({ - ctx: { - Provider: "webchat", - Surface: "webchat", - OriginatingChannel: "webchat", - SenderId: "123456789012345678", - GatewayClientScopes: ["operator.write"], - } as MsgContext, - cfg, - commandAuthorized: true, - }); - - expect(auth.providerId).toBeUndefined(); - expect(auth.senderIsOwner).toBe(false); - }); - - it("applies channel-prefixed owner identities to matching providers", () => { - const cfg = { - commands: { ownerAllowFrom: ["discord:123456789012345678"] }, - } as OpenClawConfig; - - const auth = resolveCommandAuthorization({ - ctx: { - Provider: "discord", - Surface: "discord", - From: "discord:123456789012345678", - SenderId: "123456789012345678", - } as MsgContext, - cfg, - commandAuthorized: true, - }); - - expect(auth.providerId).toBe("discord"); - expect(auth.senderIsOwner).toBe(true); - }); - - it("does not apply channel-prefixed owner wildcards to mismatched providers", () => { - const cfg = { - commands: { ownerAllowFrom: ["telegram:*"] }, - } as OpenClawConfig; - - const auth = resolveCommandAuthorization({ - ctx: { - Provider: "discord", - Surface: "discord", - From: "discord:123456789012345678", - SenderId: "123456789012345678", - } as MsgContext, - cfg, - commandAuthorized: true, - }); - - expect(auth.providerId).toBe("discord"); - expect(auth.senderIsOwner).toBe(false); + expect(auth.providerId).toBe(expectedProvider); + expect(auth.senderIsOwner).toBe(expectedOwner); }); it("preserves external channel command auth in mixed webchat contexts", () => { @@ -721,78 +654,56 @@ describe("resolveCommandAuthorization", () => { expect(auth.isAuthorizedSender).toBe(true); }); - it("does not treat conversation ids in From as sender identities", () => { - const cfg = { - commands: { - allowFrom: { - discord: ["channel:123456789012345678"], - }, - }, - } as OpenClawConfig; + it.each([ + { + name: "does not treat conversation ids in From as sender identities", + provider: "discord", + chatType: "channel", + from: "discord:channel:123456789012345678", + senderId: "999999999999999999", + senderE164: undefined, + allowFrom: { discord: ["channel:123456789012345678"] }, + expected: false, + }, + { + name: "still falls back to From for direct messages when sender fields are absent", + provider: "discord", + chatType: "direct", + from: "discord:123456789012345678", + senderId: " ", + senderE164: " ", + allowFrom: { discord: ["123456789012345678"] }, + expected: true, + }, + { + name: "does not fall back to conversation-shaped From when chat type is missing", + provider: "whatsapp", + chatType: undefined, + from: "demo:group:room-1", + senderId: " ", + senderE164: " ", + allowFrom: { "*": ["demo:group:room-1"] }, + expected: false, + }, + ] as const)( + "$name", + ({ provider, chatType, from, senderId, senderE164, allowFrom, expected }) => { + const auth = resolveCommandAuthorization({ + ctx: { + Provider: provider, + Surface: provider, + ChatType: chatType, + From: from, + SenderId: senderId, + SenderE164: senderE164, + } as MsgContext, + cfg: { commands: { allowFrom } } as unknown as OpenClawConfig, + commandAuthorized: false, + }); - const auth = resolveCommandAuthorization({ - ctx: { - Provider: "discord", - Surface: "discord", - ChatType: "channel", - From: "discord:channel:123456789012345678", - SenderId: "999999999999999999", - } as MsgContext, - cfg, - commandAuthorized: false, - }); - - expect(auth.isAuthorizedSender).toBe(false); - }); - - it("still falls back to From for direct messages when sender fields are absent", () => { - const cfg = { - commands: { - allowFrom: { - discord: ["123456789012345678"], - }, - }, - } as OpenClawConfig; - - const auth = resolveCommandAuthorization({ - ctx: { - Provider: "discord", - Surface: "discord", - ChatType: "direct", - From: "discord:123456789012345678", - SenderId: " ", - SenderE164: " ", - } as MsgContext, - cfg, - commandAuthorized: false, - }); - - expect(auth.isAuthorizedSender).toBe(true); - }); - - it("does not fall back to conversation-shaped From when chat type is missing", () => { - const cfg = { - commands: { - allowFrom: { - "*": ["demo:group:room-1"], - }, - }, - } as OpenClawConfig; - - const auth = resolveCommandAuthorization({ - ctx: { - Provider: "whatsapp", - Surface: "whatsapp", - From: "demo:group:room-1", - SenderId: " ", - SenderE164: " ", - } as MsgContext, - cfg, - commandAuthorized: false, - }); - - expect(auth.isAuthorizedSender).toBe(false); - }); + expect(auth.isAuthorizedSender).toBe(expected); + }, + ); it("normalizes Discord commands.allowFrom prefixes and mentions", () => { const cfg = { @@ -835,121 +746,80 @@ describe("resolveCommandAuthorization", () => { expect(deniedAuth.isAuthorizedSender).toBe(false); }); - it("fails closed when provider inference hits unresolved SecretRef allowlists", () => { - registerAllowFromPlugins( - createThrowingAllowFromPlugin( - "telegram", - "channels.telegram.botToken: unresolved SecretRef", - ), - ); - - const cfg = { - commands: { - allowFrom: { - telegram: ["123"], - }, - }, - channels: { - telegram: { - allowFrom: ["123"], - }, - }, - } as OpenClawConfig; - - const auth = resolveCommandAuthorization({ - ctx: { - SenderId: "123", - } as MsgContext, - cfg, + it.each([ + { + name: "fails closed when provider inference hits unresolved SecretRef allowlists", + failingProvider: "telegram", + allowKey: "telegram", + channelMode: "configured", + validTelegram: false, commandAuthorized: false, - }); - - expect(auth.providerId).toBe("telegram"); - expect(auth.isAuthorizedSender).toBe(false); - }); - - it("preserves provider resolution errors when inferred fallback allowFrom is empty", () => { - registerAllowFromPlugins( - createThrowingAllowFromPlugin( - "telegram", - "channels.telegram.botToken: unresolved SecretRef", - ), - ); - - const auth = resolveCommandAuthorization({ - ctx: { - SenderId: "123", - } as MsgContext, - cfg: { - commands: { - allowFrom: { - telegram: ["123"], - }, - }, - channels: { - telegram: {}, - }, - } as OpenClawConfig, + expectedProvider: "telegram", + expectedAuthorized: false, + }, + { + name: "preserves provider resolution errors when inferred fallback allowFrom is empty", + failingProvider: "telegram", + allowKey: "telegram", + channelMode: "empty", + validTelegram: false, commandAuthorized: true, - }); - - expect(auth.providerId).toBeUndefined(); - expect(auth.isAuthorizedSender).toBe(false); - }); - - it("fails closed for global commands.allowFrom when inference errors drop every provider", () => { - registerAllowFromPlugins( - createThrowingAllowFromPlugin("slack", "channels.slack.token: unresolved SecretRef"), - ); - - const auth = resolveCommandAuthorization({ - ctx: { - SenderId: "123", - } as MsgContext, - cfg: { - commands: { - allowFrom: { - "*": ["123"], - }, - }, - channels: { - slack: {}, - }, - } as OpenClawConfig, + expectedProvider: undefined, + expectedAuthorized: false, + }, + { + name: "fails closed for global commands.allowFrom when inference errors drop every provider", + failingProvider: "slack", + allowKey: "*", + channelMode: "empty", + validTelegram: false, commandAuthorized: false, - }); - - expect(auth.providerId).toBeUndefined(); - expect(auth.isAuthorizedSender).toBe(false); - }); - it("does not let an unrelated provider resolution error poison inferred commands.allowFrom", () => { - registerAllowFromPlugins( - createAllowFromPlugin("telegram", () => ["123"]), - createThrowingAllowFromPlugin("slack", "channels.slack.token: unresolved SecretRef"), - ); - - const auth = resolveCommandAuthorization({ - ctx: { - SenderId: "123", - } as MsgContext, - cfg: { - commands: { - allowFrom: { - telegram: ["123"], - }, - }, - channels: { - telegram: { - allowFrom: ["123"], - }, - }, - } as OpenClawConfig, + expectedProvider: undefined, + expectedAuthorized: false, + }, + { + name: "does not let an unrelated provider resolution error poison inferred commands.allowFrom", + failingProvider: "slack", + allowKey: "telegram", + channelMode: "configured", + validTelegram: true, commandAuthorized: false, - }); + expectedProvider: "telegram", + expectedAuthorized: true, + }, + ] as const)( + "$name", + ({ + failingProvider, + allowKey, + channelMode, + validTelegram, + commandAuthorized, + expectedProvider, + expectedAuthorized, + }) => { + registerAllowFromPlugins( + ...(validTelegram ? [createAllowFromPlugin("telegram", () => ["123"])] : []), + createThrowingAllowFromPlugin( + failingProvider, + `channels.${failingProvider}.${failingProvider === "telegram" ? "botToken" : "token"}: unresolved SecretRef`, + ), + ); + const channelId = validTelegram ? "telegram" : failingProvider; + const channelConfig = channelMode === "configured" ? { allowFrom: ["123"] } : {}; + const auth = resolveCommandAuthorization({ + ctx: { SenderId: "123" } as MsgContext, + cfg: { + commands: { allowFrom: { [allowKey]: ["123"] } }, + channels: { [channelId]: channelConfig }, + } as OpenClawConfig, + commandAuthorized, + }); - expect(auth.providerId).toBe("telegram"); - expect(auth.isAuthorizedSender).toBe(true); - }); + expect(auth.providerId).toBe(expectedProvider); + expect(auth.isAuthorizedSender).toBe(expectedAuthorized); + }, + ); it("preserves default-account allowFrom on SecretRef fallback", () => { registerAllowFromPlugins( @@ -1034,84 +904,66 @@ describe("resolveCommandAuthorization", () => { }); }); - it("grants senderIsOwner for internal channel with operator.admin scope", () => { - const cfg = {} as OpenClawConfig; - const ctx = { - Provider: "webchat", - Surface: "webchat", - GatewayClientScopes: ["operator.admin"], - } as MsgContext; + it.each([ + { + name: "grants senderIsOwner for internal channel with operator.admin scope", + provider: "webchat", + scope: "operator.admin", + expectedOwner: true, + }, + { + name: "does not grant senderIsOwner for internal channel without admin scope", + provider: "webchat", + scope: "operator.approvals", + expectedOwner: false, + }, + { + name: "does not grant senderIsOwner for external channel even with admin scope", + provider: "telegram", + scope: "operator.admin", + expectedOwner: false, + }, + ] as const)("$name", ({ provider, scope, expectedOwner }) => { const auth = resolveCommandAuthorization({ - ctx, - cfg, + ctx: { + Provider: provider, + Surface: provider, + ...(provider === "telegram" ? { From: "telegram:12345" } : {}), + GatewayClientScopes: [scope], + } as MsgContext, + cfg: {} as OpenClawConfig, commandAuthorized: true, }); - expect(auth.senderIsOwner).toBe(true); - }); - - it("does not grant senderIsOwner for internal channel without admin scope", () => { - const cfg = {} as OpenClawConfig; - const ctx = { - Provider: "webchat", - Surface: "webchat", - GatewayClientScopes: ["operator.approvals"], - } as MsgContext; - const auth = resolveCommandAuthorization({ - ctx, - cfg, - commandAuthorized: true, - }); - expect(auth.senderIsOwner).toBe(false); - }); - - it("does not grant senderIsOwner for external channel even with admin scope", () => { - const cfg = {} as OpenClawConfig; - const ctx = { - Provider: "telegram", - Surface: "telegram", - From: "telegram:12345", - GatewayClientScopes: ["operator.admin"], - } as MsgContext; - const auth = resolveCommandAuthorization({ - ctx, - cfg, - commandAuthorized: true, - }); - expect(auth.senderIsOwner).toBe(false); + expect(auth.senderIsOwner).toBe(expectedOwner); }); }); describe("control command parsing", () => { + function expectCases( + parse: (value: string) => unknown, + cases: readonly (readonly [string, unknown])[], + ) { + cases.forEach(([value, expected]) => expect(parse(value)).toEqual(expected)); + } + it("requires slash for send policy", () => { - expect(parseSendPolicyCommand("/send on")).toEqual({ - hasCommand: true, - mode: "allow", - }); - expect(parseSendPolicyCommand("/send: on")).toEqual({ - hasCommand: true, - mode: "allow", - }); - expect(parseSendPolicyCommand("/send")).toEqual({ hasCommand: true }); - expect(parseSendPolicyCommand("/send:")).toEqual({ hasCommand: true }); - expect(parseSendPolicyCommand("send on")).toEqual({ hasCommand: false }); - expect(parseSendPolicyCommand("send")).toEqual({ hasCommand: false }); + expectCases(parseSendPolicyCommand, [ + ["/send on", { hasCommand: true, mode: "allow" }], + ["/send: on", { hasCommand: true, mode: "allow" }], + ["/send", { hasCommand: true }], + ["/send:", { hasCommand: true }], + ["send on", { hasCommand: false }], + ["send", { hasCommand: false }], + ]); }); it("requires slash for activation", () => { - expect(parseActivationCommand("/activation mention")).toEqual({ - hasCommand: true, - mode: "mention", - }); - expect(parseActivationCommand("/activation: mention")).toEqual({ - hasCommand: true, - mode: "mention", - }); - expect(parseActivationCommand("/activation:")).toEqual({ - hasCommand: true, - }); - expect(parseActivationCommand("activation mention")).toEqual({ - hasCommand: false, - }); + expectCases(parseActivationCommand, [ + ["/activation mention", { hasCommand: true, mode: "mention" }], + ["/activation: mention", { hasCommand: true, mode: "mention" }], + ["/activation:", { hasCommand: true }], + ["activation mention", { hasCommand: false }], + ]); }); it("treats bare commands as non-control", () => { @@ -1195,40 +1047,31 @@ describe("control command parsing", () => { ).toBe(true); }); + function expectCommandAfterMetadata(label: string, json: string, command: string) { + expect(hasControlCommand([label, "```json", json, "```", "", command].join("\n"))).toBe(true); + } + it("detects commands wrapped in inbound metadata blocks", () => { - const metaWrapped = [ + expectCommandAfterMetadata( markInboundContextLabel("Conversation info:"), - "```json", '{"message_id":"msg-abc","chat_id":"chat-123"}', - "```", - "", "/model spark", - ].join("\n"); - expect(hasControlCommand(metaWrapped)).toBe(true); + ); }); it("detects /new command after metadata prefix", () => { - const metaWrapped = [ + expectCommandAfterMetadata( markInboundContextLabel("Sender:"), - "```json", '{"name":"Alice","id":"user-1"}', - "```", - "", "/new spark", - ].join("\n"); - expect(hasControlCommand(metaWrapped)).toBe(true); + ); }); it("detects /status command after timestamp + metadata prefix", () => { - const metaWrapped = [ + expectCommandAfterMetadata( `[Wed 2026-03-11 23:51 PDT] ${markInboundContextLabel("Conversation info:")}`, - "```json", '{"chat_id":"chat-123"}', - "```", - "", "/status", - ].join("\n"); - expect(hasControlCommand(metaWrapped)).toBe(true); + ); }); }); -/* oxlint-disable max-lines -- TODO: split this grandfathered oversized file. */ diff --git a/src/auto-reply/dispatch.freshness.test.ts b/src/auto-reply/dispatch.freshness.test.ts index 191e1d28b0df..69cfdbf2cbd3 100644 --- a/src/auto-reply/dispatch.freshness.test.ts +++ b/src/auto-reply/dispatch.freshness.test.ts @@ -87,6 +87,50 @@ function dispatchWithDeliveries( }); } +type DispatcherOptions = NonNullable[2]>; + +async function runDelayedOlderFinalRace( + createNewerOptions: (deliveries: Delivery[]) => DispatcherOptions = () => ({}), + olderOptions: DispatcherOptions = {}, +) { + const deliveries: Delivery[] = []; + const beforeDeliverStarted = createDeferred(); + const releaseBeforeDeliver = createDeferred(); + const beforeDeliver = vi.fn(() => { + beforeDeliverStarted.resolve(); + return releaseBeforeDeliver.promise; + }); + + hoisted.dispatchReplyFromConfigMock.mockImplementation( + async (params: DispatchReplyFromConfigParams) => { + if (params.ctx.MessageSid === "old-message") { + params.dispatcher.sendFinalReply({ text: "old final" }); + return queuedFinalResult(); + } + if (params.ctx.MessageSid === "new-message") { + params.dispatcher.sendFinalReply({ text: "new final" }); + return queuedFinalResult(); + } + throw new Error(`unexpected test message ${params.ctx.MessageSid ?? ""}`); + }, + ); + + const olderDispatch = dispatchWithDeliveries( + buildForegroundCtx({ MessageSid: "old-message" }), + deliveries, + { ...olderOptions, beforeDeliver }, + ); + await beforeDeliverStarted.promise; + const newerResult = await dispatchWithDeliveries( + buildForegroundCtx({ MessageSid: "new-message" }), + deliveries, + createNewerOptions(deliveries), + ); + releaseBeforeDeliver.resolve({ text: "old rewritten final" }); + + return { beforeDeliver, deliveries, newerResult, olderResult: await olderDispatch }; +} + describe("foreground reply freshness", () => { beforeEach(() => { resetGlobalHookRunner(); @@ -493,48 +537,14 @@ describe("foreground reply freshness", () => { }); it("keeps an older foreground final when a newer visible delivery fails", async () => { - const deliveries: Delivery[] = []; - const beforeDeliverStarted = createDeferred(); - const releaseBeforeDeliver = createDeferred(); - const beforeDeliver = vi.fn(() => { - beforeDeliverStarted.resolve(); - return releaseBeforeDeliver.promise; - }); - - hoisted.dispatchReplyFromConfigMock.mockImplementation( - async (params: DispatchReplyFromConfigParams) => { - if (params.ctx.MessageSid === "old-message") { - params.dispatcher.sendFinalReply({ text: "old final" }); - return queuedFinalResult(); - } - if (params.ctx.MessageSid === "new-message") { - params.dispatcher.sendFinalReply({ text: "new final" }); - return queuedFinalResult(); - } - throw new Error(`unexpected test message ${params.ctx.MessageSid ?? ""}`); - }, - ); - - const olderDispatch = dispatchWithDeliveries( - buildForegroundCtx({ MessageSid: "old-message" }), - deliveries, - { beforeDeliver }, - ); - await beforeDeliverStarted.promise; - - const newerResult = await dispatchWithDeliveries( - buildForegroundCtx({ MessageSid: "new-message" }), - deliveries, - { + const { beforeDeliver, deliveries, newerResult, olderResult } = await runDelayedOlderFinalRace( + () => ({ deliver: async () => { throw new Error("delivery failed"); }, - }, + }), ); - releaseBeforeDeliver.resolve({ text: "old rewritten final" }); - const olderResult = await olderDispatch; - expect(beforeDeliver).toHaveBeenCalledTimes(1); expect(newerResult).toEqual({ queuedFinal: false, @@ -567,49 +577,15 @@ describe("foreground reply freshness", () => { }), }, ])("suppresses an older foreground final after $label", async ({ createError }) => { - const deliveries: Delivery[] = []; - const beforeDeliverStarted = createDeferred(); - const releaseBeforeDeliver = createDeferred(); - const beforeDeliver = vi.fn(() => { - beforeDeliverStarted.resolve(); - return releaseBeforeDeliver.promise; - }); - - hoisted.dispatchReplyFromConfigMock.mockImplementation( - async (params: DispatchReplyFromConfigParams) => { - if (params.ctx.MessageSid === "old-message") { - params.dispatcher.sendFinalReply({ text: "old final" }); - return queuedFinalResult(); - } - if (params.ctx.MessageSid === "new-message") { - params.dispatcher.sendFinalReply({ text: "new final" }); - return queuedFinalResult(); - } - throw new Error(`unexpected test message ${params.ctx.MessageSid ?? ""}`); - }, - ); - - const olderDispatch = dispatchWithDeliveries( - buildForegroundCtx({ MessageSid: "old-message" }), - deliveries, - { beforeDeliver }, - ); - await beforeDeliverStarted.promise; - - const newerResult = await dispatchWithDeliveries( - buildForegroundCtx({ MessageSid: "new-message" }), - deliveries, - { + const { beforeDeliver, deliveries, newerResult, olderResult } = await runDelayedOlderFinalRace( + (raceDeliveries) => ({ deliver: async (payload, info) => { - deliveries.push({ kind: info.kind, text: payload.text }); + raceDeliveries.push({ kind: info.kind, text: payload.text }); throw createError(); }, - }, + }), ); - releaseBeforeDeliver.resolve({ text: "old rewritten final" }); - const olderResult = await olderDispatch; - expect(beforeDeliver).toHaveBeenCalledTimes(1); expect(newerResult).toEqual({ queuedFinal: false, @@ -624,46 +600,12 @@ describe("foreground reply freshness", () => { }); it("keeps an older foreground final when a newer adapter reports non-visible delivery", async () => { - const deliveries: Delivery[] = []; - const beforeDeliverStarted = createDeferred(); - const releaseBeforeDeliver = createDeferred(); - const beforeDeliver = vi.fn(() => { - beforeDeliverStarted.resolve(); - return releaseBeforeDeliver.promise; - }); - - hoisted.dispatchReplyFromConfigMock.mockImplementation( - async (params: DispatchReplyFromConfigParams) => { - if (params.ctx.MessageSid === "old-message") { - params.dispatcher.sendFinalReply({ text: "old final" }); - return queuedFinalResult(); - } - if (params.ctx.MessageSid === "new-message") { - params.dispatcher.sendFinalReply({ text: "new final" }); - return queuedFinalResult(); - } - throw new Error(`unexpected test message ${params.ctx.MessageSid ?? ""}`); - }, - ); - - const olderDispatch = dispatchWithDeliveries( - buildForegroundCtx({ MessageSid: "old-message" }), - deliveries, - { beforeDeliver }, - ); - await beforeDeliverStarted.promise; - - const newerResult = await dispatchWithDeliveries( - buildForegroundCtx({ MessageSid: "new-message" }), - deliveries, - { + const { beforeDeliver, deliveries, newerResult, olderResult } = await runDelayedOlderFinalRace( + () => ({ deliver: async () => ({ visibleReplySent: false }), - }, + }), ); - releaseBeforeDeliver.resolve({ text: "old rewritten final" }); - const olderResult = await olderDispatch; - expect(beforeDeliver).toHaveBeenCalledTimes(1); expect(newerResult).toEqual({ queuedFinal: true, @@ -677,47 +619,13 @@ describe("foreground reply freshness", () => { }); it("suppresses an older foreground final when a newer settled hook reports visible delivery", async () => { - const deliveries: Delivery[] = []; - const beforeDeliverStarted = createDeferred(); - const releaseBeforeDeliver = createDeferred(); - const beforeDeliver = vi.fn(() => { - beforeDeliverStarted.resolve(); - return releaseBeforeDeliver.promise; - }); - - hoisted.dispatchReplyFromConfigMock.mockImplementation( - async (params: DispatchReplyFromConfigParams) => { - if (params.ctx.MessageSid === "old-message") { - params.dispatcher.sendFinalReply({ text: "old final" }); - return queuedFinalResult(); - } - if (params.ctx.MessageSid === "new-message") { - params.dispatcher.sendFinalReply({ text: "new final" }); - return queuedFinalResult(); - } - throw new Error(`unexpected test message ${params.ctx.MessageSid ?? ""}`); - }, - ); - - const olderDispatch = dispatchWithDeliveries( - buildForegroundCtx({ MessageSid: "old-message" }), - deliveries, - { beforeDeliver }, - ); - await beforeDeliverStarted.promise; - - const newerResult = await dispatchWithDeliveries( - buildForegroundCtx({ MessageSid: "new-message" }), - deliveries, - { + const { beforeDeliver, deliveries, newerResult, olderResult } = await runDelayedOlderFinalRace( + () => ({ deliver: async () => ({ visibleReplySent: false }), onSettled: async () => ({ visibleReplySent: true }), - }, + }), ); - releaseBeforeDeliver.resolve({ text: "old rewritten final" }); - const olderResult = await olderDispatch; - expect(beforeDeliver).toHaveBeenCalledTimes(1); expect(newerResult).toEqual({ queuedFinal: true, @@ -731,44 +639,12 @@ describe("foreground reply freshness", () => { }); it("still runs stale generic settled hooks after a newer visible reply", async () => { - const deliveries: Delivery[] = []; - const beforeDeliverStarted = createDeferred(); - const releaseBeforeDeliver = createDeferred(); - const beforeDeliver = vi.fn(() => { - beforeDeliverStarted.resolve(); - return releaseBeforeDeliver.promise; - }); const olderSettled = vi.fn(); - - hoisted.dispatchReplyFromConfigMock.mockImplementation( - async (params: DispatchReplyFromConfigParams) => { - if (params.ctx.MessageSid === "old-message") { - params.dispatcher.sendFinalReply({ text: "old final" }); - return queuedFinalResult(); - } - if (params.ctx.MessageSid === "new-message") { - params.dispatcher.sendFinalReply({ text: "new final" }); - return queuedFinalResult(); - } - throw new Error(`unexpected test message ${params.ctx.MessageSid ?? ""}`); - }, + const { beforeDeliver, deliveries, newerResult, olderResult } = await runDelayedOlderFinalRace( + () => ({}), + { onSettled: olderSettled }, ); - const olderDispatch = dispatchWithDeliveries( - buildForegroundCtx({ MessageSid: "old-message" }), - deliveries, - { beforeDeliver, onSettled: olderSettled }, - ); - await beforeDeliverStarted.promise; - - const newerResult = await dispatchWithDeliveries( - buildForegroundCtx({ MessageSid: "new-message" }), - deliveries, - ); - - releaseBeforeDeliver.resolve({ text: "old rewritten final" }); - const olderResult = await olderDispatch; - expect(beforeDeliver).toHaveBeenCalledTimes(1); expect(olderSettled).toHaveBeenCalledTimes(1); expect(newerResult).toEqual({ @@ -783,47 +659,16 @@ describe("foreground reply freshness", () => { }); it("suppresses an older fresh settled delivery after a newer visible reply", async () => { - const deliveries: Delivery[] = []; - const beforeDeliverStarted = createDeferred(); - const releaseBeforeDeliver = createDeferred(); - const beforeDeliver = vi.fn(() => { - beforeDeliverStarted.resolve(); - return releaseBeforeDeliver.promise; - }); const olderFreshDelivery = vi.fn(() => { - deliveries.push({ kind: "final", text: "old settled fallback" }); return { visibleReplySent: true }; }); - - hoisted.dispatchReplyFromConfigMock.mockImplementation( - async (params: DispatchReplyFromConfigParams) => { - if (params.ctx.MessageSid === "old-message") { - params.dispatcher.sendFinalReply({ text: "old final" }); - return queuedFinalResult(); - } - if (params.ctx.MessageSid === "new-message") { - params.dispatcher.sendFinalReply({ text: "new final" }); - return queuedFinalResult(); - } - throw new Error(`unexpected test message ${params.ctx.MessageSid ?? ""}`); + const { beforeDeliver, deliveries, newerResult, olderResult } = await runDelayedOlderFinalRace( + () => ({}), + { + onFreshSettledDelivery: olderFreshDelivery, }, ); - const olderDispatch = dispatchWithDeliveries( - buildForegroundCtx({ MessageSid: "old-message" }), - deliveries, - { beforeDeliver, onFreshSettledDelivery: olderFreshDelivery }, - ); - await beforeDeliverStarted.promise; - - const newerResult = await dispatchWithDeliveries( - buildForegroundCtx({ MessageSid: "new-message" }), - deliveries, - ); - - releaseBeforeDeliver.resolve({ text: "old rewritten final" }); - const olderResult = await olderDispatch; - expect(beforeDeliver).toHaveBeenCalledTimes(1); expect(olderFreshDelivery).not.toHaveBeenCalled(); expect(newerResult).toEqual({ diff --git a/src/auto-reply/reply/commands-approve.test.ts b/src/auto-reply/reply/commands-approve.test.ts index fce2a48bdd6c..7ec6e0ca1b83 100644 --- a/src/auto-reply/reply/commands-approve.test.ts +++ b/src/auto-reply/reply/commands-approve.test.ts @@ -484,83 +484,101 @@ describe("handleApproveCommand", () => { expect(result?.reply?.text).toContain("Usage: /approve"); }); - it("submits approval", async () => { - resolveApprovalOverGatewayMock.mockResolvedValue(undefined); - const result = await handleApproveCommand( - buildApproveParams( - "/approve abc allow-once", - { - commands: { text: true }, - channels: { whatsapp: { allowFrom: ["*"] } }, - } as OpenClawConfig, - { SenderId: "123" }, - ), - true, - ); - - expect(result?.shouldContinue).toBe(false); - expect(result?.reply?.text).toContain("Approval allow-once submitted"); - expectApprovalResolverCall({ method: "exec.approval.resolve", id: "abc" }); - }); - - it("accepts bare approve text for Slack-style manual approvals", async () => { - resolveApprovalOverGatewayMock.mockResolvedValue(undefined); - const result = await handleApproveCommand( - buildApproveParams( - "approve abc allow-once", - { - commands: { text: true }, - channels: { slack: { allowFrom: ["*"] } }, - } as OpenClawConfig, - { - Provider: "slack", - Surface: "slack", - SenderId: "U123", - }, - ), - true, - ); - - expect(result?.shouldContinue).toBe(false); - expect(result?.reply?.text).toContain("Approval allow-once submitted"); - expectApprovalResolverCall({ method: "exec.approval.resolve", id: "abc" }); - }); - - it("accepts Telegram /approve from configured approvers even when chat access is otherwise blocked", async () => { - const params = buildApproveParams("/approve abc12345 allow-once", createTelegramApproveCfg(), { - Provider: "telegram", - Surface: "telegram", - SenderId: "123", - }); - params.command.isAuthorizedSender = false; - resolveApprovalOverGatewayMock.mockResolvedValue(undefined); - - const result = await handleApproveCommand(params, true); - expect(result?.shouldContinue).toBe(false); - expect(result?.reply?.text).toContain("Approval allow-once submitted"); - expectApprovalResolverCall({ method: "exec.approval.resolve", id: "abc12345" }); - }); - - it("accepts forwarded Telegram plugin approvals from approvers when native delivery is disabled", async () => { - const params = buildApproveParams( - "/approve plugin:abc12345 allow-once", - createTelegramApproveCfg({ enabled: false, approvers: ["123"], target: "dm" }), - { - Provider: "telegram", - Surface: "telegram", - SenderId: "123", - }, - ); - params.command.isAuthorizedSender = false; - resolveApprovalOverGatewayMock.mockResolvedValue(undefined); - - const result = await handleApproveCommand(params, true); - expect(result?.shouldContinue).toBe(false); - expect(result?.reply?.text).toContain("Approval allow-once submitted"); - expectApprovalResolverCall({ + it.each([ + { + name: "submits approval", + commandBody: "/approve abc allow-once", + cfg: { + commands: { text: true }, + channels: { whatsapp: { allowFrom: ["*"] } }, + } as OpenClawConfig, + ctx: { SenderId: "123" }, + authorized: true, + method: "exec.approval.resolve", + id: "abc", + }, + { + name: "accepts bare approve text for Slack-style manual approvals", + commandBody: "approve abc allow-once", + cfg: { + commands: { text: true }, + channels: { slack: { allowFrom: ["*"] } }, + } as OpenClawConfig, + ctx: { Provider: "slack", Surface: "slack", SenderId: "U123" }, + authorized: true, + method: "exec.approval.resolve", + id: "abc", + }, + { + name: "accepts Telegram /approve from configured approvers even when chat access is otherwise blocked", + commandBody: "/approve abc12345 allow-once", + cfg: createTelegramApproveCfg(), + ctx: { Provider: "telegram", Surface: "telegram", SenderId: "123" }, + authorized: false, + method: "exec.approval.resolve", + id: "abc12345", + }, + { + name: "accepts forwarded Telegram plugin approvals from approvers when native delivery is disabled", + commandBody: "/approve plugin:abc12345 allow-once", + cfg: createTelegramApproveCfg({ enabled: false, approvers: ["123"], target: "dm" }), + ctx: { Provider: "telegram", Surface: "telegram", SenderId: "123" }, + authorized: false, method: "plugin.approval.resolve", id: "plugin:abc12345", - }); + }, + { + name: "accepts Signal /approve from configured approvers even when chat access is otherwise blocked", + commandBody: "/approve abc12345 allow-once", + cfg: { + commands: { text: true }, + channels: { signal: { allowFrom: ["+15551230000"] } }, + } as OpenClawConfig, + ctx: { Provider: "signal", Surface: "signal", SenderId: "+15551230000" }, + authorized: false, + method: "exec.approval.resolve", + id: "abc12345", + }, + { + name: "keeps same-chat /approve available to authorized senders when helper approvers are empty", + commandBody: "/approve abc12345 allow-once", + cfg: { + commands: { text: true }, + channels: { signal: { allowFrom: [] } }, + } as OpenClawConfig, + ctx: { Provider: "signal", Surface: "signal", SenderId: "+15551239999" }, + authorized: true, + method: "exec.approval.resolve", + id: "abc12345", + }, + { + name: "accepts Telegram /approve from exec target recipients when native approvals are disabled", + commandBody: "/approve abc12345 allow-once", + cfg: { + commands: { text: true }, + approvals: { + exec: { + enabled: true, + mode: "targets", + targets: [{ channel: "telegram", to: "123" }], + }, + }, + channels: { telegram: { allowFrom: ["*"] } }, + } as OpenClawConfig, + ctx: { Provider: "telegram", Surface: "telegram", SenderId: "123" }, + authorized: false, + method: "exec.approval.resolve", + id: "abc12345", + }, + ] as const)("$name", async ({ commandBody, cfg, ctx, authorized, method, id }) => { + const params = buildApproveParams(commandBody, cfg, ctx); + params.command.isAuthorizedSender = authorized; + resolveApprovalOverGatewayMock.mockResolvedValue(undefined); + + const result = await handleApproveCommand(params, true); + expect(result?.shouldContinue).toBe(false); + expect(result?.reply?.text).toContain("Approval allow-once submitted"); + expectApprovalResolverCall({ method, id }); }); it("honors the configured default account for omitted-account /approve auth", async () => { @@ -605,80 +623,49 @@ describe("handleApproveCommand", () => { expectApprovalResolverCall({ method: "exec.approval.resolve", id: "abc12345" }); }); - it("accepts Signal /approve from configured approvers even when chat access is otherwise blocked", async () => { - const params = buildApproveParams( - "/approve abc12345 allow-once", - { - commands: { text: true }, - channels: { - signal: { - allowFrom: ["+15551230000"], - }, - }, - } as OpenClawConfig, - { - Provider: "signal", - Surface: "signal", - SenderId: "+15551230000", - }, - ); - params.command.isAuthorizedSender = false; - resolveApprovalOverGatewayMock.mockResolvedValue(undefined); - - const result = await handleApproveCommand(params, true); - expect(result?.shouldContinue).toBe(false); - expect(result?.reply?.text).toContain("Approval allow-once submitted"); - expectApprovalResolverCall({ method: "exec.approval.resolve", id: "abc12345" }); - }); - - it("does not treat implicit default approval auth as a bypass for unauthorized senders", async () => { - const params = buildApproveParams( - "/approve abc12345 allow-once", - { - commands: { text: true }, - } as OpenClawConfig, - { - Provider: "webchat", - Surface: "webchat", - SenderId: "123", - }, - ); - params.command.isAuthorizedSender = false; - - const result = await handleApproveCommand(params, true); - expect(result?.shouldContinue).toBe(false); - expect(result?.reply).toBeUndefined(); - expect(resolveApprovalOverGatewayMock).not.toHaveBeenCalled(); - }); - - it("does not treat implicit same-chat approval auth as a bypass for unauthorized senders", async () => { - setActivePluginRegistry( - createTestRegistry([ - { - pluginId: "slack", - plugin: { - ...createChannelTestPluginBase({ id: "slack", label: "Slack" }), - approvalCapability: { - authorizeActorAction: () => ({ authorized: true }), - getActionAvailabilityState: () => ({ kind: "disabled" }), - }, - }, - source: "test", - }, - ]), - ); - const params = buildApproveParams( - "/approve abc12345 allow-once", - { + it.each([ + { + name: "does not treat implicit default approval auth as a bypass for unauthorized senders", + cfg: { commands: { text: true } } as OpenClawConfig, + ctx: { Provider: "webchat", Surface: "webchat", SenderId: "123" }, + setup: undefined, + }, + { + name: "does not treat implicit same-chat approval auth as a bypass for unauthorized senders", + cfg: { commands: { text: true }, channels: { slack: { allowFrom: ["*"] } }, } as OpenClawConfig, - { - Provider: "slack", - Surface: "slack", - SenderId: "U123", - }, - ); + ctx: { Provider: "slack", Surface: "slack", SenderId: "U123" }, + setup: () => + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "slack", + plugin: { + ...createChannelTestPluginBase({ id: "slack", label: "Slack" }), + approvalCapability: { + authorizeActorAction: () => ({ authorized: true }), + getActionAvailabilityState: () => ({ kind: "disabled" }), + }, + }, + source: "test", + }, + ]), + ), + }, + { + name: "does not allow empty helper approvers to bypass unauthorized sender checks", + cfg: { + commands: { text: true }, + channels: { signal: { allowFrom: [] } }, + } as OpenClawConfig, + ctx: { Provider: "signal", Surface: "signal", SenderId: "+15551239999" }, + setup: undefined, + }, + ] as const)("$name", async ({ cfg, ctx, setup }) => { + setup?.(); + const params = buildApproveParams("/approve abc12345 allow-once", cfg, ctx); params.command.isAuthorizedSender = false; const result = await handleApproveCommand(params, true); @@ -687,90 +674,6 @@ describe("handleApproveCommand", () => { expect(resolveApprovalOverGatewayMock).not.toHaveBeenCalled(); }); - it("does not allow empty helper approvers to bypass unauthorized sender checks", async () => { - const params = buildApproveParams( - "/approve abc12345 allow-once", - { - commands: { text: true }, - channels: { - signal: { - allowFrom: [], - }, - }, - } as OpenClawConfig, - { - Provider: "signal", - Surface: "signal", - SenderId: "+15551239999", - }, - ); - params.command.isAuthorizedSender = false; - - const result = await handleApproveCommand(params, true); - expect(result?.shouldContinue).toBe(false); - expect(result?.reply).toBeUndefined(); - expect(resolveApprovalOverGatewayMock).not.toHaveBeenCalled(); - }); - - it("keeps same-chat /approve available to authorized senders when helper approvers are empty", async () => { - resolveApprovalOverGatewayMock.mockResolvedValue(undefined); - const params = buildApproveParams( - "/approve abc12345 allow-once", - { - commands: { text: true }, - channels: { - signal: { - allowFrom: [], - }, - }, - } as OpenClawConfig, - { - Provider: "signal", - Surface: "signal", - SenderId: "+15551239999", - }, - ); - params.command.isAuthorizedSender = true; - - const result = await handleApproveCommand(params, true); - expect(result?.shouldContinue).toBe(false); - expect(result?.reply?.text).toContain("Approval allow-once submitted"); - expectApprovalResolverCall({ method: "exec.approval.resolve", id: "abc12345" }); - }); - - it("accepts Telegram /approve from exec target recipients when native approvals are disabled", async () => { - const params = buildApproveParams( - "/approve abc12345 allow-once", - { - commands: { text: true }, - approvals: { - exec: { - enabled: true, - mode: "targets", - targets: [{ channel: "telegram", to: "123" }], - }, - }, - channels: { - telegram: { - allowFrom: ["*"], - }, - }, - } as OpenClawConfig, - { - Provider: "telegram", - Surface: "telegram", - SenderId: "123", - }, - ); - params.command.isAuthorizedSender = false; - resolveApprovalOverGatewayMock.mockResolvedValue(undefined); - - const result = await handleApproveCommand(params, true); - expect(result?.shouldContinue).toBe(false); - expect(result?.reply?.text).toContain("Approval allow-once submitted"); - expectApprovalResolverCall({ method: "exec.approval.resolve", id: "abc12345" }); - }); - it("requires configured Discord approvers for exec approvals", async () => { for (const testCase of [ { @@ -1098,4 +1001,3 @@ describe("handleApproveCommand", () => { } }); }); -/* oxlint-disable max-lines -- TODO: split this grandfathered oversized file. */ diff --git a/src/auto-reply/reply/dispatch-from-config.base.test-utils.ts b/src/auto-reply/reply/dispatch-from-config.base.test-utils.ts index b8615952f093..dbf83a844356 100644 --- a/src/auto-reply/reply/dispatch-from-config.base.test-utils.ts +++ b/src/auto-reply/reply/dispatch-from-config.base.test-utils.ts @@ -57,6 +57,36 @@ beforeAll(globalBeforeAll0); describe("dispatchReplyFromConfig", () => { beforeEach(describe0BeforeEach0); + function createActiveSlackThread(userId: string) { + setNoAbort(); + const sessionKey = `agent:main:slack:direct:${userId}`; + const sessionId = "active-session"; + sessionStoreMocks.currentEntry = { sessionId, updatedAt: Date.now() }; + const activeOperation = createReplyOperation({ + sessionKey, + sessionId, + resetTriggered: false, + routeThreadId: "500.000", + }); + activeOperation.setPhase("running"); + return { + activeOperation, + sessionId, + sessionKey, + createCtx: (overrides: Partial = {}) => + buildTestCtx({ + Provider: "slack", + Surface: "slack", + OriginatingChannel: "slack", + OriginatingTo: `user:${userId}`, + ChatType: "direct", + SessionKey: sessionKey, + MessageThreadId: "501.000", + ...overrides, + }), + }; + } + it("loads a registry handle before reading inbound hook state", async () => { setNoAbort(); const cfg = emptyConfig; @@ -730,17 +760,7 @@ describe("dispatchReplyFromConfig", () => { }); it("lets a different Slack DM routed thread reach reply resolution while another thread is active", async () => { - setNoAbort(); - const sessionKey = "agent:main:slack:direct:U1"; - const sessionId = "active-session"; - sessionStoreMocks.currentEntry = { sessionId, updatedAt: Date.now() }; - const activeOperation = createReplyOperation({ - sessionKey, - sessionId, - resetTriggered: false, - routeThreadId: "500.000", - }); - activeOperation.setPhase("running"); + const { activeOperation, createCtx, sessionId, sessionKey } = createActiveSlackThread("U1"); const dispatcher = createDispatcher(); let inBandMutationRan = false; const rotatedSessionId = "rotated-session"; @@ -779,16 +799,7 @@ describe("dispatchReplyFromConfig", () => { try { const resultPromise = dispatchReplyFromConfig({ - ctx: buildTestCtx({ - Provider: "slack", - Surface: "slack", - OriginatingChannel: "slack", - OriginatingTo: "user:U1", - ChatType: "direct", - SessionKey: sessionKey, - MessageThreadId: "501.000", - BodyForAgent: "second top-level DM", - }), + ctx: createCtx({ BodyForAgent: "second top-level DM" }), cfg: emptyConfig, dispatcher, replyResolver, @@ -819,17 +830,12 @@ describe("dispatchReplyFromConfig", () => { }); it("releases a Slack bypass lease when the competing routed thread changes during admission", async () => { - setNoAbort(); - const sessionKey = "agent:main:slack:direct:U2"; - const sessionId = "active-session"; - sessionStoreMocks.currentEntry = { sessionId, updatedAt: Date.now() }; - const originalOperation = createReplyOperation({ - sessionKey, + const { + activeOperation: originalOperation, + createCtx, sessionId, - resetTriggered: false, - routeThreadId: "500.000", - }); - originalOperation.setPhase("running"); + sessionKey, + } = createActiveSlackThread("U2"); let replacementOperation: ReturnType | undefined; let releaseMutation: () => void = () => {}; const mutationGate = new Promise((resolve) => { @@ -870,16 +876,7 @@ describe("dispatchReplyFromConfig", () => { try { const result = await dispatchReplyFromConfig({ - ctx: buildTestCtx({ - Provider: "slack", - Surface: "slack", - OriginatingChannel: "slack", - OriginatingTo: "user:U2", - ChatType: "direct", - SessionKey: sessionKey, - MessageThreadId: "501.000", - BodyForAgent: "same routed thread after replacement", - }), + ctx: createCtx({ BodyForAgent: "same routed thread after replacement" }), cfg: emptyConfig, dispatcher: createDispatcher(), replyResolver, @@ -900,17 +897,7 @@ describe("dispatchReplyFromConfig", () => { }); it("holds a Slack bypass lease until an abort-insensitive resolver settles", async () => { - setNoAbort(); - const sessionKey = "agent:main:slack:direct:U3"; - const sessionId = "active-session"; - sessionStoreMocks.currentEntry = { sessionId, updatedAt: Date.now() }; - const activeOperation = createReplyOperation({ - sessionKey, - sessionId, - resetTriggered: false, - routeThreadId: "500.000", - }); - activeOperation.setPhase("running"); + const { activeOperation, createCtx, sessionId, sessionKey } = createActiveSlackThread("U3"); let releaseResolver: () => void = () => {}; const resolverGate = new Promise((resolve) => { releaseResolver = resolve; @@ -927,16 +914,7 @@ describe("dispatchReplyFromConfig", () => { }); const dispatcher = createDispatcher(); const dispatch = dispatchReplyFromConfig({ - ctx: buildTestCtx({ - Provider: "slack", - Surface: "slack", - OriginatingChannel: "slack", - OriginatingTo: "user:U3", - ChatType: "direct", - SessionKey: sessionKey, - MessageThreadId: "501.000", - BodyForAgent: "abort-insensitive routed thread", - }), + ctx: createCtx({ BodyForAgent: "abort-insensitive routed thread" }), cfg: emptyConfig, dispatcher, replyResolver, @@ -983,17 +961,7 @@ describe("dispatchReplyFromConfig", () => { }); it("bounds Slack bypass lease cleanup when dispatcher idle never settles", async () => { - setNoAbort(); - const sessionKey = "agent:main:slack:direct:U4"; - const sessionId = "active-session"; - sessionStoreMocks.currentEntry = { sessionId, updatedAt: Date.now() }; - const activeOperation = createReplyOperation({ - sessionKey, - sessionId, - resetTriggered: false, - routeThreadId: "500.000", - }); - activeOperation.setPhase("running"); + const { activeOperation, createCtx, sessionId, sessionKey } = createActiveSlackThread("U4"); const dispatcher = createDispatcher(); dispatcher.waitForIdle = vi.fn(async () => await new Promise(() => {})); dispatcher.resolveFollowupAdmissionBarrierTimeoutPolicy = () => ({ @@ -1003,16 +971,7 @@ describe("dispatchReplyFromConfig", () => { try { const result = await dispatchReplyFromConfig({ - ctx: buildTestCtx({ - Provider: "slack", - Surface: "slack", - OriginatingChannel: "slack", - OriginatingTo: "user:U4", - ChatType: "direct", - SessionKey: sessionKey, - MessageThreadId: "501.000", - BodyForAgent: "hung delivery barrier", - }), + ctx: createCtx({ BodyForAgent: "hung delivery barrier" }), cfg: emptyConfig, dispatcher, replyResolver: async () => undefined, @@ -1035,17 +994,7 @@ describe("dispatchReplyFromConfig", () => { }); it("holds a Slack bypass lease until queued delivery settles before revalidation", async () => { - setNoAbort(); - const sessionKey = "agent:main:slack:direct:U5"; - const sessionId = "active-session"; - sessionStoreMocks.currentEntry = { sessionId, updatedAt: Date.now() }; - const activeOperation = createReplyOperation({ - sessionKey, - sessionId, - resetTriggered: false, - routeThreadId: "500.000", - }); - activeOperation.setPhase("running"); + const { activeOperation, createCtx, sessionId, sessionKey } = createActiveSlackThread("U5"); let releaseDelivery: () => void = () => {}; const deliveryGate = new Promise((resolve) => { releaseDelivery = resolve; @@ -1099,16 +1048,7 @@ describe("dispatchReplyFromConfig", () => { try { const dispatch = dispatchReplyFromConfig({ - ctx: buildTestCtx({ - Provider: "slack", - Surface: "slack", - OriginatingChannel: "slack", - OriginatingTo: "user:U5", - ChatType: "direct", - SessionKey: sessionKey, - MessageThreadId: "501.000", - BodyForAgent: "hold queued delivery", - }), + ctx: createCtx({ BodyForAgent: "hold queued delivery" }), cfg: emptyConfig, dispatcher, replyResolver, @@ -1142,17 +1082,7 @@ describe("dispatchReplyFromConfig", () => { }); it("runs ACP tail dispatch inside a borrowed Slack lifecycle admission", async () => { - setNoAbort(); - const sessionKey = "agent:main:slack:direct:U6"; - const sessionId = "active-session"; - sessionStoreMocks.currentEntry = { sessionId, updatedAt: Date.now() }; - const activeOperation = createReplyOperation({ - sessionKey, - sessionId, - resetTriggered: false, - routeThreadId: "500.000", - }); - activeOperation.setPhase("running"); + const { activeOperation, createCtx, sessionId, sessionKey } = createActiveSlackThread("U6"); let initiatingAdmissionExcluded = false; let mutationRan = false; hookMocks.runner.hasHooks.mockImplementation( @@ -1185,14 +1115,7 @@ describe("dispatchReplyFromConfig", () => { try { const result = await dispatchReplyFromConfig({ - ctx: buildTestCtx({ - Provider: "slack", - Surface: "slack", - OriginatingChannel: "slack", - OriginatingTo: "user:U6", - ChatType: "direct", - SessionKey: sessionKey, - MessageThreadId: "501.000", + ctx: createCtx({ BodyForAgent: "run tail after reset", AcpDispatchTailAfterReset: true, }), diff --git a/src/auto-reply/reply/dispatch-from-config.send-policy-routing.test-utils.ts b/src/auto-reply/reply/dispatch-from-config.send-policy-routing.test-utils.ts index 9be61b7beba8..355322eea94d 100644 --- a/src/auto-reply/reply/dispatch-from-config.send-policy-routing.test-utils.ts +++ b/src/auto-reply/reply/dispatch-from-config.send-policy-routing.test-utils.ts @@ -524,355 +524,189 @@ describe("sendPolicy deny — suppress delivery, not processing (#53328)", () => } }); - it("keeps Codex direct source delivery message-tool-only when config is unset", async () => { + type HarnessDeliveryCase = { + name: string; + harnessId?: string; + supportsProvider?: string; + currentEntry: typeof sessionStoreMocks.currentEntry; + ctx: Partial; + cfg: OpenClawConfig; + replyOptions?: GetReplyOptions; + expectedMode: "automatic" | "message_tool_only"; + text: string; + }; + + async function runHarnessDeliveryCase(testCase: HarnessDeliveryCase) { setNoAbort(); + const harnessId = testCase.harnessId ?? "codex"; registerAgentHarness({ - id: "codex", - label: "Codex", + id: harnessId, + label: harnessId === "codex" ? "Codex" : "Custom", deliveryDefaults: { visibleReplies: "message_tool" }, - supports: () => ({ supported: true, priority: 100 }), + supports: (ctx) => + !testCase.supportsProvider || ctx.provider === testCase.supportsProvider + ? { supported: true, priority: harnessId === "codex" ? 100 : 200 } + : { supported: false, reason: `${testCase.supportsProvider} provider only` }, runAttempt: vi.fn(async () => ({}) as never), }); - sessionStoreMocks.currentEntry = { - sessionId: "s1", - updatedAt: 0, - agentHarnessId: "codex", - sendPolicy: "allow", - }; + sessionStoreMocks.currentEntry = structuredClone(testCase.currentEntry); const dispatcher = createDispatcher(); const replyResolver = vi.fn(async (_ctx: MsgContext, opts?: GetReplyOptions) => { - expect(opts?.sourceReplyDeliveryMode).toBe("message_tool_only"); - return { text: "private final reply" } satisfies ReplyPayload; + expect(opts?.sourceReplyDeliveryMode).toBe(testCase.expectedMode); + return { text: testCase.text } satisfies ReplyPayload; }); const result = await dispatchReplyFromConfig({ - ctx: buildTestCtx({ - ChatType: "direct", - CommandSource: undefined, - SessionKey: "agent:main:main", - }), - cfg: emptyConfig, + ctx: buildTestCtx(testCase.ctx), + cfg: testCase.cfg, dispatcher, + replyOptions: testCase.replyOptions, replyResolver, }); expect(replyResolver).toHaveBeenCalledTimes(1); - expect(result.queuedFinal).toBe(false); - expect(dispatcher.sendFinalReply).not.toHaveBeenCalled(); - }); + expect(result.queuedFinal).toBe(testCase.expectedMode === "automatic"); + if (testCase.expectedMode === "automatic") { + expect(firstFinalReplyPayload(dispatcher)?.text).toBe(testCase.text); + } else { + expect(result.sourceReplyDeliveryMode).toBe("message_tool_only"); + expect(dispatcher.sendFinalReply).not.toHaveBeenCalled(); + } + } - it("keeps locked supervised Codex delivery defaults across outer model overrides", async () => { - setNoAbort(); - registerAgentHarness({ - id: "codex", - label: "Codex", - deliveryDefaults: { visibleReplies: "message_tool" }, - supports: (ctx) => - ctx.provider === "codex" - ? { supported: true, priority: 100 } - : { supported: false, reason: "codex provider only" }, - runAttempt: vi.fn(async () => ({}) as never), - }); - sessionStoreMocks.currentEntry = { - sessionId: "catalog-adopted-session", - updatedAt: 0, - agentHarnessId: "codex", - modelSelectionLocked: true, - pluginExtensions: { - codex: { - supervision: { - sourceThreadId: "019f-codex-thread", - modelLocked: true, - }, + const directCtx = { + ChatType: "direct", + CommandSource: undefined, + SessionKey: "agent:main:main", + } satisfies Partial; + const telegramDirectCtx = { + ...directCtx, + Provider: "telegram", + Surface: "telegram", + SessionKey: "agent:main:telegram:direct:U1", + } satisfies Partial; + const codexEntry = { + sessionId: "s1", + updatedAt: 0, + agentHarnessId: "codex", + sendPolicy: "allow", + } as const; + const cachedCodexEntry = { + ...codexEntry, + modelProvider: "codex", + model: "gpt-5.5", + } as const; + const channelModelConfig = { + channels: { modelByChannel: { telegram: { "*": "anthropic/claude-sonnet-4.6" } } }, + } as OpenClawConfig; + + it.each([ + { + name: "keeps Codex direct source delivery message-tool-only when config is unset", + currentEntry: codexEntry, + ctx: directCtx, + cfg: emptyConfig, + expectedMode: "message_tool_only", + text: "private final reply", + }, + { + name: "keeps locked supervised Codex delivery defaults across outer model overrides", + supportsProvider: "codex", + currentEntry: { + ...codexEntry, + sessionId: "catalog-adopted-session", + modelSelectionLocked: true, + pluginExtensions: { + codex: { supervision: { sourceThreadId: "019f-codex-thread", modelLocked: true } }, }, + providerOverride: "anthropic", + modelOverride: "claude-sonnet-4.6", }, - providerOverride: "anthropic", - modelOverride: "claude-sonnet-4.6", - sendPolicy: "allow", - }; - const dispatcher = createDispatcher(); - const replyResolver = vi.fn(async (_ctx: MsgContext, opts?: GetReplyOptions) => { - expect(opts?.sourceReplyDeliveryMode).toBe("message_tool_only"); - return { text: "private supervised reply" } satisfies ReplyPayload; - }); - - const result = await dispatchReplyFromConfig({ - ctx: buildTestCtx({ - ChatType: "direct", - CommandSource: undefined, - SessionKey: "agent:main:main", - }), + ctx: directCtx, cfg: emptyConfig, - dispatcher, - replyResolver, - }); - - expect(result.queuedFinal).toBe(false); - expect(result.sourceReplyDeliveryMode).toBe("message_tool_only"); - expect(dispatcher.sendFinalReply).not.toHaveBeenCalled(); - }); - - it("uses Codex direct source delivery defaults before a session entry exists", async () => { - setNoAbort(); - registerAgentHarness({ - id: "codex", - label: "Codex", - deliveryDefaults: { visibleReplies: "message_tool" }, - supports: () => ({ supported: true, priority: 100 }), - runAttempt: vi.fn(async () => ({}) as never), - }); - sessionStoreMocks.currentEntry = undefined; - const dispatcher = createDispatcher(); - const replyResolver = vi.fn(async (_ctx: MsgContext, opts?: GetReplyOptions) => { - expect(opts?.sourceReplyDeliveryMode).toBe("message_tool_only"); - return { text: "private first reply" } satisfies ReplyPayload; - }); - - const result = await dispatchReplyFromConfig({ - ctx: buildTestCtx({ - ChatType: "direct", - CommandSource: undefined, - Provider: "telegram", - Surface: "telegram", - SessionKey: "agent:main:telegram:direct:U1", - }), + expectedMode: "message_tool_only", + text: "private supervised reply", + }, + { + name: "uses Codex direct source delivery defaults before a session entry exists", + currentEntry: undefined, + ctx: telegramDirectCtx, cfg: emptyConfig, - dispatcher, - replyResolver, - }); - - expect(replyResolver).toHaveBeenCalledTimes(1); - expect(result.queuedFinal).toBe(false); - expect(dispatcher.sendFinalReply).not.toHaveBeenCalled(); - }); - - it("uses channel model overrides before Codex first-turn direct source delivery defaults", async () => { - setNoAbort(); - registerAgentHarness({ - id: "codex", - label: "Codex", - deliveryDefaults: { visibleReplies: "message_tool" }, - supports: (ctx) => - ctx.provider === "codex" - ? { supported: true, priority: 100 } - : { supported: false, reason: "codex provider only" }, - runAttempt: vi.fn(async () => ({}) as never), - }); - sessionStoreMocks.currentEntry = undefined; - const dispatcher = createDispatcher(); - const replyResolver = vi.fn(async (_ctx: MsgContext, opts?: GetReplyOptions) => { - expect(opts?.sourceReplyDeliveryMode).toBe("automatic"); - return { text: "visible channel-model reply" } satisfies ReplyPayload; - }); - - const result = await dispatchReplyFromConfig({ - ctx: buildTestCtx({ - ChatType: "direct", - CommandSource: undefined, - Provider: "telegram", - Surface: "telegram", - SessionKey: "agent:main:telegram:direct:U1", - }), + expectedMode: "message_tool_only", + text: "private first reply", + }, + { + name: "uses channel model overrides before Codex first-turn direct source delivery defaults", + supportsProvider: "codex", + currentEntry: undefined, + ctx: telegramDirectCtx, + cfg: channelModelConfig, + expectedMode: "automatic", + text: "visible channel-model reply", + }, + { + name: "uses channel model overrides before cached Codex runtime defaults", + supportsProvider: "codex", + currentEntry: { ...cachedCodexEntry, channel: "telegram" }, + ctx: telegramDirectCtx, + cfg: channelModelConfig, + expectedMode: "automatic", + text: "visible existing-channel-model reply", + }, + { + name: "uses configured defaults before cached Codex runtime metadata", + supportsProvider: "codex", + currentEntry: cachedCodexEntry, + ctx: telegramDirectCtx, cfg: { - channels: { - modelByChannel: { - telegram: { - "*": "anthropic/claude-sonnet-4.6", - }, - }, - }, + agents: { defaults: { model: { primary: "anthropic/claude-sonnet-4.6" } } }, } as OpenClawConfig, - dispatcher, - replyResolver, - }); - - expect(replyResolver).toHaveBeenCalledTimes(1); - expect(result.queuedFinal).toBe(true); - expect(firstFinalReplyPayload(dispatcher)?.text).toBe("visible channel-model reply"); - }); - - it("uses channel model overrides before cached Codex runtime defaults", async () => { - setNoAbort(); - registerAgentHarness({ - id: "codex", - label: "Codex", - deliveryDefaults: { visibleReplies: "message_tool" }, - supports: (ctx) => - ctx.provider === "codex" - ? { supported: true, priority: 100 } - : { supported: false, reason: "codex provider only" }, - runAttempt: vi.fn(async () => ({}) as never), - }); - sessionStoreMocks.currentEntry = { - sessionId: "s1", - updatedAt: 0, - agentHarnessId: "codex", - modelProvider: "codex", - model: "gpt-5.5", - channel: "telegram", - sendPolicy: "allow", - }; - const dispatcher = createDispatcher(); - const replyResolver = vi.fn(async (_ctx: MsgContext, opts?: GetReplyOptions) => { - expect(opts?.sourceReplyDeliveryMode).toBe("automatic"); - return { text: "visible existing-channel-model reply" } satisfies ReplyPayload; - }); - - const result = await dispatchReplyFromConfig({ - ctx: buildTestCtx({ - ChatType: "direct", - CommandSource: undefined, - Provider: "telegram", - Surface: "telegram", - SessionKey: "agent:main:telegram:direct:U1", - }), - cfg: { - channels: { - modelByChannel: { - telegram: { - "*": "anthropic/claude-sonnet-4.6", - }, - }, - }, - } as OpenClawConfig, - dispatcher, - replyResolver, - }); - - expect(replyResolver).toHaveBeenCalledTimes(1); - expect(result.queuedFinal).toBe(true); - expect(firstFinalReplyPayload(dispatcher)?.text).toBe("visible existing-channel-model reply"); - }); - - it("uses configured defaults before cached Codex runtime metadata", async () => { - setNoAbort(); - registerAgentHarness({ - id: "codex", - label: "Codex", - deliveryDefaults: { visibleReplies: "message_tool" }, - supports: (ctx) => - ctx.provider === "codex" - ? { supported: true, priority: 100 } - : { supported: false, reason: "codex provider only" }, - runAttempt: vi.fn(async () => ({}) as never), - }); - sessionStoreMocks.currentEntry = { - sessionId: "s1", - updatedAt: 0, - agentHarnessId: "codex", - modelProvider: "codex", - model: "gpt-5.5", - sendPolicy: "allow", - }; - const dispatcher = createDispatcher(); - const replyResolver = vi.fn(async (_ctx: MsgContext, opts?: GetReplyOptions) => { - expect(opts?.sourceReplyDeliveryMode).toBe("automatic"); - return { text: "visible configured-default reply" } satisfies ReplyPayload; - }); - - const result = await dispatchReplyFromConfig({ - ctx: buildTestCtx({ - ChatType: "direct", - CommandSource: undefined, - Provider: "telegram", - Surface: "telegram", - SessionKey: "agent:main:telegram:direct:U1", - }), - cfg: { - agents: { - defaults: { - model: { primary: "anthropic/claude-sonnet-4.6" }, - }, - }, - } as OpenClawConfig, - dispatcher, - replyResolver, - }); - - expect(replyResolver).toHaveBeenCalledTimes(1); - expect(result.queuedFinal).toBe(true); - expect(firstFinalReplyPayload(dispatcher)?.text).toBe("visible configured-default reply"); - }); - - it("lets config restore automatic Codex direct source delivery", async () => { - setNoAbort(); - registerAgentHarness({ - id: "codex", - label: "Codex", - deliveryDefaults: { visibleReplies: "message_tool" }, - supports: () => ({ supported: true, priority: 100 }), - runAttempt: vi.fn(async () => ({}) as never), - }); - sessionStoreMocks.currentEntry = { - sessionId: "s1", - updatedAt: 0, - agentHarnessId: "codex", - sendPolicy: "allow", - }; - const dispatcher = createDispatcher(); - const replyResolver = vi.fn(async (_ctx: MsgContext, opts?: GetReplyOptions) => { - expect(opts?.sourceReplyDeliveryMode).toBe("automatic"); - return { text: "visible final reply" } satisfies ReplyPayload; - }); - - const result = await dispatchReplyFromConfig({ - ctx: buildTestCtx({ - ChatType: "direct", - CommandSource: undefined, - SessionKey: "agent:main:main", - }), + expectedMode: "automatic", + text: "visible configured-default reply", + }, + { + name: "lets config restore automatic Codex direct source delivery", + currentEntry: codexEntry, + ctx: directCtx, cfg: { messages: { visibleReplies: "automatic" } } as OpenClawConfig, - dispatcher, - replyResolver, - }); - - expect(replyResolver).toHaveBeenCalledTimes(1); - expect(result.queuedFinal).toBe(true); - expect(firstFinalReplyPayload(dispatcher)?.text).toBe("visible final reply"); - }); - - it("honors model overrides before cached Codex direct source delivery defaults", async () => { - setNoAbort(); - registerAgentHarness({ - id: "codex", - label: "Codex", - deliveryDefaults: { visibleReplies: "message_tool" }, - supports: (ctx) => - ctx.provider === "codex" - ? { supported: true, priority: 100 } - : { supported: false, reason: "codex provider only" }, - runAttempt: vi.fn(async () => ({}) as never), - }); - sessionStoreMocks.currentEntry = { - sessionId: "s1", - updatedAt: 0, - agentHarnessId: "codex", - agentRuntimeOverride: "codex", - providerOverride: "anthropic", - modelOverride: "claude-sonnet-4.6", - sendPolicy: "allow", - }; - const dispatcher = createDispatcher(); - const replyResolver = vi.fn(async (_ctx: MsgContext, opts?: GetReplyOptions) => { - expect(opts?.sourceReplyDeliveryMode).toBe("automatic"); - return { text: "visible switched-model reply" } satisfies ReplyPayload; - }); - - const result = await dispatchReplyFromConfig({ - ctx: buildTestCtx({ - ChatType: "direct", - CommandSource: undefined, - SessionKey: "agent:main:main", - }), + expectedMode: "automatic", + text: "visible final reply", + }, + { + name: "honors model overrides before cached Codex direct source delivery defaults", + supportsProvider: "codex", + currentEntry: { + ...codexEntry, + agentRuntimeOverride: "codex", + providerOverride: "anthropic", + modelOverride: "claude-sonnet-4.6", + }, + ctx: directCtx, cfg: emptyConfig, - dispatcher, - replyResolver, - }); - - expect(replyResolver).toHaveBeenCalledTimes(1); - expect(result.queuedFinal).toBe(true); - expect(firstFinalReplyPayload(dispatcher)?.text).toBe("visible switched-model reply"); - }); + expectedMode: "automatic", + text: "visible switched-model reply", + }, + { + name: "honors heartbeat model overrides before Codex direct source delivery defaults", + supportsProvider: "codex", + currentEntry: codexEntry, + ctx: telegramDirectCtx, + cfg: emptyConfig, + replyOptions: { isHeartbeat: true, heartbeatModelOverride: "anthropic/claude-sonnet-4.6" }, + expectedMode: "automatic", + text: "visible heartbeat-model reply", + }, + { + name: "preserves non-Codex harness direct source delivery defaults", + harnessId: "custom", + supportsProvider: "custom", + currentEntry: { ...codexEntry, agentHarnessId: "custom" }, + ctx: { ...directCtx, Provider: "custom" }, + cfg: emptyConfig, + expectedMode: "message_tool_only", + text: "private final reply", + }, + ] satisfies HarnessDeliveryCase[])("$name", runHarnessDeliveryCase); it("honors parent model overrides before Codex direct source delivery defaults", async () => { setNoAbort(); @@ -943,137 +777,60 @@ describe("sendPolicy deny — suppress delivery, not processing (#53328)", () => sessionStoreMocks.loadSessionStoreEntry.mockImplementation(defaultLoadSessionStoreEntry); }); - it("honors heartbeat model overrides before Codex direct source delivery defaults", async () => { + async function expectAutomaticDelivery(params: { + ctx: Partial; + cfg: OpenClawConfig; + text: string; + replyOptions?: GetReplyOptions; + checkTyping?: boolean; + }) { setNoAbort(); - registerAgentHarness({ - id: "codex", - label: "Codex", - deliveryDefaults: { visibleReplies: "message_tool" }, - supports: (ctx) => - ctx.provider === "codex" - ? { supported: true, priority: 100 } - : { supported: false, reason: "codex provider only" }, - runAttempt: vi.fn(async () => ({}) as never), - }); - sessionStoreMocks.currentEntry = { - sessionId: "s1", - updatedAt: 0, - agentHarnessId: "codex", - sendPolicy: "allow", - }; const dispatcher = createDispatcher(); const replyResolver = vi.fn(async (_ctx: MsgContext, opts?: GetReplyOptions) => { expect(opts?.sourceReplyDeliveryMode).toBe("automatic"); - return { text: "visible heartbeat-model reply" } satisfies ReplyPayload; + if (params.checkTyping) { + expect(opts?.suppressTyping).toBe(false); + } + return { text: params.text } satisfies ReplyPayload; }); - const result = await dispatchReplyFromConfig({ - ctx: buildTestCtx({ - ChatType: "direct", - CommandSource: undefined, - Provider: "telegram", - Surface: "telegram", - SessionKey: "agent:main:telegram:direct:U1", - }), - cfg: emptyConfig, + ctx: buildTestCtx(params.ctx), + cfg: params.cfg, dispatcher, - replyOptions: { - isHeartbeat: true, - heartbeatModelOverride: "anthropic/claude-sonnet-4.6", - }, + replyOptions: params.replyOptions, replyResolver, }); expect(replyResolver).toHaveBeenCalledTimes(1); expect(result.queuedFinal).toBe(true); - expect(firstFinalReplyPayload(dispatcher)?.text).toBe("visible heartbeat-model reply"); - }); - - it("preserves non-Codex harness direct source delivery defaults", async () => { - setNoAbort(); - registerAgentHarness({ - id: "custom", - label: "Custom", - deliveryDefaults: { visibleReplies: "message_tool" }, - supports: (ctx) => - ctx.provider === "custom" - ? { supported: true, priority: 200 } - : { supported: false, reason: "custom provider only" }, - runAttempt: vi.fn(async () => ({}) as never), - }); - sessionStoreMocks.currentEntry = { - sessionId: "s1", - updatedAt: 0, - agentHarnessId: "custom", - sendPolicy: "allow", - }; - const dispatcher = createDispatcher(); - const replyResolver = vi.fn(async (_ctx: MsgContext, opts?: GetReplyOptions) => { - expect(opts?.sourceReplyDeliveryMode).toBe("message_tool_only"); - return { text: "private final reply" } satisfies ReplyPayload; - }); - - const result = await dispatchReplyFromConfig({ - ctx: buildTestCtx({ - ChatType: "direct", - CommandSource: undefined, - Provider: "custom", - SessionKey: "agent:main:main", - }), - cfg: emptyConfig, - dispatcher, - replyResolver, - }); - - expect(replyResolver).toHaveBeenCalledTimes(1); - expect(result.queuedFinal).toBe(false); - expect(dispatcher.sendFinalReply).not.toHaveBeenCalled(); - }); + expect(firstFinalReplyPayload(dispatcher)?.text).toBe(params.text); + } it("falls back to automatic group/channel delivery when the message tool is unavailable", async () => { - setNoAbort(); - const dispatcher = createDispatcher(); - const replyResolver = vi.fn(async (_ctx: MsgContext, opts?: GetReplyOptions) => { - expect(opts?.sourceReplyDeliveryMode).toBe("automatic"); - return { text: "visible fallback" } satisfies ReplyPayload; - }); - - const result = await dispatchReplyFromConfig({ - ctx: buildTestCtx({ + await expectAutomaticDelivery({ + ctx: { ChatType: "channel", SessionKey: "test:discord:channel:C1", - }), + }, cfg: { messages: { groupChat: { visibleReplies: "message_tool" }, }, tools: { allow: ["read"] }, } as OpenClawConfig, - dispatcher, - replyResolver, + text: "visible fallback", }); - - expect(replyResolver).toHaveBeenCalledTimes(1); - expect(result.queuedFinal).toBe(true); - expect(firstFinalReplyPayload(dispatcher)?.text).toBe("visible fallback"); }); it("falls back to automatic group/channel delivery when group tools remove the message tool", async () => { - setNoAbort(); - const dispatcher = createDispatcher(); - const replyResolver = vi.fn(async (_ctx: MsgContext, opts?: GetReplyOptions) => { - expect(opts?.sourceReplyDeliveryMode).toBe("automatic"); - return { text: "group policy fallback" } satisfies ReplyPayload; - }); - - const result = await dispatchReplyFromConfig({ - ctx: buildTestCtx({ + await expectAutomaticDelivery({ + ctx: { ChatType: "channel", From: "discord:channel:C1", Provider: "discord", Surface: "discord", SessionKey: "agent:main:discord:channel:C1", - }), + }, cfg: { messages: { groupChat: { visibleReplies: "message_tool" }, @@ -1086,90 +843,49 @@ describe("sendPolicy deny — suppress delivery, not processing (#53328)", () => }, }, } as OpenClawConfig, - dispatcher, - replyResolver, + text: "group policy fallback", }); - - expect(replyResolver).toHaveBeenCalledTimes(1); - expect(result.queuedFinal).toBe(true); - expect(firstFinalReplyPayload(dispatcher)?.text).toBe("group policy fallback"); }); it("falls back when a channel precomputed message-tool-only delivery but the message tool is unavailable", async () => { - setNoAbort(); - const dispatcher = createDispatcher(); - const replyResolver = vi.fn(async (_ctx: MsgContext, opts?: GetReplyOptions) => { - expect(opts?.sourceReplyDeliveryMode).toBe("automatic"); - return { text: "requested fallback" } satisfies ReplyPayload; - }); - - const result = await dispatchReplyFromConfig({ - ctx: buildTestCtx({ + await expectAutomaticDelivery({ + ctx: { ChatType: "channel", SessionKey: "test:discord:channel:C1", - }), + }, cfg: { tools: { allow: ["read"] } } as OpenClawConfig, - dispatcher, - replyResolver, replyOptions: { sourceReplyDeliveryMode: "message_tool_only", }, + text: "requested fallback", }); - - expect(replyResolver).toHaveBeenCalledTimes(1); - expect(result.queuedFinal).toBe(true); - expect(firstFinalReplyPayload(dispatcher)?.text).toBe("requested fallback"); }); it("keeps native command replies visible in group/channel events", async () => { - setNoAbort(); - const dispatcher = createDispatcher(); - const replyResolver = vi.fn(async (_ctx: MsgContext, opts?: GetReplyOptions) => { - expect(opts?.sourceReplyDeliveryMode).toBe("automatic"); - expect(opts?.suppressTyping).toBe(false); - return { text: "status reply" } satisfies ReplyPayload; - }); - - const result = await dispatchReplyFromConfig({ - ctx: buildTestCtx({ + await expectAutomaticDelivery({ + ctx: { ChatType: "group", CommandSource: "native", CommandAuthorized: true, WasMentioned: true, SessionKey: "test:telegram:group:G1", - }), + }, cfg: emptyConfig, - dispatcher, - replyResolver, + text: "status reply", + checkTyping: true, }); - - expect(replyResolver).toHaveBeenCalledTimes(1); - expect(result.queuedFinal).toBe(true); - expect(firstFinalReplyPayload(dispatcher)?.text).toBe("status reply"); }); it("keeps default group/channel source delivery automatic", async () => { - setNoAbort(); - const dispatcher = createDispatcher(); - const replyResolver = vi.fn(async (_ctx: MsgContext, opts?: GetReplyOptions) => { - expect(opts?.sourceReplyDeliveryMode).toBe("automatic"); - return { text: "final reply" } satisfies ReplyPayload; - }); - - const result = await dispatchReplyFromConfig({ - ctx: buildTestCtx({ + await expectAutomaticDelivery({ + ctx: { ChatType: "group", WasMentioned: true, SessionKey: "test:telegram:group:G1", - }), + }, cfg: emptyConfig, - dispatcher, - replyResolver, + text: "final reply", }); - - expect(replyResolver).toHaveBeenCalledTimes(1); - expect(result.queuedFinal).toBe(true); - expect(firstFinalReplyPayload(dispatcher)?.text).toBe("final reply"); }); }); /* oxlint-disable max-lines -- TODO: split this grandfathered oversized file. */ diff --git a/src/auto-reply/reply/followup-turn-admission.test.ts b/src/auto-reply/reply/followup-turn-admission.test.ts index 6d97daea0c9b..e08b59f552dc 100644 --- a/src/auto-reply/reply/followup-turn-admission.test.ts +++ b/src/auto-reply/reply/followup-turn-admission.test.ts @@ -304,92 +304,63 @@ describe("admitFollowupTurn", () => { expect(state.refreshGoal).toHaveBeenCalledWith(undefined, undefined); }); - it("restores the item when persisted state changes generation after admission", async () => { - const operation = createOperation(); - const initialEntry: SessionEntry = { sessionId: "queued-session", updatedAt: 1 }; - const replacementEntry: SessionEntry = { sessionId: "replacement-session", updatedAt: 2 }; - state.admitReply.mockResolvedValue({ status: "owned", operation, sessionEntry: initialEntry }); - state.loadEntry.mockReturnValue(replacementEntry); - - await expect( - admitFollowupTurn({ - queued: createRun(), - defaults: createDefaults({ sessionEntry: initialEntry, storePath: "/tmp/sessions.json" }), - }), - ).rejects.toThrow("Follow-up session generation changed after reply admission"); - expect(operation.complete).toHaveBeenCalledOnce(); - expect(state.preflight).not.toHaveBeenCalled(); - }); - - it("restores the item when persisted lifecycle revision changes after admission", async () => { + it.each([ + { + name: "restores the item when persisted state changes generation after admission", + mode: "persisted-session", + }, + { + name: "restores the item when persisted lifecycle revision changes after admission", + mode: "persisted-revision", + }, + { + name: "restores the item when an in-memory generation changes while admission awaits", + mode: "memory", + }, + { + name: "restores the item when the admitted persisted generation disappears", + mode: "disappeared", + }, + ] as const)("$name", async ({ mode }) => { const operation = createOperation(); + const hasRevision = mode === "persisted-revision" || mode === "memory"; const initialEntry: SessionEntry = { sessionId: "queued-session", - lifecycleRevision: "admitted", + ...(hasRevision ? { lifecycleRevision: "admitted" } : {}), updatedAt: 1, }; const replacementEntry: SessionEntry = { - ...initialEntry, - lifecycleRevision: "replacement", - updatedAt: 2, - }; - state.admitReply.mockResolvedValue({ status: "owned", operation, sessionEntry: initialEntry }); - state.loadEntry.mockReturnValue(replacementEntry); - - await expect( - admitFollowupTurn({ - queued: createRun(), - defaults: createDefaults({ sessionEntry: initialEntry, storePath: "/tmp/sessions.json" }), - }), - ).rejects.toThrow("Follow-up session generation changed after reply admission"); - expect(operation.complete).toHaveBeenCalledOnce(); - expect(state.preflight).not.toHaveBeenCalled(); - }); - - it("restores the item when an in-memory generation changes while admission awaits", async () => { - const operation = createOperation(); - const initialEntry: SessionEntry = { - sessionId: "queued-session", - lifecycleRevision: "admitted", - updatedAt: 1, - }; - const replacementEntry: SessionEntry = { - sessionId: "replacement-session", - lifecycleRevision: "replacement", + ...(mode === "persisted-revision" ? initialEntry : {}), + sessionId: mode === "persisted-revision" ? initialEntry.sessionId : "replacement-session", + lifecycleRevision: hasRevision ? "replacement" : undefined, updatedAt: 2, }; const sessionStore = { main: initialEntry }; state.admitReply.mockResolvedValue({ status: "owned", operation, sessionEntry: initialEntry }); - const onQueuedFollowupAdmitted = vi.fn(async () => { - sessionStore.main = replacementEntry; - }); - - await expect( - admitFollowupTurn({ - queued: createRun(), - defaults: createDefaults({ - sessionEntry: initialEntry, - sessionStore, - opts: { onQueuedFollowupAdmitted }, + if (mode === "memory") { + await expect( + admitFollowupTurn({ + queued: createRun(), + defaults: createDefaults({ + sessionEntry: initialEntry, + sessionStore, + opts: { + onQueuedFollowupAdmitted: vi.fn(async () => { + sessionStore.main = replacementEntry; + }), + }, + }), }), - }), - ).rejects.toThrow("Follow-up session generation changed after reply admission"); - expect(operation.complete).toHaveBeenCalledOnce(); - expect(state.preflight).not.toHaveBeenCalled(); - }); - - it("restores the item when the admitted persisted generation disappears", async () => { - const operation = createOperation(); - const initialEntry: SessionEntry = { sessionId: "queued-session", updatedAt: 1 }; - state.admitReply.mockResolvedValue({ status: "owned", operation, sessionEntry: initialEntry }); - state.loadEntry.mockReturnValue(undefined); - - await expect( - admitFollowupTurn({ - queued: createRun(), - defaults: createDefaults({ sessionEntry: initialEntry, storePath: "/tmp/sessions.json" }), - }), - ).rejects.toThrow("Follow-up session generation changed after reply admission"); + ).rejects.toThrow("Follow-up session generation changed after reply admission"); + } else { + state.loadEntry.mockReturnValue(mode === "disappeared" ? undefined : replacementEntry); + await expect( + admitFollowupTurn({ + queued: createRun(), + defaults: createDefaults({ sessionEntry: initialEntry, storePath: "/tmp/sessions.json" }), + }), + ).rejects.toThrow("Follow-up session generation changed after reply admission"); + } expect(operation.complete).toHaveBeenCalledOnce(); expect(state.preflight).not.toHaveBeenCalled(); }); @@ -667,82 +638,80 @@ describe("admitFollowupTurn", () => { expect(operation.complete).toHaveBeenCalledOnce(); }); - it("restores the item when preflight adoption races a replacement generation", async () => { + it.each([ + { + name: "restores the item when preflight adoption races a replacement generation", + outcome: "rotated", + mutation: "replace", + loadPersisted: true, + error: "Follow-up session generation changed", + checksFailureText: true, + }, + { + name: "restores the item when a no-op preflight observes a replacement generation", + outcome: "initial", + mutation: "replace", + loadPersisted: true, + error: "Follow-up session generation changed", + checksFailureText: false, + }, + { + name: "restores the item when a successful preflight observes in-memory deletion", + outcome: "initial", + mutation: "delete", + loadPersisted: false, + error: "Follow-up session generation changed", + checksFailureText: false, + }, + { + name: "restores the item when a failing preflight observes a replacement generation", + outcome: "failure", + mutation: "replace", + loadPersisted: false, + error: "Follow-up session generation changed after reply admission", + checksFailureText: true, + }, + { + name: "restores the item when a failing preflight observes in-memory deletion", + outcome: "failure", + mutation: "delete", + loadPersisted: false, + error: "Follow-up session generation changed", + checksFailureText: true, + }, + ] as const)("$name", async ({ outcome, mutation, loadPersisted, error, checksFailureText }) => { const operation = createOperation(); const initialEntry: SessionEntry = { sessionId: "queued-session", - lifecycleRevision: "initial", - updatedAt: 1, - }; - const rotatedEntry: SessionEntry = { - sessionId: "compacted-session", - lifecycleRevision: "compacted", - updatedAt: 2, - }; - const replacementEntry: SessionEntry = { - sessionId: "replacement-session", - lifecycleRevision: "replacement", - updatedAt: 3, - }; - const sessionStore = { main: initialEntry }; - state.admitReply.mockResolvedValue({ status: "owned", operation, sessionEntry: initialEntry }); - state.loadEntry.mockReturnValue(initialEntry); - state.preflight.mockImplementation(async () => { - sessionStore.main = replacementEntry; - return rotatedEntry; - }); - - await expect( - admitFollowupTurn({ - queued: createRun(), - defaults: createDefaults({ sessionStore, sessionEntry: initialEntry }), - }), - ).rejects.toThrow("Follow-up session generation changed"); - expect(operation.complete).toHaveBeenCalledOnce(); - expect(state.buildPreflightFailureText).not.toHaveBeenCalled(); - }); - - it("restores the item when a no-op preflight observes a replacement generation", async () => { - const operation = createOperation(); - const initialEntry: SessionEntry = { - sessionId: "queued-session", - lifecycleRevision: "initial", + lifecycleRevision: outcome === "failure" ? "admitted" : "initial", updatedAt: 1, }; const replacementEntry: SessionEntry = { sessionId: "replacement-session", lifecycleRevision: "replacement", - updatedAt: 2, - }; - const sessionStore = { main: initialEntry }; - state.admitReply.mockResolvedValue({ status: "owned", operation, sessionEntry: initialEntry }); - state.loadEntry.mockReturnValue(initialEntry); - state.preflight.mockImplementation(async () => { - sessionStore.main = replacementEntry; - return initialEntry; - }); - - await expect( - admitFollowupTurn({ - queued: createRun(), - defaults: createDefaults({ sessionStore, sessionEntry: initialEntry }), - }), - ).rejects.toThrow("Follow-up session generation changed"); - expect(operation.complete).toHaveBeenCalledOnce(); - }); - - it("restores the item when a successful preflight observes in-memory deletion", async () => { - const operation = createOperation(); - const initialEntry: SessionEntry = { - sessionId: "queued-session", - lifecycleRevision: "initial", - updatedAt: 1, + updatedAt: outcome === "rotated" ? 3 : 2, }; const sessionStore: Record = { main: initialEntry }; state.admitReply.mockResolvedValue({ status: "owned", operation, sessionEntry: initialEntry }); + if (loadPersisted) { + state.loadEntry.mockReturnValue(initialEntry); + } state.preflight.mockImplementation(async () => { - delete sessionStore.main; - return initialEntry; + if (mutation === "replace") { + sessionStore.main = replacementEntry; + } else { + delete sessionStore.main; + } + if (outcome === "failure") { + throw new Error("preflight failed"); + } + return outcome === "rotated" + ? ({ + sessionId: "compacted-session", + lifecycleRevision: "compacted", + updatedAt: 2, + } satisfies SessionEntry) + : initialEntry; }); await expect( @@ -750,8 +719,11 @@ describe("admitFollowupTurn", () => { queued: createRun(), defaults: createDefaults({ sessionStore, sessionEntry: initialEntry }), }), - ).rejects.toThrow("Follow-up session generation changed"); + ).rejects.toThrow(error); expect(operation.complete).toHaveBeenCalledOnce(); + if (checksFailureText) { + expect(state.buildPreflightFailureText).not.toHaveBeenCalled(); + } }); it("refreshes send policy and goal context after preflight rotates the generation", async () => { @@ -1029,59 +1001,6 @@ describe("admitFollowupTurn", () => { }); }); - it("restores the item when a failing preflight observes a replacement generation", async () => { - const operation = createOperation(); - const initialEntry: SessionEntry = { - sessionId: "queued-session", - lifecycleRevision: "admitted", - updatedAt: 1, - }; - const replacementEntry: SessionEntry = { - sessionId: "replacement-session", - lifecycleRevision: "replacement", - updatedAt: 2, - }; - const sessionStore = { main: initialEntry }; - state.admitReply.mockResolvedValue({ status: "owned", operation, sessionEntry: initialEntry }); - state.preflight.mockImplementation(async () => { - sessionStore.main = replacementEntry; - throw new Error("preflight failed"); - }); - - await expect( - admitFollowupTurn({ - queued: createRun(), - defaults: createDefaults({ sessionEntry: initialEntry, sessionStore }), - }), - ).rejects.toThrow("Follow-up session generation changed after reply admission"); - expect(operation.complete).toHaveBeenCalledOnce(); - expect(state.buildPreflightFailureText).not.toHaveBeenCalled(); - }); - - it("restores the item when a failing preflight observes in-memory deletion", async () => { - const operation = createOperation(); - const initialEntry: SessionEntry = { - sessionId: "queued-session", - lifecycleRevision: "admitted", - updatedAt: 1, - }; - const sessionStore: Record = { main: initialEntry }; - state.admitReply.mockResolvedValue({ status: "owned", operation, sessionEntry: initialEntry }); - state.preflight.mockImplementation(async () => { - delete sessionStore.main; - throw new Error("preflight failed"); - }); - - await expect( - admitFollowupTurn({ - queued: createRun(), - defaults: createDefaults({ sessionEntry: initialEntry, sessionStore }), - }), - ).rejects.toThrow("Follow-up session generation changed"); - expect(operation.complete).toHaveBeenCalledOnce(); - expect(state.buildPreflightFailureText).not.toHaveBeenCalled(); - }); - it("uses admitted verbosity when formatting a preflight failure", async () => { const operation = createOperation(); const admittedEntry: SessionEntry = {