Fix test corrupting production sessions.json

The test 'falls back to most recent session when no to is provided' was using resolveStorePath() which returns the real ~/.warelay/sessions.json. This overwrote production session data with test values, causing session fragmentation issues. Changed to use a temp directory like other tests.
2025-12-02 05:54:31 +00:00
parent c5ab442f46
commit d107b79c63
5 changed files with 412 additions and 38 deletions
--- a/src/auto-reply/command-reply.test.ts
+++ b/src/auto-reply/command-reply.test.ts
@@ -292,4 +292,79 @@ describe("runCommandReply", () => {
    expect(meta.queuedMs).toBe(25);
    expect(meta.queuedAhead).toBe(2);
  });
+
+  it("handles empty result string without dumping raw JSON", async () => {
+    // Bug fix: Claude CLI returning {"result": ""} should not send raw JSON to WhatsApp
+    // The fix changed from truthy check to explicit typeof check
+    const runner = makeRunner({
+      stdout: '{"result":"","duration_ms":50,"total_cost_usd":0.001}',
+    });
+    const { payload } = await runCommandReply({
+      reply: {
+        mode: "command",
+        command: ["claude", "{{Body}}"],
+        claudeOutputFormat: "json",
+      },
+      templatingCtx: noopTemplateCtx,
+      sendSystemOnce: false,
+      isNewSession: true,
+      isFirstTurnInSession: true,
+      systemSent: false,
+      timeoutMs: 1000,
+      timeoutSeconds: 1,
+      commandRunner: runner,
+      enqueue: enqueueImmediate,
+    });
+    // Should NOT contain raw JSON - empty result should produce fallback message
+    expect(payload?.text).not.toContain('{"result"');
+    expect(payload?.text).toContain("command produced no output");
+  });
+
+  it("handles empty text string in Claude JSON", async () => {
+    const runner = makeRunner({
+      stdout: '{"text":"","duration_ms":50}',
+    });
+    const { payload } = await runCommandReply({
+      reply: {
+        mode: "command",
+        command: ["claude", "{{Body}}"],
+        claudeOutputFormat: "json",
+      },
+      templatingCtx: noopTemplateCtx,
+      sendSystemOnce: false,
+      isNewSession: true,
+      isFirstTurnInSession: true,
+      systemSent: false,
+      timeoutMs: 1000,
+      timeoutSeconds: 1,
+      commandRunner: runner,
+      enqueue: enqueueImmediate,
+    });
+    // Empty text should produce fallback message, not raw JSON
+    expect(payload?.text).not.toContain('{"text"');
+    expect(payload?.text).toContain("command produced no output");
+  });
+
+  it("returns actual text when result is non-empty", async () => {
+    const runner = makeRunner({
+      stdout: '{"result":"hello world","duration_ms":50}',
+    });
+    const { payload } = await runCommandReply({
+      reply: {
+        mode: "command",
+        command: ["claude", "{{Body}}"],
+        claudeOutputFormat: "json",
+      },
+      templatingCtx: noopTemplateCtx,
+      sendSystemOnce: false,
+      isNewSession: true,
+      isFirstTurnInSession: true,
+      systemSent: false,
+      timeoutMs: 1000,
+      timeoutSeconds: 1,
+      commandRunner: runner,
+      enqueue: enqueueImmediate,
+    });
+    expect(payload?.text).toBe("hello world");
+  });
 });
