test: table-drive status reactions and session key cases

src/channels/status-reactions.test.ts

@@ -28,55 +28,61 @@ const createMockAdapter = () => {
   };
 };
 
+const createEnabledController = (
+  overrides: Partial<Parameters<typeof createStatusReactionController>[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<typeof createStatusReactionController>) => controller.setDone(),
+      expected: DEFAULT_EMOJIS.done,
+    },
+    {
+      name: "setError",
+      run: (controller: ReturnType<typeof createStatusReactionController>) => 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<typeof createStatusReactionController>) =>
+        controller.setDone(),
+      followup: (controller: ReturnType<typeof createStatusReactionController>) => {
+        void controller.setThinking();
+      },
+    },
+    {
+      name: "ignore setTool after setError (terminal state)",
+      terminal: (controller: ReturnType<typeof createStatusReactionController>) =>
+        controller.setError(),
+      followup: (controller: ReturnType<typeof createStatusReactionController>) => {
+        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);
+    }
   });
 });

src/config/sessions.test.ts

@@ -37,39 +37,44 @@ describe("sessions", () => {
   const withStateDir = <T>(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";