From 5164822cd5a1c261da83d3519f2a8b59b114a590 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 21 Feb 2026 22:59:39 +0000 Subject: [PATCH] test: table-drive status reactions and session key cases --- src/channels/status-reactions.test.ts | 395 +++++++++++--------------- src/config/sessions.test.ts | 155 ++++++---- 2 files changed, 259 insertions(+), 291 deletions(-) diff --git a/src/channels/status-reactions.test.ts b/src/channels/status-reactions.test.ts index fcccffbb266..96e59da992a 100644 --- a/src/channels/status-reactions.test.ts +++ b/src/channels/status-reactions.test.ts @@ -28,55 +28,61 @@ const createMockAdapter = () => { }; }; +const createEnabledController = ( + overrides: Partial[0]> = {}, +) => { + const { adapter, calls } = createMockAdapter(); + const controller = createStatusReactionController({ + enabled: true, + adapter, + initialEmoji: "👀", + ...overrides, + }); + return { adapter, calls, controller }; +}; + // ───────────────────────────────────────────────────────────────────────────── // Tests // ───────────────────────────────────────────────────────────────────────────── describe("resolveToolEmoji", () => { - it("should return coding emoji for exec tool", () => { - const result = resolveToolEmoji("exec", DEFAULT_EMOJIS); - expect(result).toBe(DEFAULT_EMOJIS.coding); - }); + const cases: Array<{ + name: string; + tool: string | undefined; + expected: string; + }> = [ + { name: "returns coding emoji for exec tool", tool: "exec", expected: DEFAULT_EMOJIS.coding }, + { + name: "returns coding emoji for process tool", + tool: "process", + expected: DEFAULT_EMOJIS.coding, + }, + { + name: "returns web emoji for web_search tool", + tool: "web_search", + expected: DEFAULT_EMOJIS.web, + }, + { name: "returns web emoji for browser tool", tool: "browser", expected: DEFAULT_EMOJIS.web }, + { + name: "returns tool emoji for unknown tool", + tool: "unknown_tool", + expected: DEFAULT_EMOJIS.tool, + }, + { name: "returns tool emoji for empty string", tool: "", expected: DEFAULT_EMOJIS.tool }, + { name: "returns tool emoji for undefined", tool: undefined, expected: DEFAULT_EMOJIS.tool }, + { name: "is case-insensitive", tool: "EXEC", expected: DEFAULT_EMOJIS.coding }, + { + name: "matches tokens within tool names", + tool: "my_exec_wrapper", + expected: DEFAULT_EMOJIS.coding, + }, + ]; - it("should return coding emoji for process tool", () => { - const result = resolveToolEmoji("process", DEFAULT_EMOJIS); - expect(result).toBe(DEFAULT_EMOJIS.coding); - }); - - it("should return web emoji for web_search tool", () => { - const result = resolveToolEmoji("web_search", DEFAULT_EMOJIS); - expect(result).toBe(DEFAULT_EMOJIS.web); - }); - - it("should return web emoji for browser tool", () => { - const result = resolveToolEmoji("browser", DEFAULT_EMOJIS); - expect(result).toBe(DEFAULT_EMOJIS.web); - }); - - it("should return tool emoji for unknown tool", () => { - const result = resolveToolEmoji("unknown_tool", DEFAULT_EMOJIS); - expect(result).toBe(DEFAULT_EMOJIS.tool); - }); - - it("should return tool emoji for empty string", () => { - const result = resolveToolEmoji("", DEFAULT_EMOJIS); - expect(result).toBe(DEFAULT_EMOJIS.tool); - }); - - it("should return tool emoji for undefined", () => { - const result = resolveToolEmoji(undefined, DEFAULT_EMOJIS); - expect(result).toBe(DEFAULT_EMOJIS.tool); - }); - - it("should be case-insensitive", () => { - const result = resolveToolEmoji("EXEC", DEFAULT_EMOJIS); - expect(result).toBe(DEFAULT_EMOJIS.coding); - }); - - it("should match tokens within tool names", () => { - const result = resolveToolEmoji("my_exec_wrapper", DEFAULT_EMOJIS); - expect(result).toBe(DEFAULT_EMOJIS.coding); - }); + for (const testCase of cases) { + it(`should ${testCase.name}`, () => { + expect(resolveToolEmoji(testCase.tool, DEFAULT_EMOJIS)).toBe(testCase.expected); + }); + } }); describe("createStatusReactionController", () => { @@ -105,12 +111,7 @@ describe("createStatusReactionController", () => { }); it("should call setReaction with initialEmoji for setQueued immediately", async () => { - const { adapter, calls } = createMockAdapter(); - const controller = createStatusReactionController({ - enabled: true, - adapter, - initialEmoji: "👀", - }); + const { calls, controller } = createEnabledController(); void controller.setQueued(); await vi.runAllTimersAsync(); @@ -119,12 +120,7 @@ describe("createStatusReactionController", () => { }); it("should debounce setThinking and eventually call adapter", async () => { - const { adapter, calls } = createMockAdapter(); - const controller = createStatusReactionController({ - enabled: true, - adapter, - initialEmoji: "👀", - }); + const { calls, controller } = createEnabledController(); void controller.setThinking(); @@ -138,12 +134,7 @@ describe("createStatusReactionController", () => { }); it("should classify tool name and debounce", async () => { - const { adapter, calls } = createMockAdapter(); - const controller = createStatusReactionController({ - enabled: true, - adapter, - initialEmoji: "👀", - }); + const { calls, controller } = createEnabledController(); void controller.setTool("exec"); await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.debounceMs); @@ -151,75 +142,64 @@ describe("createStatusReactionController", () => { expect(calls).toContainEqual({ method: "set", emoji: DEFAULT_EMOJIS.coding }); }); - it("should execute setDone immediately without debounce", async () => { - const { adapter, calls } = createMockAdapter(); - const controller = createStatusReactionController({ - enabled: true, - adapter, - initialEmoji: "👀", + const immediateTerminalCases = [ + { + name: "setDone", + run: (controller: ReturnType) => controller.setDone(), + expected: DEFAULT_EMOJIS.done, + }, + { + name: "setError", + run: (controller: ReturnType) => controller.setError(), + expected: DEFAULT_EMOJIS.error, + }, + ] as const; + + for (const testCase of immediateTerminalCases) { + it(`should execute ${testCase.name} immediately without debounce`, async () => { + const { calls, controller } = createEnabledController(); + + await testCase.run(controller); + await vi.runAllTimersAsync(); + + expect(calls).toContainEqual({ method: "set", emoji: testCase.expected }); }); + } - await controller.setDone(); - await vi.runAllTimersAsync(); + const terminalIgnoreCases = [ + { + name: "ignore setThinking after setDone (terminal state)", + terminal: (controller: ReturnType) => + controller.setDone(), + followup: (controller: ReturnType) => { + void controller.setThinking(); + }, + }, + { + name: "ignore setTool after setError (terminal state)", + terminal: (controller: ReturnType) => + controller.setError(), + followup: (controller: ReturnType) => { + void controller.setTool("exec"); + }, + }, + ] as const; - expect(calls).toContainEqual({ method: "set", emoji: DEFAULT_EMOJIS.done }); - }); + for (const testCase of terminalIgnoreCases) { + it(`should ${testCase.name}`, async () => { + const { calls, controller } = createEnabledController(); - it("should execute setError immediately without debounce", async () => { - const { adapter, calls } = createMockAdapter(); - const controller = createStatusReactionController({ - enabled: true, - adapter, - initialEmoji: "👀", + await testCase.terminal(controller); + const callsAfterTerminal = calls.length; + testCase.followup(controller); + await vi.advanceTimersByTimeAsync(1000); + + expect(calls.length).toBe(callsAfterTerminal); }); - - await controller.setError(); - await vi.runAllTimersAsync(); - - expect(calls).toContainEqual({ method: "set", emoji: DEFAULT_EMOJIS.error }); - }); - - it("should ignore setThinking after setDone (terminal state)", async () => { - const { adapter, calls } = createMockAdapter(); - const controller = createStatusReactionController({ - enabled: true, - adapter, - initialEmoji: "👀", - }); - - await controller.setDone(); - const callsAfterDone = calls.length; - - void controller.setThinking(); - await vi.advanceTimersByTimeAsync(1000); - - expect(calls.length).toBe(callsAfterDone); - }); - - it("should ignore setTool after setError (terminal state)", async () => { - const { adapter, calls } = createMockAdapter(); - const controller = createStatusReactionController({ - enabled: true, - adapter, - initialEmoji: "👀", - }); - - await controller.setError(); - const callsAfterError = calls.length; - - void controller.setTool("exec"); - await vi.advanceTimersByTimeAsync(1000); - - expect(calls.length).toBe(callsAfterError); - }); + } it("should only fire last state when rapidly changing (debounce)", async () => { - const { adapter, calls } = createMockAdapter(); - const controller = createStatusReactionController({ - enabled: true, - adapter, - initialEmoji: "👀", - }); + const { calls, controller } = createEnabledController(); void controller.setThinking(); await vi.advanceTimersByTimeAsync(100); @@ -236,12 +216,7 @@ describe("createStatusReactionController", () => { }); it("should deduplicate same emoji calls", async () => { - const { adapter, calls } = createMockAdapter(); - const controller = createStatusReactionController({ - enabled: true, - adapter, - initialEmoji: "👀", - }); + const { calls, controller } = createEnabledController(); void controller.setThinking(); await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.debounceMs); @@ -256,12 +231,7 @@ describe("createStatusReactionController", () => { }); it("should call removeReaction when adapter supports it and emoji changes", async () => { - const { adapter, calls } = createMockAdapter(); - const controller = createStatusReactionController({ - enabled: true, - adapter, - initialEmoji: "👀", - }); + const { calls, controller } = createEnabledController(); void controller.setQueued(); await vi.runAllTimersAsync(); @@ -302,12 +272,7 @@ describe("createStatusReactionController", () => { }); it("should clear all known emojis when adapter supports removeReaction", async () => { - const { adapter, calls } = createMockAdapter(); - const controller = createStatusReactionController({ - enabled: true, - adapter, - initialEmoji: "👀", - }); + const { calls, controller } = createEnabledController(); void controller.setQueued(); await vi.runAllTimersAsync(); @@ -341,12 +306,7 @@ describe("createStatusReactionController", () => { }); it("should restore initial emoji", async () => { - const { adapter, calls } = createMockAdapter(); - const controller = createStatusReactionController({ - enabled: true, - adapter, - initialEmoji: "👀", - }); + const { calls, controller } = createEnabledController(); void controller.setThinking(); await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.debounceMs); @@ -357,17 +317,11 @@ describe("createStatusReactionController", () => { }); it("should use custom emojis when provided", async () => { - const { adapter, calls } = createMockAdapter(); - const customEmojis = { - thinking: "🤔", - done: "🎉", - }; - - const controller = createStatusReactionController({ - enabled: true, - adapter, - initialEmoji: "👀", - emojis: customEmojis, + const { calls, controller } = createEnabledController({ + emojis: { + thinking: "🤔", + done: "🎉", + }, }); void controller.setThinking(); @@ -381,16 +335,10 @@ describe("createStatusReactionController", () => { }); it("should use custom timing when provided", async () => { - const { adapter, calls } = createMockAdapter(); - const customTiming = { - debounceMs: 100, - }; - - const controller = createStatusReactionController({ - enabled: true, - adapter, - initialEmoji: "👀", - timing: customTiming, + const { calls, controller } = createEnabledController({ + timing: { + debounceMs: 100, + }, }); void controller.setThinking(); @@ -404,47 +352,33 @@ describe("createStatusReactionController", () => { expect(calls).toContainEqual({ method: "set", emoji: DEFAULT_EMOJIS.thinking }); }); - it("should trigger soft stall timer after stallSoftMs", async () => { - const { adapter, calls } = createMockAdapter(); - const controller = createStatusReactionController({ - enabled: true, - adapter, - initialEmoji: "👀", + const stallCases = [ + { + name: "soft stall timer after stallSoftMs", + delayMs: DEFAULT_TIMING.stallSoftMs, + expected: DEFAULT_EMOJIS.stallSoft, + }, + { + name: "hard stall timer after stallHardMs", + delayMs: DEFAULT_TIMING.stallHardMs, + expected: DEFAULT_EMOJIS.stallHard, + }, + ] as const; + + for (const testCase of stallCases) { + it(`should trigger ${testCase.name}`, async () => { + const { calls, controller } = createEnabledController(); + + void controller.setThinking(); + await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.debounceMs); + await vi.advanceTimersByTimeAsync(testCase.delayMs); + + expect(calls).toContainEqual({ method: "set", emoji: testCase.expected }); }); - - void controller.setThinking(); - await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.debounceMs); - - // Advance to soft stall threshold - await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.stallSoftMs); - - expect(calls).toContainEqual({ method: "set", emoji: DEFAULT_EMOJIS.stallSoft }); - }); - - it("should trigger hard stall timer after stallHardMs", async () => { - const { adapter, calls } = createMockAdapter(); - const controller = createStatusReactionController({ - enabled: true, - adapter, - initialEmoji: "👀", - }); - - void controller.setThinking(); - await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.debounceMs); - - // Advance to hard stall threshold - await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.stallHardMs); - - expect(calls).toContainEqual({ method: "set", emoji: DEFAULT_EMOJIS.stallHard }); - }); + } it("should reset stall timers on phase change", async () => { - const { adapter, calls } = createMockAdapter(); - const controller = createStatusReactionController({ - enabled: true, - adapter, - initialEmoji: "👀", - }); + const { calls, controller } = createEnabledController(); void controller.setThinking(); await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.debounceMs); @@ -464,12 +398,7 @@ describe("createStatusReactionController", () => { }); it("should reset stall timers on repeated same-phase updates", async () => { - const { adapter, calls } = createMockAdapter(); - const controller = createStatusReactionController({ - enabled: true, - adapter, - initialEmoji: "👀", - }); + const { calls, controller } = createEnabledController(); void controller.setThinking(); await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.debounceMs); @@ -511,33 +440,37 @@ describe("createStatusReactionController", () => { describe("constants", () => { it("should export CODING_TOOL_TOKENS", () => { - expect(CODING_TOOL_TOKENS).toContain("exec"); - expect(CODING_TOOL_TOKENS).toContain("read"); - expect(CODING_TOOL_TOKENS).toContain("write"); + for (const token of ["exec", "read", "write"]) { + expect(CODING_TOOL_TOKENS).toContain(token); + } }); it("should export WEB_TOOL_TOKENS", () => { - expect(WEB_TOOL_TOKENS).toContain("web_search"); - expect(WEB_TOOL_TOKENS).toContain("browser"); + for (const token of ["web_search", "browser"]) { + expect(WEB_TOOL_TOKENS).toContain(token); + } }); it("should export DEFAULT_EMOJIS with all required keys", () => { - expect(DEFAULT_EMOJIS).toHaveProperty("queued"); - expect(DEFAULT_EMOJIS).toHaveProperty("thinking"); - expect(DEFAULT_EMOJIS).toHaveProperty("tool"); - expect(DEFAULT_EMOJIS).toHaveProperty("coding"); - expect(DEFAULT_EMOJIS).toHaveProperty("web"); - expect(DEFAULT_EMOJIS).toHaveProperty("done"); - expect(DEFAULT_EMOJIS).toHaveProperty("error"); - expect(DEFAULT_EMOJIS).toHaveProperty("stallSoft"); - expect(DEFAULT_EMOJIS).toHaveProperty("stallHard"); + const emojiKeys = [ + "queued", + "thinking", + "tool", + "coding", + "web", + "done", + "error", + "stallSoft", + "stallHard", + ] as const; + for (const key of emojiKeys) { + expect(DEFAULT_EMOJIS).toHaveProperty(key); + } }); it("should export DEFAULT_TIMING with all required keys", () => { - expect(DEFAULT_TIMING).toHaveProperty("debounceMs"); - expect(DEFAULT_TIMING).toHaveProperty("stallSoftMs"); - expect(DEFAULT_TIMING).toHaveProperty("stallHardMs"); - expect(DEFAULT_TIMING).toHaveProperty("doneHoldMs"); - expect(DEFAULT_TIMING).toHaveProperty("errorHoldMs"); + for (const key of ["debounceMs", "stallSoftMs", "stallHardMs", "doneHoldMs", "errorHoldMs"]) { + expect(DEFAULT_TIMING).toHaveProperty(key); + } }); }); diff --git a/src/config/sessions.test.ts b/src/config/sessions.test.ts index 13c2f647447..221654659d9 100644 --- a/src/config/sessions.test.ts +++ b/src/config/sessions.test.ts @@ -37,39 +37,44 @@ describe("sessions", () => { const withStateDir = (stateDir: string, fn: () => T): T => withEnv({ OPENCLAW_STATE_DIR: stateDir }, fn); - it("returns normalized per-sender key", () => { - expect(deriveSessionKey("per-sender", { From: "whatsapp:+1555" })).toBe("+1555"); - }); + const deriveSessionKeyCases = [ + { + name: "returns normalized per-sender key", + scope: "per-sender" as const, + ctx: { From: "whatsapp:+1555" }, + expected: "+1555", + }, + { + name: "falls back to unknown when sender missing", + scope: "per-sender" as const, + ctx: {}, + expected: "unknown", + }, + { + name: "global scope returns global", + scope: "global" as const, + ctx: { From: "+1" }, + expected: "global", + }, + { + name: "keeps group chats distinct", + scope: "per-sender" as const, + ctx: { From: "12345-678@g.us" }, + expected: "whatsapp:group:12345-678@g.us", + }, + { + name: "prefixes group keys with provider when available", + scope: "per-sender" as const, + ctx: { From: "12345-678@g.us", ChatType: "group", Provider: "whatsapp" }, + expected: "whatsapp:group:12345-678@g.us", + }, + ] as const; - it("falls back to unknown when sender missing", () => { - expect(deriveSessionKey("per-sender", {})).toBe("unknown"); - }); - - it("global scope returns global", () => { - expect(deriveSessionKey("global", { From: "+1" })).toBe("global"); - }); - - it("keeps group chats distinct", () => { - expect(deriveSessionKey("per-sender", { From: "12345-678@g.us" })).toBe( - "whatsapp:group:12345-678@g.us", - ); - }); - - it("prefixes group keys with provider when available", () => { - expect( - deriveSessionKey("per-sender", { - From: "12345-678@g.us", - ChatType: "group", - Provider: "whatsapp", - }), - ).toBe("whatsapp:group:12345-678@g.us"); - }); - - it("keeps explicit provider when provided in group key", () => { - expect( - resolveSessionKey("per-sender", { From: "discord:group:12345", ChatType: "group" }, "main"), - ).toBe("agent:main:discord:group:12345"); - }); + for (const testCase of deriveSessionKeyCases) { + it(testCase.name, () => { + expect(deriveSessionKey(testCase.scope, testCase.ctx)).toBe(testCase.expected); + }); + } it("builds discord display name with guild+channel slugs", () => { expect( @@ -83,35 +88,65 @@ describe("sessions", () => { ).toBe("discord:friends-of-openclaw#general"); }); - it("collapses direct chats to main by default", () => { - expect(resolveSessionKey("per-sender", { From: "+1555" })).toBe("agent:main:main"); - }); + const resolveSessionKeyCases = [ + { + name: "keeps explicit provider when provided in group key", + scope: "per-sender" as const, + ctx: { From: "discord:group:12345", ChatType: "group" }, + mainKey: "main", + expected: "agent:main:discord:group:12345", + }, + { + name: "collapses direct chats to main by default", + scope: "per-sender" as const, + ctx: { From: "+1555" }, + mainKey: undefined, + expected: "agent:main:main", + }, + { + name: "collapses direct chats to main even when sender missing", + scope: "per-sender" as const, + ctx: {}, + mainKey: undefined, + expected: "agent:main:main", + }, + { + name: "maps direct chats to main key when provided", + scope: "per-sender" as const, + ctx: { From: "whatsapp:+1555" }, + mainKey: "main", + expected: "agent:main:main", + }, + { + name: "uses custom main key when provided", + scope: "per-sender" as const, + ctx: { From: "+1555" }, + mainKey: "primary", + expected: "agent:main:primary", + }, + { + name: "keeps global scope untouched", + scope: "global" as const, + ctx: { From: "+1555" }, + mainKey: undefined, + expected: "global", + }, + { + name: "leaves groups untouched even with main key", + scope: "per-sender" as const, + ctx: { From: "12345-678@g.us" }, + mainKey: "main", + expected: "agent:main:whatsapp:group:12345-678@g.us", + }, + ] as const; - it("collapses direct chats to main even when sender missing", () => { - expect(resolveSessionKey("per-sender", {})).toBe("agent:main:main"); - }); - - it("maps direct chats to main key when provided", () => { - expect(resolveSessionKey("per-sender", { From: "whatsapp:+1555" }, "main")).toBe( - "agent:main:main", - ); - }); - - it("uses custom main key when provided", () => { - expect(resolveSessionKey("per-sender", { From: "+1555" }, "primary")).toBe( - "agent:main:primary", - ); - }); - - it("keeps global scope untouched", () => { - expect(resolveSessionKey("global", { From: "+1555" })).toBe("global"); - }); - - it("leaves groups untouched even with main key", () => { - expect(resolveSessionKey("per-sender", { From: "12345-678@g.us" }, "main")).toBe( - "agent:main:whatsapp:group:12345-678@g.us", - ); - }); + for (const testCase of resolveSessionKeyCases) { + it(testCase.name, () => { + expect(resolveSessionKey(testCase.scope, testCase.ctx, testCase.mainKey)).toBe( + testCase.expected, + ); + }); + } it("updateLastRoute persists channel and target", async () => { const mainSessionKey = "agent:main:main";