--- a/src/auto-reply/command-reply.ts
+++ b/src/auto-reply/command-reply.ts
@@ -259,8 +259,11 @@ export async function runCommandReply(
      console.error(
        `Command auto-reply exited with code ${code ?? "unknown"} (signal: ${signal ?? "none"})`,
      );
+      // Include any partial output or stderr in error message
+      const partialOut = trimmed ? `\n\nOutput: ${trimmed.slice(0, 500)}${trimmed.length > 500 ? "..." : ""}` : "";
+      const errorText = `⚠️ Command exited with code ${code ?? "unknown"}${signal ? ` (${signal})` : ""}${partialOut}`;
      return {
-        payload: undefined,
+        payload: { text: errorText },
        meta: {
          durationMs: Date.now() - started,
          queuedMs,
@@ -278,8 +281,9 @@ export async function runCommandReply(
      console.error(
        `Command auto-reply process killed before completion (exit code ${code ?? "unknown"})`,
      );
+      const errorText = `⚠️ Command was killed before completion (exit code ${code ?? "unknown"})`;
      return {
-        payload: undefined,
+        payload: { text: errorText },
        meta: {
          durationMs: Date.now() - started,
          queuedMs,
@@ -379,8 +383,11 @@ export async function runCommandReply(
      };
    }
    logError(`Command auto-reply failed after ${elapsed}ms: ${String(err)}`);
+    // Send error message to user so they know the command failed
+    const errMsg = err instanceof Error ? err.message : String(err);
+    const errorText = `⚠️ Command failed: ${errMsg}`;
    return {
-      payload: undefined,
+      payload: { text: errorText },
      meta: {
        durationMs: elapsed,
        queuedMs,
--- a/src/web/auto-reply.test.ts
+++ b/src/web/auto-reply.test.ts
@@ -1,3 +1,10 @@
+// Import test-helpers FIRST to set up mocks before other imports
+import {
+  resetBaileysMocks,
+  resetLoadConfigMock,
+  setLoadConfigMock,
+} from "./test-helpers.js";
+
 import crypto from "node:crypto";
 import fs from "node:fs/promises";
 import os from "node:os";
@@ -18,11 +25,6 @@ import {
  stripHeartbeatToken,
 } from "./auto-reply.js";
 import type { sendMessageWeb } from "./outbound.js";
-import {
-  resetBaileysMocks,
-  resetLoadConfigMock,
-  setLoadConfigMock,
-} from "./test-helpers.js";

 describe("heartbeat helpers", () => {
  it("strips heartbeat token and skips when only token", () => {
@@ -186,6 +188,11 @@ describe("runWebHeartbeatOnce", () => {
  });

  it("falls back to most recent session when no to is provided", async () => {
+    // Use temp directory to avoid corrupting production sessions.json
+    const tmpDir = await fs.mkdtemp(
+      path.join(os.tmpdir(), "warelay-fallback-session-"),
+    );
+    const storePath = path.join(tmpDir, "sessions.json");
    const sender: typeof sendMessageWeb = vi
      .fn()
      .mockResolvedValue({ messageId: "m1", toJid: "jid" });
@@ -196,15 +203,11 @@ describe("runWebHeartbeatOnce", () => {
      "+1222": { sessionId: "s1", updatedAt: now - 1000 },
      "+1333": { sessionId: "s2", updatedAt: now },
    };
-    const storePath = resolveStorePath();
-    await fs.mkdir(resolveStorePath().replace("sessions.json", ""), {
-      recursive: true,
-    });
    await fs.writeFile(storePath, JSON.stringify(store));
    setLoadConfigMock({
      inbound: {
        allowFrom: ["+1999"],
-        reply: { mode: "command", session: {} },
+        reply: { mode: "command", session: { store: storePath } },
      },
    });
    await runWebHeartbeatOnce({
@@ -947,6 +950,16 @@ describe("web auto-reply", () => {
  });

  it("prefixes body with same-phone marker when from === to", async () => {
+    // Enable messagePrefix for same-phone mode testing
+    setLoadConfigMock(() => ({
+      inbound: {
+        allowFrom: ["*"],
+        messagePrefix: "[same-phone]",
+        responsePrefix: undefined,
+        timestampPrefix: false,
+      },
+    }));
+
    let capturedOnMessage:
      | ((msg: import("./inbound.js").WebInboundMessage) => Promise<void>)
      | undefined;
@@ -974,12 +987,11 @@ describe("web auto-reply", () => {
      sendMedia: vi.fn(),
    });

-    // The resolver should receive a prefixed body (the exact marker depends on config)
-    // Key test: body should start with some marker and end with original message
+    // The resolver should receive a prefixed body with the configured marker
    const callArg = resolver.mock.calls[0]?.[0] as { Body?: string };
    expect(callArg?.Body).toBeDefined();
-    expect(callArg?.Body).toMatch(/^\[.*\] hello$/);
-    expect(callArg?.Body).not.toBe("hello"); // Should be prefixed
+    expect(callArg?.Body).toBe("[same-phone] hello");
+    resetLoadConfigMock();
  });

  it("does not prefix body when from !== to", async () => {
@@ -1014,4 +1026,135 @@ describe("web auto-reply", () => {
    const callArg = resolver.mock.calls[0]?.[0] as { Body?: string };
    expect(callArg?.Body).toBe("hello");
  });
+
+  it("applies responsePrefix to regular replies", async () => {
+    setLoadConfigMock(() => ({
+      inbound: {
+        allowFrom: ["*"],
+        messagePrefix: undefined,
+        responsePrefix: "🦞",
+        timestampPrefix: false,
+      },
+    }));
+
+    let capturedOnMessage:
+      | ((msg: import("./inbound.js").WebInboundMessage) => Promise<void>)
+      | undefined;
+    const reply = vi.fn();
+    const listenerFactory = async (opts: {
+      onMessage: (
+        msg: import("./inbound.js").WebInboundMessage,
+      ) => Promise<void>;
+    }) => {
+      capturedOnMessage = opts.onMessage;
+      return { close: vi.fn() };
+    };
+
+    const resolver = vi.fn().mockResolvedValue({ text: "hello there" });
+
+    await monitorWebProvider(false, listenerFactory, false, resolver);
+    expect(capturedOnMessage).toBeDefined();
+
+    await capturedOnMessage?.({
+      body: "hi",
+      from: "+1555",
+      to: "+2666",
+      id: "msg1",
+      sendComposing: vi.fn(),
+      reply,
+      sendMedia: vi.fn(),
+    });
+
+    // Reply should have responsePrefix prepended
+    expect(reply).toHaveBeenCalledWith("🦞 hello there");
+    resetLoadConfigMock();
+  });
+
+  it("skips responsePrefix for HEARTBEAT_OK responses", async () => {
+    setLoadConfigMock(() => ({
+      inbound: {
+        allowFrom: ["*"],
+        messagePrefix: undefined,
+        responsePrefix: "🦞",
+        timestampPrefix: false,
+      },
+    }));
+
+    let capturedOnMessage:
+      | ((msg: import("./inbound.js").WebInboundMessage) => Promise<void>)
+      | undefined;
+    const reply = vi.fn();
+    const listenerFactory = async (opts: {
+      onMessage: (
+        msg: import("./inbound.js").WebInboundMessage,
+      ) => Promise<void>;
+    }) => {
+      capturedOnMessage = opts.onMessage;
+      return { close: vi.fn() };
+    };
+
+    // Resolver returns exact HEARTBEAT_OK
+    const resolver = vi.fn().mockResolvedValue({ text: HEARTBEAT_TOKEN });
+
+    await monitorWebProvider(false, listenerFactory, false, resolver);
+    expect(capturedOnMessage).toBeDefined();
+
+    await capturedOnMessage?.({
+      body: "test",
+      from: "+1555",
+      to: "+2666",
+      id: "msg1",
+      sendComposing: vi.fn(),
+      reply,
+      sendMedia: vi.fn(),
+    });
+
+    // HEARTBEAT_OK should NOT have prefix - warelay needs exact match
+    expect(reply).toHaveBeenCalledWith(HEARTBEAT_TOKEN);
+    resetLoadConfigMock();
+  });
+
+  it("does not double-prefix if responsePrefix already present", async () => {
+    setLoadConfigMock(() => ({
+      inbound: {
+        allowFrom: ["*"],
+        messagePrefix: undefined,
+        responsePrefix: "🦞",
+        timestampPrefix: false,
+      },
+    }));
+
+    let capturedOnMessage:
+      | ((msg: import("./inbound.js").WebInboundMessage) => Promise<void>)
+      | undefined;
+    const reply = vi.fn();
+    const listenerFactory = async (opts: {
+      onMessage: (
+        msg: import("./inbound.js").WebInboundMessage,
+      ) => Promise<void>;
+    }) => {
+      capturedOnMessage = opts.onMessage;
+      return { close: vi.fn() };
+    };
+
+    // Resolver returns text that already has prefix
+    const resolver = vi.fn().mockResolvedValue({ text: "🦞 already prefixed" });
+
+    await monitorWebProvider(false, listenerFactory, false, resolver);
+    expect(capturedOnMessage).toBeDefined();
+
+    await capturedOnMessage?.({
+      body: "test",
+      from: "+1555",
+      to: "+2666",
+      id: "msg1",
+      sendComposing: vi.fn(),
+      reply,
+      sendMedia: vi.fn(),
+    });
+
+    // Should not double-prefix
+    expect(reply).toHaveBeenCalledWith("🦞 already prefixed");
+    resetLoadConfigMock();
+  });
 });
--- a/src/web/monitor-inbox.test.ts
+++ b/src/web/monitor-inbox.test.ts
@@ -9,15 +9,17 @@ vi.mock("../media/store.js", () => ({
  }),
 }));

+const mockLoadConfig = vi.fn().mockReturnValue({
+  inbound: {
+    allowFrom: ["*"], // Allow all in tests
+    messagePrefix: undefined,
+    responsePrefix: undefined,
+    timestampPrefix: false,
+  },
+});
+
 vi.mock("../config/config.js", () => ({
-  loadConfig: vi.fn().mockReturnValue({
-    inbound: {
-      allowFrom: ["*"], // Allow all in tests
-      messagePrefix: undefined,
-      responsePrefix: undefined,
-      timestampPrefix: false,
-    },
-  }),
+  loadConfig: () => mockLoadConfig(),
 }));

 vi.mock("./session.js", () => {
@@ -227,4 +229,146 @@ describe("web monitor inbox", () => {
    ]);
    await listener.close();
  });
+
+  it("blocks messages from unauthorized senders not in allowFrom", async () => {
+    // Test for auto-recovery fix: early allowFrom filtering prevents Bad MAC errors
+    // from unauthorized senders corrupting sessions
+    mockLoadConfig.mockReturnValue({
+      inbound: {
+        allowFrom: ["+111"], // Only allow +111
+        messagePrefix: undefined,
+        responsePrefix: undefined,
+        timestampPrefix: false,
+      },
+    });
+
+    const onMessage = vi.fn();
+    const listener = await monitorWebInbox({ verbose: false, onMessage });
+    const sock = await createWaSocket();
+
+    // Message from unauthorized sender +999 (not in allowFrom)
+    const upsert = {
+      type: "notify",
+      messages: [
+        {
+          key: { id: "unauth1", fromMe: false, remoteJid: "999@s.whatsapp.net" },
+          message: { conversation: "unauthorized message" },
+          messageTimestamp: 1_700_000_000,
+        },
+      ],
+    };
+
+    sock.ev.emit("messages.upsert", upsert);
+    await new Promise((resolve) => setImmediate(resolve));
+
+    // Should NOT call onMessage for unauthorized senders
+    expect(onMessage).not.toHaveBeenCalled();
+
+    // Reset mock for other tests
+    mockLoadConfig.mockReturnValue({
+      inbound: {
+        allowFrom: ["*"],
+        messagePrefix: undefined,
+        responsePrefix: undefined,
+        timestampPrefix: false,
+      },
+    });
+
+    await listener.close();
+  });
+
+  it("allows messages from senders in allowFrom list", async () => {
+    mockLoadConfig.mockReturnValue({
+      inbound: {
+        allowFrom: ["+111", "+999"], // Allow +999
+        messagePrefix: undefined,
+        responsePrefix: undefined,
+        timestampPrefix: false,
+      },
+    });
+
+    const onMessage = vi.fn();
+    const listener = await monitorWebInbox({ verbose: false, onMessage });
+    const sock = await createWaSocket();
+
+    const upsert = {
+      type: "notify",
+      messages: [
+        {
+          key: { id: "auth1", fromMe: false, remoteJid: "999@s.whatsapp.net" },
+          message: { conversation: "authorized message" },
+          messageTimestamp: 1_700_000_000,
+        },
+      ],
+    };
+
+    sock.ev.emit("messages.upsert", upsert);
+    await new Promise((resolve) => setImmediate(resolve));
+
+    // Should call onMessage for authorized senders
+    expect(onMessage).toHaveBeenCalledWith(
+      expect.objectContaining({ body: "authorized message", from: "+999" }),
+    );
+
+    // Reset mock for other tests
+    mockLoadConfig.mockReturnValue({
+      inbound: {
+        allowFrom: ["*"],
+        messagePrefix: undefined,
+        responsePrefix: undefined,
+        timestampPrefix: false,
+      },
+    });
+
+    await listener.close();
+  });
+
+  it("allows same-phone messages even if not in allowFrom", async () => {
+    // Same-phone mode: when from === selfJid, should always be allowed
+    // This allows users to message themselves even with restrictive allowFrom
+    mockLoadConfig.mockReturnValue({
+      inbound: {
+        allowFrom: ["+111"], // Only allow +111, but self is +123
+        messagePrefix: undefined,
+        responsePrefix: undefined,
+        timestampPrefix: false,
+      },
+    });
+
+    const onMessage = vi.fn();
+    const listener = await monitorWebInbox({ verbose: false, onMessage });
+    const sock = await createWaSocket();
+
+    // Message from self (sock.user.id is "123@s.whatsapp.net" in mock)
+    const upsert = {
+      type: "notify",
+      messages: [
+        {
+          key: { id: "self1", fromMe: false, remoteJid: "123@s.whatsapp.net" },
+          message: { conversation: "self message" },
+          messageTimestamp: 1_700_000_000,
+        },
+      ],
+    };
+
+    sock.ev.emit("messages.upsert", upsert);
+    await new Promise((resolve) => setImmediate(resolve));
+
+    // Should allow self-messages even if not in allowFrom
+    expect(onMessage).toHaveBeenCalledWith(
+      expect.objectContaining({ body: "self message", from: "+123" }),
+    );
+
+    // Reset mock for other tests
+    mockLoadConfig.mockReturnValue({
+      inbound: {
+        allowFrom: ["*"],
+        messagePrefix: undefined,
+        responsePrefix: undefined,
+        timestampPrefix: false,
+      },
+    });
+
+    await listener.close();
+  });
 });
--- a/src/web/test-helpers.ts
+++ b/src/web/test-helpers.ts
@@ -3,32 +3,37 @@ import { vi } from "vitest";
 import type { MockBaileysSocket } from "../../test/mocks/baileys.js";
 import { createMockBaileys } from "../../test/mocks/baileys.js";

-let loadConfigMock: () => unknown = () => ({
+// Use globalThis to store the mock config so it survives vi.mock hoisting
+const CONFIG_KEY = Symbol.for("warelay:testConfigMock");
+const DEFAULT_CONFIG = {
  inbound: {
    allowFrom: ["*"], // Allow all in tests by default
    messagePrefix: undefined, // No message prefix in tests
    responsePrefix: undefined, // No response prefix in tests
    timestampPrefix: false, // No timestamp in tests
  },
-});
+};

-export function setLoadConfigMock(fn: () => unknown) {
-  loadConfigMock = fn;
+// Initialize default if not set
+if (!(globalThis as Record<symbol, unknown>)[CONFIG_KEY]) {
+  (globalThis as Record<symbol, unknown>)[CONFIG_KEY] = () => DEFAULT_CONFIG;
+}
+
+export function setLoadConfigMock(fn: (() => unknown) | unknown) {
+  (globalThis as Record<symbol, unknown>)[CONFIG_KEY] =
+    typeof fn === "function" ? fn : () => fn;
 }

 export function resetLoadConfigMock() {
-  loadConfigMock = () => ({
-    inbound: {
-      allowFrom: ["*"],
-      messagePrefix: undefined,
-      responsePrefix: undefined,
-      timestampPrefix: false,
-    },
-  });
+  (globalThis as Record<symbol, unknown>)[CONFIG_KEY] = () => DEFAULT_CONFIG;
 }

 vi.mock("../config/config.js", () => ({
-  loadConfig: () => loadConfigMock(),
+  loadConfig: () => {
+    const getter = (globalThis as Record<symbol, unknown>)[CONFIG_KEY];
+    if (typeof getter === "function") return getter();
+    return DEFAULT_CONFIG;
+  },
 }));

 vi.mock("../media/store.js", () => ({