From 21b0eac91787bc648f9a68ce1ebe4aab6c21b0c7 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 21 Feb 2026 22:23:00 +0000 Subject: [PATCH] test: consolidate infra approval and heartbeat test matrices --- src/infra/exec-approvals.test.ts | 775 +++++----- ...tbeat-runner.returns-default-unset.test.ts | 1351 ++++++----------- src/infra/outbound/outbound.test.ts | 298 ++-- 3 files changed, 941 insertions(+), 1483 deletions(-) diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts index c12a59014cf..530562a3355 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -62,47 +62,30 @@ function loadShellParserParityFixtureCases(): ShellParserParityFixtureCase[] { } describe("exec approvals allowlist matching", () => { - it("ignores basename-only patterns", () => { - const resolution = { - rawExecutable: "rg", - resolvedPath: "/opt/homebrew/bin/rg", - executableName: "rg", - }; - const entries: ExecAllowlistEntry[] = [{ pattern: "RG" }]; - const match = matchAllowlist(entries, resolution); - expect(match).toBeNull(); - }); + const baseResolution = { + rawExecutable: "rg", + resolvedPath: "/opt/homebrew/bin/rg", + executableName: "rg", + }; - it("matches by resolved path with **", () => { - const resolution = { - rawExecutable: "rg", - resolvedPath: "/opt/homebrew/bin/rg", - executableName: "rg", - }; - const entries: ExecAllowlistEntry[] = [{ pattern: "/opt/**/rg" }]; - const match = matchAllowlist(entries, resolution); - expect(match?.pattern).toBe("/opt/**/rg"); - }); - - it("does not let * cross path separators", () => { - const resolution = { - rawExecutable: "rg", - resolvedPath: "/opt/homebrew/bin/rg", - executableName: "rg", - }; - const entries: ExecAllowlistEntry[] = [{ pattern: "/opt/*/rg" }]; - const match = matchAllowlist(entries, resolution); - expect(match).toBeNull(); + it("handles wildcard/path matching semantics", () => { + const cases: Array<{ entries: ExecAllowlistEntry[]; expectedPattern: string | null }> = [ + { entries: [{ pattern: "RG" }], expectedPattern: null }, + { entries: [{ pattern: "/opt/**/rg" }], expectedPattern: "/opt/**/rg" }, + { entries: [{ pattern: "/opt/*/rg" }], expectedPattern: null }, + ]; + for (const testCase of cases) { + const match = matchAllowlist(testCase.entries, baseResolution); + expect(match?.pattern ?? null).toBe(testCase.expectedPattern); + } }); it("requires a resolved path", () => { - const resolution = { + const match = matchAllowlist([{ pattern: "bin/rg" }], { rawExecutable: "bin/rg", resolvedPath: undefined, executableName: "rg", - }; - const entries: ExecAllowlistEntry[] = [{ pattern: "bin/rg" }]; - const match = matchAllowlist(entries, resolution); + }); expect(match).toBeNull(); }); }); @@ -188,53 +171,105 @@ describe("exec approvals safe shell command builder", () => { }); describe("exec approvals command resolution", () => { - it("resolves PATH executables", () => { - const dir = makeTempDir(); - const binDir = path.join(dir, "bin"); - fs.mkdirSync(binDir, { recursive: true }); - const exeName = process.platform === "win32" ? "rg.exe" : "rg"; - const exe = path.join(binDir, exeName); - fs.writeFileSync(exe, ""); - fs.chmodSync(exe, 0o755); - const res = resolveCommandResolution("rg -n foo", undefined, makePathEnv(binDir)); - expect(res?.resolvedPath).toBe(exe); - expect(res?.executableName).toBe(exeName); - }); + it("resolves PATH, relative, and quoted executables", () => { + const cases = [ + { + name: "PATH executable", + setup: () => { + const dir = makeTempDir(); + const binDir = path.join(dir, "bin"); + fs.mkdirSync(binDir, { recursive: true }); + const exeName = process.platform === "win32" ? "rg.exe" : "rg"; + const exe = path.join(binDir, exeName); + fs.writeFileSync(exe, ""); + fs.chmodSync(exe, 0o755); + return { + command: "rg -n foo", + cwd: undefined as string | undefined, + envPath: makePathEnv(binDir), + expectedPath: exe, + expectedExecutableName: exeName, + }; + }, + }, + { + name: "relative executable", + setup: () => { + const dir = makeTempDir(); + const cwd = path.join(dir, "project"); + const script = path.join(cwd, "scripts", "run.sh"); + fs.mkdirSync(path.dirname(script), { recursive: true }); + fs.writeFileSync(script, ""); + fs.chmodSync(script, 0o755); + return { + command: "./scripts/run.sh --flag", + cwd, + envPath: undefined as string | undefined, + expectedPath: script, + expectedExecutableName: undefined, + }; + }, + }, + { + name: "quoted executable", + setup: () => { + const dir = makeTempDir(); + const cwd = path.join(dir, "project"); + const script = path.join(cwd, "bin", "tool"); + fs.mkdirSync(path.dirname(script), { recursive: true }); + fs.writeFileSync(script, ""); + fs.chmodSync(script, 0o755); + return { + command: '"./bin/tool" --version', + cwd, + envPath: undefined as string | undefined, + expectedPath: script, + expectedExecutableName: undefined, + }; + }, + }, + ] as const; - it("resolves relative paths against cwd", () => { - const dir = makeTempDir(); - const cwd = path.join(dir, "project"); - const script = path.join(cwd, "scripts", "run.sh"); - fs.mkdirSync(path.dirname(script), { recursive: true }); - fs.writeFileSync(script, ""); - fs.chmodSync(script, 0o755); - const res = resolveCommandResolution("./scripts/run.sh --flag", cwd, undefined); - expect(res?.resolvedPath).toBe(script); - }); - - it("parses quoted executables", () => { - const dir = makeTempDir(); - const cwd = path.join(dir, "project"); - const script = path.join(cwd, "bin", "tool"); - fs.mkdirSync(path.dirname(script), { recursive: true }); - fs.writeFileSync(script, ""); - fs.chmodSync(script, 0o755); - const res = resolveCommandResolution('"./bin/tool" --version', cwd, undefined); - expect(res?.resolvedPath).toBe(script); + for (const testCase of cases) { + const setup = testCase.setup(); + const res = resolveCommandResolution(setup.command, setup.cwd, setup.envPath); + expect(res?.resolvedPath, testCase.name).toBe(setup.expectedPath); + if (setup.expectedExecutableName) { + expect(res?.executableName, testCase.name).toBe(setup.expectedExecutableName); + } + } }); }); describe("exec approvals shell parsing", () => { - it("parses simple pipelines", () => { - const res = analyzeShellCommand({ command: "echo ok | jq .foo" }); - expect(res.ok).toBe(true); - expect(res.segments.map((seg) => seg.argv[0])).toEqual(["echo", "jq"]); - }); - - it("parses chained commands", () => { - const res = analyzeShellCommand({ command: "ls && rm -rf /" }); - expect(res.ok).toBe(true); - expect(res.chains?.map((chain) => chain[0]?.argv[0])).toEqual(["ls", "rm"]); + it("parses pipelines and chained commands", () => { + const cases = [ + { + name: "pipeline", + command: "echo ok | jq .foo", + expectedSegments: ["echo", "jq"], + }, + { + name: "chain", + command: "ls && rm -rf /", + expectedChainHeads: ["ls", "rm"], + }, + ] as const; + for (const testCase of cases) { + const res = analyzeShellCommand({ command: testCase.command }); + expect(res.ok, testCase.name).toBe(true); + if (testCase.expectedSegments) { + expect( + res.segments.map((seg) => seg.argv[0]), + testCase.name, + ).toEqual(testCase.expectedSegments); + } else { + expect( + res.chains?.map((chain) => chain[0]?.argv[0]), + testCase.name, + ).toEqual(testCase.expectedChainHeads); + } + } }); it("parses argv commands", () => { @@ -243,180 +278,97 @@ describe("exec approvals shell parsing", () => { expect(res.segments[0]?.argv).toEqual(["/bin/echo", "ok"]); }); - it("rejects command substitution inside double quotes", () => { - const res = analyzeShellCommand({ command: 'echo "output: $(whoami)"' }); - expect(res.ok).toBe(false); - expect(res.reason).toBe("unsupported shell token: $()"); + it("rejects unsupported shell constructs", () => { + const cases: Array<{ command: string; reason: string; platform?: NodeJS.Platform }> = [ + { command: 'echo "output: $(whoami)"', reason: "unsupported shell token: $()" }, + { command: 'echo "output: `id`"', reason: "unsupported shell token: `" }, + { command: "echo $(whoami)", reason: "unsupported shell token: $()" }, + { command: "cat < input.txt", reason: "unsupported shell token: <" }, + { command: "echo ok > output.txt", reason: "unsupported shell token: >" }, + { + command: "/usr/bin/echo first line\n/usr/bin/echo second line", + reason: "unsupported shell token: \n", + }, + { + command: "ping 127.0.0.1 -n 1 & whoami", + reason: "unsupported windows shell token: &", + platform: "win32", + }, + ]; + for (const testCase of cases) { + const res = analyzeShellCommand({ command: testCase.command, platform: testCase.platform }); + expect(res.ok).toBe(false); + expect(res.reason).toBe(testCase.reason); + } }); - it("rejects backticks inside double quotes", () => { - const res = analyzeShellCommand({ command: 'echo "output: `id`"' }); - expect(res.ok).toBe(false); - expect(res.reason).toBe("unsupported shell token: `"); + it("accepts inert substitution-like syntax", () => { + const cases = ['echo "output: \\$(whoami)"', "echo 'output: $(whoami)'"]; + for (const command of cases) { + const res = analyzeShellCommand({ command }); + expect(res.ok).toBe(true); + expect(res.segments[0]?.argv[0]).toBe("echo"); + } }); - it("rejects command substitution outside quotes", () => { - const res = analyzeShellCommand({ command: "echo $(whoami)" }); - expect(res.ok).toBe(false); - expect(res.reason).toBe("unsupported shell token: $()"); + it("accepts safe heredoc forms", () => { + const cases: Array<{ command: string; expectedArgv: string[] }> = [ + { command: "/usr/bin/tee /tmp/file << 'EOF'\nEOF", expectedArgv: ["/usr/bin/tee"] }, + { command: "/usr/bin/tee /tmp/file < segment.argv[0])).toEqual(testCase.expectedArgv); + } }); - it("allows escaped command substitution inside double quotes", () => { - const res = analyzeShellCommand({ command: 'echo "output: \\$(whoami)"' }); - expect(res.ok).toBe(true); - expect(res.segments[0]?.argv[0]).toBe("echo"); - }); - - it("allows command substitution syntax inside single quotes", () => { - const res = analyzeShellCommand({ command: "echo 'output: $(whoami)'" }); - expect(res.ok).toBe(true); - expect(res.segments[0]?.argv[0]).toBe("echo"); - }); - - it("rejects input redirection (<)", () => { - const res = analyzeShellCommand({ command: "cat < input.txt" }); - expect(res.ok).toBe(false); - expect(res.reason).toBe("unsupported shell token: <"); - }); - - it("rejects output redirection (>)", () => { - const res = analyzeShellCommand({ command: "echo ok > output.txt" }); - expect(res.ok).toBe(false); - expect(res.reason).toBe("unsupported shell token: >"); - }); - - it("allows heredoc operator (<<)", () => { - const res = analyzeShellCommand({ command: "/usr/bin/tee /tmp/file << 'EOF'\nEOF" }); - expect(res.ok).toBe(true); - expect(res.segments[0]?.argv[0]).toBe("/usr/bin/tee"); - }); - - it("allows heredoc without space before delimiter", () => { - const res = analyzeShellCommand({ command: "/usr/bin/tee /tmp/file < { - const res = analyzeShellCommand({ command: "/usr/bin/cat <<-DELIM\n\tDELIM" }); - expect(res.ok).toBe(true); - expect(res.segments[0]?.argv[0]).toBe("/usr/bin/cat"); - }); - - it("allows heredoc in pipeline", () => { - const res = analyzeShellCommand({ - command: "/usr/bin/cat << 'EOF' | /usr/bin/grep pattern\npattern\nEOF", - }); - expect(res.ok).toBe(true); - expect(res.segments).toHaveLength(2); - expect(res.segments[0]?.argv[0]).toBe("/usr/bin/cat"); - expect(res.segments[1]?.argv[0]).toBe("/usr/bin/grep"); - }); - - it("allows multiline heredoc body", () => { - const res = analyzeShellCommand({ - command: "/usr/bin/tee /tmp/file << 'EOF'\nline one\nline two\nEOF", - }); - expect(res.ok).toBe(true); - expect(res.segments[0]?.argv[0]).toBe("/usr/bin/tee"); - }); - - it("allows multiline heredoc body with strip-tabs operator (<<-)", () => { - const res = analyzeShellCommand({ - command: "/usr/bin/cat <<-EOF\n\tline one\n\tline two\n\tEOF", - }); - expect(res.ok).toBe(true); - expect(res.segments[0]?.argv[0]).toBe("/usr/bin/cat"); - }); - - it("rejects command substitution in unquoted heredoc body", () => { - const res = analyzeShellCommand({ - command: "/usr/bin/cat < { - const res = analyzeShellCommand({ - command: "/usr/bin/cat < { - const res = analyzeShellCommand({ - command: "/usr/bin/cat < { - const res = analyzeShellCommand({ - command: "/usr/bin/cat < { - const res = analyzeShellCommand({ - command: "/usr/bin/cat <<'EOF'\n$(id)\nEOF", - }); - expect(res.ok).toBe(true); - expect(res.segments[0]?.argv[0]).toBe("/usr/bin/cat"); - }); - - it("allows command substitution in double-quoted heredoc body (shell ignores it)", () => { - const res = analyzeShellCommand({ - command: '/usr/bin/cat <<"EOF"\n$(id)\nEOF', - }); - expect(res.ok).toBe(true); - expect(res.segments[0]?.argv[0]).toBe("/usr/bin/cat"); - }); - - it("rejects nested command substitution in unquoted heredoc", () => { - const res = analyzeShellCommand({ - command: - "/usr/bin/cat < { - const res = analyzeShellCommand({ - command: "/usr/bin/cat < { - const res = analyzeShellCommand({ - command: "/usr/bin/cat < { - const res = analyzeShellCommand({ - command: "/usr/bin/echo first line\n/usr/bin/echo second line", - }); - expect(res.ok).toBe(false); - expect(res.reason).toBe("unsupported shell token: \n"); - }); - - it("rejects windows shell metacharacters", () => { - const res = analyzeShellCommand({ - command: "ping 127.0.0.1 -n 1 & whoami", - platform: "win32", - }); - expect(res.ok).toBe(false); - expect(res.reason).toBe("unsupported windows shell token: &"); + it("rejects unsafe or malformed heredoc forms", () => { + const cases: Array<{ command: string; reason: string }> = [ + { + command: "/usr/bin/cat < { @@ -469,81 +421,67 @@ describe("exec approvals shell parser parity fixture", () => { }); describe("exec approvals shell allowlist (chained commands)", () => { - it("allows chained commands when all parts are allowlisted", () => { - const allowlist: ExecAllowlistEntry[] = [ - { pattern: "/usr/bin/obsidian-cli" }, - { pattern: "/usr/bin/head" }, + it("evaluates chained command allowlist scenarios", () => { + const cases: Array<{ + allowlist: ExecAllowlistEntry[]; + command: string; + expectedAnalysisOk: boolean; + expectedAllowlistSatisfied: boolean; + platform?: NodeJS.Platform; + }> = [ + { + allowlist: [{ pattern: "/usr/bin/obsidian-cli" }, { pattern: "/usr/bin/head" }], + command: + "/usr/bin/obsidian-cli print-default && /usr/bin/obsidian-cli search foo | /usr/bin/head", + expectedAnalysisOk: true, + expectedAllowlistSatisfied: true, + }, + { + allowlist: [{ pattern: "/usr/bin/obsidian-cli" }], + command: "/usr/bin/obsidian-cli print-default && /usr/bin/rm -rf /", + expectedAnalysisOk: true, + expectedAllowlistSatisfied: false, + }, + { + allowlist: [{ pattern: "/usr/bin/echo" }], + command: "/usr/bin/echo ok &&", + expectedAnalysisOk: false, + expectedAllowlistSatisfied: false, + }, + { + allowlist: [{ pattern: "/usr/bin/ping" }], + command: "ping 127.0.0.1 -n 1 & whoami", + expectedAnalysisOk: false, + expectedAllowlistSatisfied: false, + platform: "win32", + }, ]; - const result = evaluateShellAllowlist({ - command: - "/usr/bin/obsidian-cli print-default && /usr/bin/obsidian-cli search foo | /usr/bin/head", - allowlist, - safeBins: new Set(), - cwd: "/tmp", - }); - expect(result.analysisOk).toBe(true); - expect(result.allowlistSatisfied).toBe(true); + for (const testCase of cases) { + const result = evaluateShellAllowlist({ + command: testCase.command, + allowlist: testCase.allowlist, + safeBins: new Set(), + cwd: "/tmp", + platform: testCase.platform, + }); + expect(result.analysisOk).toBe(testCase.expectedAnalysisOk); + expect(result.allowlistSatisfied).toBe(testCase.expectedAllowlistSatisfied); + } }); - it("rejects chained commands when any part is not allowlisted", () => { - const allowlist: ExecAllowlistEntry[] = [{ pattern: "/usr/bin/obsidian-cli" }]; - const result = evaluateShellAllowlist({ - command: "/usr/bin/obsidian-cli print-default && /usr/bin/rm -rf /", - allowlist, - safeBins: new Set(), - cwd: "/tmp", - }); - expect(result.analysisOk).toBe(true); - expect(result.allowlistSatisfied).toBe(false); - }); - - it("returns analysisOk=false for malformed chains", () => { + it("respects quoted chain separators", () => { const allowlist: ExecAllowlistEntry[] = [{ pattern: "/usr/bin/echo" }]; - const result = evaluateShellAllowlist({ - command: "/usr/bin/echo ok &&", - allowlist, - safeBins: new Set(), - cwd: "/tmp", - }); - expect(result.analysisOk).toBe(false); - expect(result.allowlistSatisfied).toBe(false); - }); - - it("respects quotes when splitting chains", () => { - const allowlist: ExecAllowlistEntry[] = [{ pattern: "/usr/bin/echo" }]; - const result = evaluateShellAllowlist({ - command: '/usr/bin/echo "foo && bar"', - allowlist, - safeBins: new Set(), - cwd: "/tmp", - }); - expect(result.analysisOk).toBe(true); - expect(result.allowlistSatisfied).toBe(true); - }); - - it("respects escaped quotes when splitting chains", () => { - const allowlist: ExecAllowlistEntry[] = [{ pattern: "/usr/bin/echo" }]; - const result = evaluateShellAllowlist({ - command: '/usr/bin/echo "foo\\" && bar"', - allowlist, - safeBins: new Set(), - cwd: "/tmp", - }); - expect(result.analysisOk).toBe(true); - expect(result.allowlistSatisfied).toBe(true); - }); - - it("rejects windows chain separators for allowlist analysis", () => { - const allowlist: ExecAllowlistEntry[] = [{ pattern: "/usr/bin/ping" }]; - const result = evaluateShellAllowlist({ - command: "ping 127.0.0.1 -n 1 & whoami", - allowlist, - safeBins: new Set(), - cwd: "/tmp", - platform: "win32", - }); - expect(result.analysisOk).toBe(false); - expect(result.allowlistSatisfied).toBe(false); + const commands = ['/usr/bin/echo "foo && bar"', '/usr/bin/echo "foo\\" && bar"']; + for (const command of commands) { + const result = evaluateShellAllowlist({ + command, + allowlist, + safeBins: new Set(), + cwd: "/tmp", + }); + expect(result.analysisOk).toBe(true); + expect(result.allowlistSatisfied).toBe(true); + } }); }); @@ -1052,46 +990,54 @@ describe("exec approvals node host allowlist check", () => { // The node host checks: matchAllowlist() || isSafeBinUsage() for each command segment // Using hardcoded resolution objects for cross-platform compatibility - it("satisfies allowlist when command matches exact path pattern", () => { - const resolution = { - rawExecutable: "python3", - resolvedPath: "/usr/bin/python3", - executableName: "python3", - }; - const entries: ExecAllowlistEntry[] = [{ pattern: "/usr/bin/python3" }]; - const match = matchAllowlist(entries, resolution); - expect(match).not.toBeNull(); - expect(match?.pattern).toBe("/usr/bin/python3"); + it("matches exact and wildcard allowlist patterns", () => { + const cases: Array<{ + resolution: { rawExecutable: string; resolvedPath: string; executableName: string }; + entries: ExecAllowlistEntry[]; + expectedPattern: string | null; + }> = [ + { + resolution: { + rawExecutable: "python3", + resolvedPath: "/usr/bin/python3", + executableName: "python3", + }, + entries: [{ pattern: "/usr/bin/python3" }], + expectedPattern: "/usr/bin/python3", + }, + { + // Simulates symlink resolution: + // /opt/homebrew/bin/python3 -> /opt/homebrew/opt/python@3.14/bin/python3.14 + resolution: { + rawExecutable: "python3", + resolvedPath: "/opt/homebrew/opt/python@3.14/bin/python3.14", + executableName: "python3.14", + }, + entries: [{ pattern: "/opt/**/python*" }], + expectedPattern: "/opt/**/python*", + }, + { + resolution: { + rawExecutable: "unknown-tool", + resolvedPath: "/usr/local/bin/unknown-tool", + executableName: "unknown-tool", + }, + entries: [{ pattern: "/usr/bin/python3" }, { pattern: "/opt/**/node" }], + expectedPattern: null, + }, + ]; + for (const testCase of cases) { + const match = matchAllowlist(testCase.entries, testCase.resolution); + expect(match?.pattern ?? null).toBe(testCase.expectedPattern); + } }); - it("satisfies allowlist when command matches ** wildcard pattern", () => { - // Simulates symlink resolution: /opt/homebrew/bin/python3 -> /opt/homebrew/opt/python@3.14/bin/python3.14 - const resolution = { - rawExecutable: "python3", - resolvedPath: "/opt/homebrew/opt/python@3.14/bin/python3.14", - executableName: "python3.14", - }; - // Pattern with ** matches across multiple directories - const entries: ExecAllowlistEntry[] = [{ pattern: "/opt/**/python*" }]; - const match = matchAllowlist(entries, resolution); - expect(match?.pattern).toBe("/opt/**/python*"); - }); - - it("does not satisfy allowlist when command is not in allowlist", () => { + it("does not treat unknown tools as safe bins", () => { const resolution = { rawExecutable: "unknown-tool", resolvedPath: "/usr/local/bin/unknown-tool", executableName: "unknown-tool", }; - // Allowlist has different commands - const entries: ExecAllowlistEntry[] = [ - { pattern: "/usr/bin/python3" }, - { pattern: "/opt/**/node" }, - ]; - const match = matchAllowlist(entries, resolution); - expect(match).toBeNull(); - - // Also not a safe bin const safe = isSafeBinUsage({ argv: ["unknown-tool", "--help"], resolution, @@ -1156,6 +1102,20 @@ describe("exec approvals default agent migration", () => { }); describe("normalizeExecApprovals handles string allowlist entries (#9790)", () => { + function getMainAllowlistPatterns(file: ExecApprovalsFile): string[] | undefined { + const normalized = normalizeExecApprovals(file); + return normalized.agents?.main?.allowlist?.map((entry) => entry.pattern); + } + + function expectNoSpreadStringArtifacts(entries: ExecAllowlistEntry[]) { + for (const entry of entries) { + expect(entry).toHaveProperty("pattern"); + expect(typeof entry.pattern).toBe("string"); + expect(entry.pattern.length).toBeGreaterThan(0); + expect(entry).not.toHaveProperty("0"); + } + } + it("converts bare string entries to proper ExecAllowlistEntry objects", () => { // Simulates a corrupted or legacy config where allowlist contains plain // strings (e.g. ["ls", "cat"]) instead of { pattern: "..." } objects. @@ -1172,15 +1132,8 @@ describe("normalizeExecApprovals handles string allowlist entries (#9790)", () = const normalized = normalizeExecApprovals(file); const entries = normalized.agents?.main?.allowlist ?? []; - // Each entry must be a proper object with a pattern string, not a - // spread-string like {"0":"t","1":"h","2":"i",...} - for (const entry of entries) { - expect(entry).toHaveProperty("pattern"); - expect(typeof entry.pattern).toBe("string"); - expect(entry.pattern.length).toBeGreaterThan(0); - // Spread-string corruption would create numeric keys — ensure none exist - expect(entry).not.toHaveProperty("0"); - } + // Spread-string corruption would create numeric keys — ensure none exist. + expectNoSpreadStringArtifacts(entries); expect(entries.map((e) => e.pattern)).toEqual([ "things", @@ -1212,73 +1165,51 @@ describe("normalizeExecApprovals handles string allowlist entries (#9790)", () = expect(entries[1]?.id).toBe("existing-id"); }); - it("handles mixed string and object entries in the same allowlist", () => { - const file = { - version: 1, - agents: { - main: { - allowlist: ["ls", { pattern: "/usr/bin/cat" }, "echo"], - }, + it("sanitizes mixed and malformed allowlist shapes", () => { + const cases: Array<{ + name: string; + allowlist: unknown; + expectedPatterns: string[] | undefined; + }> = [ + { + name: "mixed entries", + allowlist: ["ls", { pattern: "/usr/bin/cat" }, "echo"], + expectedPatterns: ["ls", "/usr/bin/cat", "echo"], }, - } as unknown as ExecApprovalsFile; + { + name: "empty strings dropped", + allowlist: ["", " ", "ls"], + expectedPatterns: ["ls"], + }, + { + name: "malformed objects dropped", + allowlist: [{ pattern: "/usr/bin/ls" }, {}, { pattern: 123 }, { pattern: " " }, "echo"], + expectedPatterns: ["/usr/bin/ls", "echo"], + }, + { + name: "non-array dropped", + allowlist: "ls", + expectedPatterns: undefined, + }, + ]; - const normalized = normalizeExecApprovals(file); - const entries = normalized.agents?.main?.allowlist ?? []; - - expect(entries).toHaveLength(3); - expect(entries.map((e) => e.pattern)).toEqual(["ls", "/usr/bin/cat", "echo"]); - for (const entry of entries) { - expect(entry).not.toHaveProperty("0"); + for (const testCase of cases) { + const patterns = getMainAllowlistPatterns({ + version: 1, + agents: { + main: { allowlist: testCase.allowlist } as ExecApprovalsFile["agents"]["main"], + }, + }); + expect(patterns, testCase.name).toEqual(testCase.expectedPatterns); + if (patterns) { + const entries = normalizeExecApprovals({ + version: 1, + agents: { + main: { allowlist: testCase.allowlist } as ExecApprovalsFile["agents"]["main"], + }, + }).agents?.main?.allowlist; + expectNoSpreadStringArtifacts(entries ?? []); + } } }); - - it("drops empty string entries", () => { - const file = { - version: 1, - agents: { - main: { - allowlist: ["", " ", "ls"], - }, - }, - } as unknown as ExecApprovalsFile; - - const normalized = normalizeExecApprovals(file); - const entries = normalized.agents?.main?.allowlist ?? []; - - // Only "ls" should survive; empty/whitespace strings should be dropped - expect(entries.map((e) => e.pattern)).toEqual(["ls"]); - }); - - it("drops malformed object entries with missing/non-string patterns", () => { - const file = { - version: 1, - agents: { - main: { - allowlist: [{ pattern: "/usr/bin/ls" }, {}, { pattern: 123 }, { pattern: " " }, "echo"], - }, - }, - } as unknown as ExecApprovalsFile; - - const normalized = normalizeExecApprovals(file); - const entries = normalized.agents?.main?.allowlist ?? []; - - expect(entries.map((e) => e.pattern)).toEqual(["/usr/bin/ls", "echo"]); - for (const entry of entries) { - expect(entry).not.toHaveProperty("0"); - } - }); - - it("drops non-array allowlist values", () => { - const file = { - version: 1, - agents: { - main: { - allowlist: "ls", - }, - }, - } as unknown as ExecApprovalsFile; - - const normalized = normalizeExecApprovals(file); - expect(normalized.agents?.main?.allowlist).toBeUndefined(); - }); }); diff --git a/src/infra/heartbeat-runner.returns-default-unset.test.ts b/src/infra/heartbeat-runner.returns-default-unset.test.ts index 260ea1b2821..ccdfc62859f 100644 --- a/src/infra/heartbeat-runner.returns-default-unset.test.ts +++ b/src/infra/heartbeat-runner.returns-default-unset.test.ts @@ -214,117 +214,117 @@ describe("resolveHeartbeatDeliveryTarget", () => { updatedAt: Date.now(), }; - it("respects target none", () => { - const cfg: OpenClawConfig = { - agents: { defaults: { heartbeat: { target: "none" } } }, - }; - expect(resolveHeartbeatDeliveryTarget({ cfg, entry: baseEntry })).toEqual({ - channel: "none", - reason: "target-none", - accountId: undefined, - lastChannel: undefined, - lastAccountId: undefined, - }); - }); - - it("uses last route by default", () => { - const cfg: OpenClawConfig = {}; - const entry = { - ...baseEntry, - lastChannel: "whatsapp" as const, - lastTo: "+1555", - }; - expect(resolveHeartbeatDeliveryTarget({ cfg, entry })).toEqual({ - channel: "whatsapp", - to: "+1555", - accountId: undefined, - lastChannel: "whatsapp", - lastAccountId: undefined, - }); - }); - - it("normalizes explicit WhatsApp targets when allowFrom is '*'", () => { - const cfg: OpenClawConfig = { - agents: { - defaults: { - heartbeat: { target: "whatsapp", to: "whatsapp:(555) 123" }, + it("resolves target variants across route and allowlist rules", () => { + const cases: Array<{ + name: string; + cfg: OpenClawConfig; + entry: typeof baseEntry & { + lastChannel?: "whatsapp" | "telegram" | "webchat"; + lastTo?: string; + }; + expected: ReturnType; + }> = [ + { + name: "target none", + cfg: { agents: { defaults: { heartbeat: { target: "none" } } } }, + entry: baseEntry, + expected: { + channel: "none", + reason: "target-none", + accountId: undefined, + lastChannel: undefined, + lastAccountId: undefined, }, }, - channels: { whatsapp: { allowFrom: ["*"] } }, - }; - expect(resolveHeartbeatDeliveryTarget({ cfg, entry: baseEntry })).toEqual({ - channel: "whatsapp", - to: "+555123", - accountId: undefined, - lastChannel: undefined, - lastAccountId: undefined, - }); - }); - - it("skips when last route is webchat", () => { - const cfg: OpenClawConfig = {}; - const entry = { - ...baseEntry, - lastChannel: "webchat" as const, - lastTo: "web", - }; - expect(resolveHeartbeatDeliveryTarget({ cfg, entry })).toEqual({ - channel: "none", - reason: "no-target", - accountId: undefined, - lastChannel: undefined, - lastAccountId: undefined, - }); - }); - - it("rejects WhatsApp target not in allowFrom (no silent fallback)", () => { - const cfg: OpenClawConfig = { - agents: { defaults: { heartbeat: { target: "whatsapp", to: "+1999" } } }, - channels: { whatsapp: { allowFrom: ["+1555", "+1666"] } }, - }; - const entry = { - ...baseEntry, - lastChannel: "whatsapp" as const, - lastTo: "+1222", - }; - expect(resolveHeartbeatDeliveryTarget({ cfg, entry })).toEqual({ - channel: "none", - reason: "no-target", - accountId: undefined, - lastChannel: "whatsapp", - lastAccountId: undefined, - }); - }); - - it("normalizes prefixed WhatsApp group targets for heartbeat delivery", () => { - const cfg: OpenClawConfig = { - channels: { whatsapp: { allowFrom: ["+1555"] } }, - }; - const entry = { - ...baseEntry, - lastChannel: "whatsapp" as const, - lastTo: "whatsapp:120363401234567890@G.US", - }; - expect(resolveHeartbeatDeliveryTarget({ cfg, entry })).toEqual({ - channel: "whatsapp", - to: "120363401234567890@g.us", - accountId: undefined, - lastChannel: "whatsapp", - lastAccountId: undefined, - }); - }); - - it("keeps explicit telegram targets", () => { - const cfg: OpenClawConfig = { - agents: { defaults: { heartbeat: { target: "telegram", to: "123" } } }, - }; - expect(resolveHeartbeatDeliveryTarget({ cfg, entry: baseEntry })).toEqual({ - channel: "telegram", - to: "123", - accountId: undefined, - lastChannel: undefined, - lastAccountId: undefined, - }); + { + name: "use last route by default", + cfg: {}, + entry: { ...baseEntry, lastChannel: "whatsapp", lastTo: "+1555" }, + expected: { + channel: "whatsapp", + to: "+1555", + accountId: undefined, + lastChannel: "whatsapp", + lastAccountId: undefined, + }, + }, + { + name: "normalize explicit whatsapp target when allowFrom wildcard", + cfg: { + agents: { defaults: { heartbeat: { target: "whatsapp", to: "whatsapp:(555) 123" } } }, + channels: { whatsapp: { allowFrom: ["*"] } }, + }, + entry: baseEntry, + expected: { + channel: "whatsapp", + to: "+555123", + accountId: undefined, + lastChannel: undefined, + lastAccountId: undefined, + }, + }, + { + name: "skip webchat last route", + cfg: {}, + entry: { ...baseEntry, lastChannel: "webchat", lastTo: "web" }, + expected: { + channel: "none", + reason: "no-target", + accountId: undefined, + lastChannel: undefined, + lastAccountId: undefined, + }, + }, + { + name: "reject explicit whatsapp target outside allowFrom", + cfg: { + agents: { defaults: { heartbeat: { target: "whatsapp", to: "+1999" } } }, + channels: { whatsapp: { allowFrom: ["+1555", "+1666"] } }, + }, + entry: { ...baseEntry, lastChannel: "whatsapp", lastTo: "+1222" }, + expected: { + channel: "none", + reason: "no-target", + accountId: undefined, + lastChannel: "whatsapp", + lastAccountId: undefined, + }, + }, + { + name: "normalize prefixed whatsapp group targets", + cfg: { channels: { whatsapp: { allowFrom: ["+1555"] } } }, + entry: { + ...baseEntry, + lastChannel: "whatsapp", + lastTo: "whatsapp:120363401234567890@G.US", + }, + expected: { + channel: "whatsapp", + to: "120363401234567890@g.us", + accountId: undefined, + lastChannel: "whatsapp", + lastAccountId: undefined, + }, + }, + { + name: "keep explicit telegram target", + cfg: { agents: { defaults: { heartbeat: { target: "telegram", to: "123" } } } }, + entry: baseEntry, + expected: { + channel: "telegram", + to: "123", + accountId: undefined, + lastChannel: undefined, + lastAccountId: undefined, + }, + }, + ]; + for (const testCase of cases) { + expect( + resolveHeartbeatDeliveryTarget({ cfg: testCase.cfg, entry: testCase.entry }), + testCase.name, + ).toEqual(testCase.expected); + } }); it("parses optional telegram :topic: threadId suffix", () => { @@ -439,6 +439,14 @@ describe("resolveHeartbeatSenderContext", () => { }); describe("runHeartbeatOnce", () => { + const createHeartbeatDeps = (sendWhatsApp: ReturnType, nowMs = 0) => ({ + sendWhatsApp, + getQueueSize: () => 0, + nowMs: () => nowMs, + webAuthExists: async () => true, + hasActiveWebListener: () => true, + }); + it("skips when agent heartbeat is not enabled", async () => { const cfg: OpenClawConfig = { agents: { @@ -515,13 +523,7 @@ describe("runHeartbeatOnce", () => { await runHeartbeatOnce({ cfg, - deps: { - sendWhatsApp, - getQueueSize: () => 0, - nowMs: () => 0, - webAuthExists: async () => true, - hasActiveWebListener: () => true, - }, + deps: createHeartbeatDeps(sendWhatsApp), }); expect(sendWhatsApp).toHaveBeenCalledTimes(1); @@ -574,13 +576,7 @@ describe("runHeartbeatOnce", () => { await runHeartbeatOnce({ cfg, agentId: "ops", - deps: { - sendWhatsApp, - getQueueSize: () => 0, - nowMs: () => 0, - webAuthExists: async () => true, - hasActiveWebListener: () => true, - }, + deps: createHeartbeatDeps(sendWhatsApp), }); expect(sendWhatsApp).toHaveBeenCalledTimes(1); expect(sendWhatsApp).toHaveBeenCalledWith("+1555", "Final alert", expect.any(Object)); @@ -656,13 +652,7 @@ describe("runHeartbeatOnce", () => { const result = await runHeartbeatOnce({ cfg, agentId, - deps: { - sendWhatsApp, - getQueueSize: () => 0, - nowMs: () => 0, - webAuthExists: async () => true, - hasActiveWebListener: () => true, - }, + deps: createHeartbeatDeps(sendWhatsApp), }); expect(result.status).toBe("ran"); @@ -683,159 +673,107 @@ describe("runHeartbeatOnce", () => { } }); - it("runs heartbeats in the explicit session key when configured", async () => { - const tmpDir = await createCaseDir("hb-explicit-session"); - const storePath = path.join(tmpDir, "sessions.json"); + it("resolves configured and forced session key overrides", async () => { const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); try { - const groupId = "120363401234567890@g.us"; - const cfg: OpenClawConfig = { - agents: { - defaults: { - workspace: tmpDir, - heartbeat: { - every: "5m", - target: "last", + const cases = [ + { + name: "heartbeat.session", + caseDir: "hb-explicit-session", + peerKind: "group" as const, + peerId: "120363401234567890@g.us", + message: "Group alert", + applyOverride: ({ cfg, sessionKey }: { cfg: OpenClawConfig; sessionKey: string }) => { + if (cfg.agents?.defaults?.heartbeat) { + cfg.agents.defaults.heartbeat.session = sessionKey; + } + }, + runOptions: ({ sessionKey: _sessionKey }: { sessionKey: string }) => ({ + sessionKey: undefined as string | undefined, + }), + }, + { + name: "runHeartbeatOnce sessionKey arg", + caseDir: "hb-forced-session-override", + peerKind: "direct" as const, + peerId: "+15559990000", + message: "Forced alert", + applyOverride: () => {}, + runOptions: ({ sessionKey }: { sessionKey: string }) => ({ sessionKey }), + }, + ] as const; + + for (const testCase of cases) { + const tmpDir = await createCaseDir(testCase.caseDir); + const storePath = path.join(tmpDir, "sessions.json"); + const cfg: OpenClawConfig = { + agents: { + defaults: { + workspace: tmpDir, + heartbeat: { + every: "5m", + target: "last", + }, }, }, - }, - channels: { whatsapp: { allowFrom: ["*"] } }, - session: { store: storePath }, - }; - const mainSessionKey = resolveMainSessionKey(cfg); - const agentId = resolveAgentIdFromSessionKey(mainSessionKey); - const groupSessionKey = buildAgentPeerSessionKey({ - agentId, - channel: "whatsapp", - peerKind: "group", - peerId: groupId, - }); - if (cfg.agents?.defaults?.heartbeat) { - cfg.agents.defaults.heartbeat.session = groupSessionKey; + channels: { whatsapp: { allowFrom: ["*"] } }, + session: { store: storePath }, + }; + const mainSessionKey = resolveMainSessionKey(cfg); + const agentId = resolveAgentIdFromSessionKey(mainSessionKey); + const overrideSessionKey = buildAgentPeerSessionKey({ + agentId, + channel: "whatsapp", + peerKind: testCase.peerKind, + peerId: testCase.peerId, + }); + testCase.applyOverride({ cfg, sessionKey: overrideSessionKey }); + + await fs.writeFile( + storePath, + JSON.stringify({ + [mainSessionKey]: { + sessionId: "sid-main", + updatedAt: Date.now(), + lastChannel: "whatsapp", + lastTo: "+1555", + }, + [overrideSessionKey]: { + sessionId: `sid-${testCase.peerKind}`, + updatedAt: Date.now() + 10_000, + lastChannel: "whatsapp", + lastTo: testCase.peerId, + }, + }), + ); + + replySpy.mockReset(); + replySpy.mockResolvedValue([{ text: testCase.message }]); + const sendWhatsApp = vi.fn().mockResolvedValue({ messageId: "m1", toJid: "jid" }); + + await runHeartbeatOnce({ + cfg, + ...testCase.runOptions({ sessionKey: overrideSessionKey }), + deps: createHeartbeatDeps(sendWhatsApp), + }); + + expect(sendWhatsApp, testCase.name).toHaveBeenCalledTimes(1); + expect(sendWhatsApp, testCase.name).toHaveBeenCalledWith( + testCase.peerId, + testCase.message, + expect.any(Object), + ); + expect(replySpy, testCase.name).toHaveBeenCalledWith( + expect.objectContaining({ + SessionKey: overrideSessionKey, + From: testCase.peerId, + To: testCase.peerId, + Provider: "heartbeat", + }), + expect.objectContaining({ isHeartbeat: true, suppressToolErrorWarnings: false }), + cfg, + ); } - - await fs.writeFile( - storePath, - JSON.stringify({ - [mainSessionKey]: { - sessionId: "sid-main", - updatedAt: Date.now(), - lastChannel: "whatsapp", - lastTo: "+1555", - }, - [groupSessionKey]: { - sessionId: "sid-group", - updatedAt: Date.now() + 10_000, - lastChannel: "whatsapp", - lastTo: groupId, - }, - }), - ); - - replySpy.mockResolvedValue([{ text: "Group alert" }]); - const sendWhatsApp = vi.fn().mockResolvedValue({ - messageId: "m1", - toJid: "jid", - }); - - await runHeartbeatOnce({ - cfg, - deps: { - sendWhatsApp, - getQueueSize: () => 0, - nowMs: () => 0, - webAuthExists: async () => true, - hasActiveWebListener: () => true, - }, - }); - - expect(sendWhatsApp).toHaveBeenCalledTimes(1); - expect(sendWhatsApp).toHaveBeenCalledWith(groupId, "Group alert", expect.any(Object)); - expect(replySpy).toHaveBeenCalledWith( - expect.objectContaining({ - SessionKey: groupSessionKey, - From: groupId, - To: groupId, - Provider: "heartbeat", - }), - expect.objectContaining({ isHeartbeat: true, suppressToolErrorWarnings: false }), - cfg, - ); - } finally { - replySpy.mockRestore(); - } - }); - - it("runs heartbeats in forced session key overrides passed at call time", async () => { - const tmpDir = await createCaseDir("hb-forced-session-override"); - const storePath = path.join(tmpDir, "sessions.json"); - const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); - try { - const cfg: OpenClawConfig = { - agents: { - defaults: { - workspace: tmpDir, - heartbeat: { - every: "5m", - target: "last", - }, - }, - }, - channels: { whatsapp: { allowFrom: ["*"] } }, - session: { store: storePath }, - }; - const mainSessionKey = resolveMainSessionKey(cfg); - const agentId = resolveAgentIdFromSessionKey(mainSessionKey); - const forcedSessionKey = buildAgentPeerSessionKey({ - agentId, - channel: "whatsapp", - peerKind: "direct", - peerId: "+15559990000", - }); - - await fs.writeFile( - storePath, - JSON.stringify({ - [mainSessionKey]: { - sessionId: "sid-main", - updatedAt: Date.now(), - lastChannel: "whatsapp", - lastTo: "+1555", - }, - [forcedSessionKey]: { - sessionId: "sid-forced", - updatedAt: Date.now() + 10_000, - lastChannel: "whatsapp", - lastTo: "+15559990000", - }, - }), - ); - - replySpy.mockResolvedValue([{ text: "Forced alert" }]); - const sendWhatsApp = vi.fn().mockResolvedValue({ - messageId: "m1", - toJid: "jid", - }); - - await runHeartbeatOnce({ - cfg, - sessionKey: forcedSessionKey, - deps: { - sendWhatsApp, - getQueueSize: () => 0, - nowMs: () => 0, - webAuthExists: async () => true, - hasActiveWebListener: () => true, - }, - }); - - expect(sendWhatsApp).toHaveBeenCalledTimes(1); - expect(sendWhatsApp).toHaveBeenCalledWith("+15559990000", "Forced alert", expect.any(Object)); - expect(replySpy).toHaveBeenCalledWith( - expect.objectContaining({ SessionKey: forcedSessionKey }), - expect.objectContaining({ isHeartbeat: true }), - cfg, - ); } finally { replySpy.mockRestore(); } @@ -877,13 +815,7 @@ describe("runHeartbeatOnce", () => { await runHeartbeatOnce({ cfg, - deps: { - sendWhatsApp, - getQueueSize: () => 0, - nowMs: () => 60_000, - webAuthExists: async () => true, - hasActiveWebListener: () => true, - }, + deps: createHeartbeatDeps(sendWhatsApp, 60_000), }); expect(sendWhatsApp).toHaveBeenCalledTimes(0); @@ -892,134 +824,75 @@ describe("runHeartbeatOnce", () => { } }); - it("can include reasoning payloads when enabled", async () => { - const tmpDir = await createCaseDir("hb-reasoning"); - const storePath = path.join(tmpDir, "sessions.json"); + it("handles reasoning payload delivery variants", async () => { const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); try { - const cfg: OpenClawConfig = { - agents: { - defaults: { - workspace: tmpDir, - heartbeat: { - every: "5m", - target: "whatsapp", - includeReasoning: true, + const cases = [ + { + name: "reasoning + final payload", + caseDir: "hb-reasoning", + replies: [{ text: "Reasoning:\n_Because it helps_" }, { text: "Final alert" }], + expectedTexts: ["Reasoning:\n_Because it helps_", "Final alert"], + }, + { + name: "reasoning + HEARTBEAT_OK", + caseDir: "hb-reasoning-heartbeat-ok", + replies: [{ text: "Reasoning:\n_Because it helps_" }, { text: "HEARTBEAT_OK" }], + expectedTexts: ["Reasoning:\n_Because it helps_"], + }, + ] as const; + + for (const testCase of cases) { + const tmpDir = await createCaseDir(testCase.caseDir); + const storePath = path.join(tmpDir, "sessions.json"); + const cfg: OpenClawConfig = { + agents: { + defaults: { + workspace: tmpDir, + heartbeat: { + every: "5m", + target: "whatsapp", + includeReasoning: true, + }, }, }, - }, - channels: { whatsapp: { allowFrom: ["*"] } }, - session: { store: storePath }, - }; - const sessionKey = resolveMainSessionKey(cfg); + channels: { whatsapp: { allowFrom: ["*"] } }, + session: { store: storePath }, + }; + const sessionKey = resolveMainSessionKey(cfg); - await fs.writeFile( - storePath, - JSON.stringify({ - [sessionKey]: { - sessionId: "sid", - updatedAt: Date.now(), - lastChannel: "whatsapp", - lastProvider: "whatsapp", - lastTo: "+1555", - }, - }), - ); - - replySpy.mockResolvedValue([ - { text: "Reasoning:\n_Because it helps_" }, - { text: "Final alert" }, - ]); - const sendWhatsApp = vi.fn().mockResolvedValue({ - messageId: "m1", - toJid: "jid", - }); - - await runHeartbeatOnce({ - cfg, - deps: { - sendWhatsApp, - getQueueSize: () => 0, - nowMs: () => 0, - webAuthExists: async () => true, - hasActiveWebListener: () => true, - }, - }); - - expect(sendWhatsApp).toHaveBeenCalledTimes(2); - expect(sendWhatsApp).toHaveBeenNthCalledWith( - 1, - "+1555", - "Reasoning:\n_Because it helps_", - expect.any(Object), - ); - expect(sendWhatsApp).toHaveBeenNthCalledWith(2, "+1555", "Final alert", expect.any(Object)); - } finally { - replySpy.mockRestore(); - } - }); - - it("delivers reasoning even when the main heartbeat reply is HEARTBEAT_OK", async () => { - const tmpDir = await createCaseDir("hb-reasoning-heartbeat-ok"); - const storePath = path.join(tmpDir, "sessions.json"); - const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); - try { - const cfg: OpenClawConfig = { - agents: { - defaults: { - workspace: tmpDir, - heartbeat: { - every: "5m", - target: "whatsapp", - includeReasoning: true, + await fs.writeFile( + storePath, + JSON.stringify({ + [sessionKey]: { + sessionId: "sid", + updatedAt: Date.now(), + lastChannel: "whatsapp", + lastProvider: "whatsapp", + lastTo: "+1555", }, - }, - }, - channels: { whatsapp: { allowFrom: ["*"] } }, - session: { store: storePath }, - }; - const sessionKey = resolveMainSessionKey(cfg); + }), + ); - await fs.writeFile( - storePath, - JSON.stringify({ - [sessionKey]: { - sessionId: "sid", - updatedAt: Date.now(), - lastChannel: "whatsapp", - lastProvider: "whatsapp", - lastTo: "+1555", - }, - }), - ); + replySpy.mockReset(); + replySpy.mockResolvedValue(testCase.replies); + const sendWhatsApp = vi.fn().mockResolvedValue({ messageId: "m1", toJid: "jid" }); - replySpy.mockResolvedValue([ - { text: "Reasoning:\n_Because it helps_" }, - { text: "HEARTBEAT_OK" }, - ]); - const sendWhatsApp = vi.fn().mockResolvedValue({ - messageId: "m1", - toJid: "jid", - }); + await runHeartbeatOnce({ + cfg, + deps: createHeartbeatDeps(sendWhatsApp), + }); - await runHeartbeatOnce({ - cfg, - deps: { - sendWhatsApp, - getQueueSize: () => 0, - nowMs: () => 0, - webAuthExists: async () => true, - hasActiveWebListener: () => true, - }, - }); - - expect(sendWhatsApp).toHaveBeenCalledTimes(1); - expect(sendWhatsApp).toHaveBeenNthCalledWith( - 1, - "+1555", - "Reasoning:\n_Because it helps_", - expect.any(Object), - ); + expect(sendWhatsApp, testCase.name).toHaveBeenCalledTimes(testCase.expectedTexts.length); + for (const [index, text] of testCase.expectedTexts.entries()) { + expect(sendWhatsApp, testCase.name).toHaveBeenNthCalledWith( + index + 1, + "+1555", + text, + expect.any(Object), + ); + } + } } finally { replySpy.mockRestore(); } @@ -1068,13 +941,7 @@ describe("runHeartbeatOnce", () => { await runHeartbeatOnce({ cfg, - deps: { - sendWhatsApp, - getQueueSize: () => 0, - nowMs: () => 0, - webAuthExists: async () => true, - hasActiveWebListener: () => true, - }, + deps: createHeartbeatDeps(sendWhatsApp), }); expect(sendWhatsApp).toHaveBeenCalledTimes(1); @@ -1088,547 +955,181 @@ describe("runHeartbeatOnce", () => { } }); - it("skips heartbeat when HEARTBEAT.md is effectively empty (saves API calls)", async () => { + type HeartbeatFileState = "empty" | "actionable" | "missing" | "read-error"; + + async function runHeartbeatFileScenario(params: { + fileState: HeartbeatFileState; + reason?: "interval" | "wake"; + queueCronEvent?: boolean; + replyText?: string; + }) { const tmpDir = await createCaseDir("openclaw-hb"); const storePath = path.join(tmpDir, "sessions.json"); const workspaceDir = path.join(tmpDir, "workspace"); - const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); - try { - await fs.mkdir(workspaceDir, { recursive: true }); + await fs.mkdir(workspaceDir, { recursive: true }); - // Create effectively empty HEARTBEAT.md (only header and comments) + if (params.fileState === "empty") { await fs.writeFile( path.join(workspaceDir, "HEARTBEAT.md"), "# HEARTBEAT.md\n\n## Tasks\n\n", "utf-8", ); - - const cfg: OpenClawConfig = { - agents: { - defaults: { - workspace: workspaceDir, - heartbeat: { every: "5m", target: "whatsapp" }, - }, - }, - channels: { whatsapp: { allowFrom: ["*"] } }, - session: { store: storePath }, - }; - const sessionKey = resolveMainSessionKey(cfg); - - await fs.writeFile( - storePath, - JSON.stringify( - { - [sessionKey]: { - sessionId: "sid", - updatedAt: Date.now(), - lastChannel: "whatsapp", - lastTo: "+1555", - }, - }, - null, - 2, - ), - ); - - const sendWhatsApp = vi.fn().mockResolvedValue({ - messageId: "m1", - toJid: "jid", - }); - - const res = await runHeartbeatOnce({ - cfg, - deps: { - sendWhatsApp, - getQueueSize: () => 0, - nowMs: () => 0, - webAuthExists: async () => true, - hasActiveWebListener: () => true, - }, - }); - - // Should skip without making API call - expect(res.status).toBe("skipped"); - if (res.status === "skipped") { - expect(res.reason).toBe("empty-heartbeat-file"); - } - expect(replySpy).not.toHaveBeenCalled(); - expect(sendWhatsApp).not.toHaveBeenCalled(); - } finally { - replySpy.mockRestore(); - } - }); - - it("does not skip wake-triggered heartbeat when HEARTBEAT.md is effectively empty", async () => { - const tmpDir = await createCaseDir("openclaw-hb"); - const storePath = path.join(tmpDir, "sessions.json"); - const workspaceDir = path.join(tmpDir, "workspace"); - const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); - try { - await fs.mkdir(workspaceDir, { recursive: true }); - await fs.writeFile( - path.join(workspaceDir, "HEARTBEAT.md"), - "# HEARTBEAT.md\n\n## Tasks\n\n", - "utf-8", - ); - - const cfg: OpenClawConfig = { - agents: { - defaults: { - workspace: workspaceDir, - heartbeat: { every: "5m", target: "whatsapp" }, - }, - }, - channels: { whatsapp: { allowFrom: ["*"] } }, - session: { store: storePath }, - }; - const sessionKey = resolveMainSessionKey(cfg); - - await fs.writeFile( - storePath, - JSON.stringify( - { - [sessionKey]: { - sessionId: "sid", - updatedAt: Date.now(), - lastChannel: "whatsapp", - lastTo: "+1555", - }, - }, - null, - 2, - ), - ); - - replySpy.mockResolvedValue({ text: "wake event processed" }); - const sendWhatsApp = vi.fn().mockResolvedValue({ - messageId: "m1", - toJid: "jid", - }); - - const res = await runHeartbeatOnce({ - cfg, - reason: "wake", - deps: { - sendWhatsApp, - getQueueSize: () => 0, - nowMs: () => 0, - webAuthExists: async () => true, - hasActiveWebListener: () => true, - }, - }); - - expect(res.status).toBe("ran"); - expect(replySpy).toHaveBeenCalled(); - expect(sendWhatsApp).toHaveBeenCalledTimes(1); - } finally { - replySpy.mockRestore(); - } - }); - - it("does not skip interval heartbeat when HEARTBEAT.md is empty but tagged cron events are queued", async () => { - const tmpDir = await createCaseDir("openclaw-hb"); - const storePath = path.join(tmpDir, "sessions.json"); - const workspaceDir = path.join(tmpDir, "workspace"); - const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); - try { - await fs.mkdir(workspaceDir, { recursive: true }); - await fs.writeFile( - path.join(workspaceDir, "HEARTBEAT.md"), - "# HEARTBEAT.md\n\n## Tasks\n\n", - "utf-8", - ); - - const cfg: OpenClawConfig = { - agents: { - defaults: { - workspace: workspaceDir, - heartbeat: { every: "5m", target: "whatsapp" }, - }, - }, - channels: { whatsapp: { allowFrom: ["*"] } }, - session: { store: storePath }, - }; - const sessionKey = resolveMainSessionKey(cfg); - - await fs.writeFile( - storePath, - JSON.stringify( - { - [sessionKey]: { - sessionId: "sid", - updatedAt: Date.now(), - lastChannel: "whatsapp", - lastTo: "+1555", - }, - }, - null, - 2, - ), - ); - - enqueueSystemEvent("Cron: QMD maintenance completed", { - sessionKey, - contextKey: "cron:qmd-maintenance", - }); - - replySpy.mockResolvedValue({ text: "Relay this cron update now" }); - const sendWhatsApp = vi.fn().mockResolvedValue({ - messageId: "m1", - toJid: "jid", - }); - - const res = await runHeartbeatOnce({ - cfg, - reason: "interval", - deps: { - sendWhatsApp, - getQueueSize: () => 0, - nowMs: () => 0, - webAuthExists: async () => true, - hasActiveWebListener: () => true, - }, - }); - - expect(res.status).toBe("ran"); - expect(replySpy).toHaveBeenCalledTimes(1); - const calledCtx = replySpy.mock.calls[0]?.[0] as { Provider?: string; Body?: string }; - expect(calledCtx.Provider).toBe("cron-event"); - expect(calledCtx.Body).toContain("scheduled reminder has been triggered"); - expect(sendWhatsApp).toHaveBeenCalledTimes(1); - } finally { - replySpy.mockRestore(); - } - }); - - it("runs heartbeat when HEARTBEAT.md has actionable content", async () => { - const tmpDir = await createCaseDir("openclaw-hb"); - const storePath = path.join(tmpDir, "sessions.json"); - const workspaceDir = path.join(tmpDir, "workspace"); - const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); - try { - await fs.mkdir(workspaceDir, { recursive: true }); - - // Create HEARTBEAT.md with actionable content + } else if (params.fileState === "actionable") { await fs.writeFile( path.join(workspaceDir, "HEARTBEAT.md"), "# HEARTBEAT.md\n\n- Check server logs\n- Review pending PRs\n", "utf-8", ); - - const cfg: OpenClawConfig = { - agents: { - defaults: { - workspace: workspaceDir, - heartbeat: { every: "5m", target: "whatsapp" }, - }, - }, - channels: { whatsapp: { allowFrom: ["*"] } }, - session: { store: storePath }, - }; - const sessionKey = resolveMainSessionKey(cfg); - - await fs.writeFile( - storePath, - JSON.stringify( - { - [sessionKey]: { - sessionId: "sid", - updatedAt: Date.now(), - lastChannel: "whatsapp", - lastTo: "+1555", - }, - }, - null, - 2, - ), - ); - - replySpy.mockResolvedValue({ text: "Checked logs and PRs" }); - const sendWhatsApp = vi.fn().mockResolvedValue({ - messageId: "m1", - toJid: "jid", - }); - - const res = await runHeartbeatOnce({ - cfg, - deps: { - sendWhatsApp, - getQueueSize: () => 0, - nowMs: () => 0, - webAuthExists: async () => true, - hasActiveWebListener: () => true, - }, - }); - - // Should run and make API call - expect(res.status).toBe("ran"); - expect(replySpy).toHaveBeenCalled(); - expect(sendWhatsApp).toHaveBeenCalledTimes(1); - } finally { - replySpy.mockRestore(); - } - }); - - it("runs heartbeat when HEARTBEAT.md does not exist", async () => { - const tmpDir = await createCaseDir("openclaw-hb"); - const storePath = path.join(tmpDir, "sessions.json"); - const workspaceDir = path.join(tmpDir, "workspace"); - const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); - try { - await fs.mkdir(workspaceDir, { recursive: true }); - // Don't create HEARTBEAT.md - it doesn't exist - - const cfg: OpenClawConfig = { - agents: { - defaults: { - workspace: workspaceDir, - heartbeat: { every: "5m", target: "whatsapp" }, - }, - }, - channels: { whatsapp: { allowFrom: ["*"] } }, - session: { store: storePath }, - }; - const sessionKey = resolveMainSessionKey(cfg); - - await fs.writeFile( - storePath, - JSON.stringify( - { - [sessionKey]: { - sessionId: "sid", - updatedAt: Date.now(), - lastChannel: "whatsapp", - lastTo: "+1555", - }, - }, - null, - 2, - ), - ); - - replySpy.mockResolvedValue({ text: "Checked logs and PRs" }); - const sendWhatsApp = vi.fn().mockResolvedValue({ - messageId: "m1", - toJid: "jid", - }); - - const res = await runHeartbeatOnce({ - cfg, - deps: { - sendWhatsApp, - getQueueSize: () => 0, - nowMs: () => 0, - webAuthExists: async () => true, - hasActiveWebListener: () => true, - }, - }); - - // Missing HEARTBEAT.md should still run so prompt/system instructions can drive work. - expect(res.status).toBe("ran"); - expect(replySpy).toHaveBeenCalled(); - expect(sendWhatsApp).toHaveBeenCalledTimes(1); - } finally { - replySpy.mockRestore(); - } - }); - - it("runs heartbeat when HEARTBEAT.md read fails with a non-ENOENT error", async () => { - const tmpDir = await createCaseDir("openclaw-hb"); - const storePath = path.join(tmpDir, "sessions.json"); - const workspaceDir = path.join(tmpDir, "workspace"); - const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); - try { - await fs.mkdir(workspaceDir, { recursive: true }); - // Simulate a read failure path (readFile on a directory returns EISDIR). + } else if (params.fileState === "read-error") { + // readFile on a directory triggers EISDIR. await fs.mkdir(path.join(workspaceDir, "HEARTBEAT.md"), { recursive: true }); - - const cfg: OpenClawConfig = { - agents: { - defaults: { - workspace: workspaceDir, - heartbeat: { every: "5m", target: "whatsapp" }, - }, - }, - channels: { whatsapp: { allowFrom: ["*"] } }, - session: { store: storePath }, - }; - const sessionKey = resolveMainSessionKey(cfg); - - await fs.writeFile( - storePath, - JSON.stringify( - { - [sessionKey]: { - sessionId: "sid", - updatedAt: Date.now(), - lastChannel: "whatsapp", - lastTo: "+1555", - }, - }, - null, - 2, - ), - ); - - replySpy.mockResolvedValue({ text: "Checked logs and PRs" }); - const sendWhatsApp = vi.fn().mockResolvedValue({ - messageId: "m1", - toJid: "jid", - }); - - const res = await runHeartbeatOnce({ - cfg, - deps: { - sendWhatsApp, - getQueueSize: () => 0, - nowMs: () => 0, - webAuthExists: async () => true, - hasActiveWebListener: () => true, - }, - }); - - // Read errors other than ENOENT should not disable heartbeat runs. - expect(res.status).toBe("ran"); - expect(replySpy).toHaveBeenCalled(); - expect(sendWhatsApp).toHaveBeenCalledTimes(1); - } finally { - replySpy.mockRestore(); } - }); - it("does not skip wake-triggered heartbeat when HEARTBEAT.md does not exist", async () => { - const tmpDir = await createCaseDir("openclaw-hb"); - const storePath = path.join(tmpDir, "sessions.json"); - const workspaceDir = path.join(tmpDir, "workspace"); - const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); - try { - await fs.mkdir(workspaceDir, { recursive: true }); - // Don't create HEARTBEAT.md - - const cfg: OpenClawConfig = { - agents: { - defaults: { - workspace: workspaceDir, - heartbeat: { every: "5m", target: "whatsapp" }, + const cfg: OpenClawConfig = { + agents: { + defaults: { + workspace: workspaceDir, + heartbeat: { every: "5m", target: "whatsapp" }, + }, + }, + channels: { whatsapp: { allowFrom: ["*"] } }, + session: { store: storePath }, + }; + const sessionKey = resolveMainSessionKey(cfg); + await fs.writeFile( + storePath, + JSON.stringify( + { + [sessionKey]: { + sessionId: "sid", + updatedAt: Date.now(), + lastChannel: "whatsapp", + lastTo: "+1555", }, }, - channels: { whatsapp: { allowFrom: ["*"] } }, - session: { store: storePath }, - }; - const sessionKey = resolveMainSessionKey(cfg); - - await fs.writeFile( - storePath, - JSON.stringify( - { - [sessionKey]: { - sessionId: "sid", - updatedAt: Date.now(), - lastChannel: "whatsapp", - lastTo: "+1555", - }, - }, - null, - 2, - ), - ); - - replySpy.mockResolvedValue({ text: "wake event processed" }); - const sendWhatsApp = vi.fn().mockResolvedValue({ - messageId: "m1", - toJid: "jid", - }); - - const res = await runHeartbeatOnce({ - cfg, - reason: "wake", - deps: { - sendWhatsApp, - getQueueSize: () => 0, - nowMs: () => 0, - webAuthExists: async () => true, - hasActiveWebListener: () => true, - }, - }); - - // Wake events should still run even without HEARTBEAT.md - expect(res.status).toBe("ran"); - expect(replySpy).toHaveBeenCalled(); - expect(sendWhatsApp).toHaveBeenCalledTimes(1); - } finally { - replySpy.mockRestore(); - } - }); - - it("does not skip interval heartbeat when tagged cron events are queued and HEARTBEAT.md is missing", async () => { - const tmpDir = await createCaseDir("openclaw-hb"); - const storePath = path.join(tmpDir, "sessions.json"); - const workspaceDir = path.join(tmpDir, "workspace"); - const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); - try { - await fs.mkdir(workspaceDir, { recursive: true }); - // Don't create HEARTBEAT.md - - const cfg: OpenClawConfig = { - agents: { - defaults: { - workspace: workspaceDir, - heartbeat: { every: "5m", target: "whatsapp" }, - }, - }, - channels: { whatsapp: { allowFrom: ["*"] } }, - session: { store: storePath }, - }; - const sessionKey = resolveMainSessionKey(cfg); - - await fs.writeFile( - storePath, - JSON.stringify( - { - [sessionKey]: { - sessionId: "sid", - updatedAt: Date.now(), - lastChannel: "whatsapp", - lastTo: "+1555", - }, - }, - null, - 2, - ), - ); - + null, + 2, + ), + ); + if (params.queueCronEvent) { enqueueSystemEvent("Cron: QMD maintenance completed", { sessionKey, contextKey: "cron:qmd-maintenance", }); + } - replySpy.mockResolvedValue({ text: "Relay this cron update now" }); - const sendWhatsApp = vi.fn().mockResolvedValue({ - messageId: "m1", - toJid: "jid", - }); + const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); + replySpy.mockResolvedValue({ text: params.replyText ?? "Checked logs and PRs" }); + const sendWhatsApp = vi.fn().mockResolvedValue({ messageId: "m1", toJid: "jid" }); + const res = await runHeartbeatOnce({ + cfg, + reason: params.reason, + deps: createHeartbeatDeps(sendWhatsApp), + }); + return { res, replySpy, sendWhatsApp }; + } - const res = await runHeartbeatOnce({ - cfg, + it("applies HEARTBEAT.md gating rules across file states and triggers", async () => { + const cases: Array<{ + name: string; + fileState: HeartbeatFileState; + reason?: "interval" | "wake"; + queueCronEvent?: boolean; + expectedStatus: "ran" | "skipped"; + expectedSkipReason?: "empty-heartbeat-file"; + expectedSendCalls: number; + expectedReplyCalls: number; + expectCronContext?: boolean; + replyText?: string; + }> = [ + { + name: "empty file + interval skips", + fileState: "empty", + expectedStatus: "skipped", + expectedSkipReason: "empty-heartbeat-file", + expectedSendCalls: 0, + expectedReplyCalls: 0, + }, + { + name: "empty file + wake runs", + fileState: "empty", + reason: "wake", + expectedStatus: "ran", + expectedSendCalls: 1, + expectedReplyCalls: 1, + replyText: "wake event processed", + }, + { + name: "empty file + queued cron interval runs", + fileState: "empty", reason: "interval", - deps: { - sendWhatsApp, - getQueueSize: () => 0, - nowMs: () => 0, - webAuthExists: async () => true, - hasActiveWebListener: () => true, - }, - }); + queueCronEvent: true, + expectedStatus: "ran", + expectedSendCalls: 1, + expectedReplyCalls: 1, + expectCronContext: true, + replyText: "Relay this cron update now", + }, + { + name: "actionable file runs", + fileState: "actionable", + expectedStatus: "ran", + expectedSendCalls: 1, + expectedReplyCalls: 1, + }, + { + name: "missing file runs", + fileState: "missing", + expectedStatus: "ran", + expectedSendCalls: 1, + expectedReplyCalls: 1, + }, + { + name: "read error runs", + fileState: "read-error", + expectedStatus: "ran", + expectedSendCalls: 1, + expectedReplyCalls: 1, + }, + { + name: "missing file + wake runs", + fileState: "missing", + reason: "wake", + expectedStatus: "ran", + expectedSendCalls: 1, + expectedReplyCalls: 1, + replyText: "wake event processed", + }, + { + name: "missing file + queued cron interval runs", + fileState: "missing", + reason: "interval", + queueCronEvent: true, + expectedStatus: "ran", + expectedSendCalls: 1, + expectedReplyCalls: 1, + expectCronContext: true, + replyText: "Relay this cron update now", + }, + ]; - expect(res.status).toBe("ran"); - expect(replySpy).toHaveBeenCalledTimes(1); - const calledCtx = replySpy.mock.calls[0]?.[0] as { Provider?: string; Body?: string }; - expect(calledCtx.Provider).toBe("cron-event"); - expect(calledCtx.Body).toContain("scheduled reminder has been triggered"); - expect(sendWhatsApp).toHaveBeenCalledTimes(1); - } finally { - replySpy.mockRestore(); + for (const testCase of cases) { + const { res, replySpy, sendWhatsApp } = await runHeartbeatFileScenario(testCase); + try { + expect(res.status, testCase.name).toBe(testCase.expectedStatus); + if (res.status === "skipped") { + expect(res.reason, testCase.name).toBe(testCase.expectedSkipReason); + } + expect(replySpy, testCase.name).toHaveBeenCalledTimes(testCase.expectedReplyCalls); + expect(sendWhatsApp, testCase.name).toHaveBeenCalledTimes(testCase.expectedSendCalls); + if (testCase.expectCronContext) { + const calledCtx = replySpy.mock.calls[0]?.[0] as { Provider?: string; Body?: string }; + expect(calledCtx.Provider, testCase.name).toBe("cron-event"); + expect(calledCtx.Body, testCase.name).toContain("scheduled reminder has been triggered"); + } + } finally { + replySpy.mockRestore(); + } } }); }); diff --git a/src/infra/outbound/outbound.test.ts b/src/infra/outbound/outbound.test.ts index 6428e73551d..18394d752a1 100644 --- a/src/infra/outbound/outbound.test.ts +++ b/src/infra/outbound/outbound.test.ts @@ -188,6 +188,12 @@ describe("delivery-queue", () => { await enqueueDelivery({ channel: "whatsapp", to: "+1", payloads: [{ text: "a" }] }, tmpDir); await enqueueDelivery({ channel: "telegram", to: "2", payloads: [{ text: "b" }] }, tmpDir); }; + const setEntryRetryCount = (id: string, retryCount: number) => { + const filePath = path.join(tmpDir, "delivery-queue", `${id}.json`); + const entry = JSON.parse(fs.readFileSync(filePath, "utf-8")); + entry.retryCount = retryCount; + fs.writeFileSync(filePath, JSON.stringify(entry), "utf-8"); + }; const runRecovery = async ({ deliver, log = createLog(), @@ -232,10 +238,7 @@ describe("delivery-queue", () => { { channel: "whatsapp", to: "+1", payloads: [{ text: "a" }] }, tmpDir, ); - const filePath = path.join(tmpDir, "delivery-queue", `${id}.json`); - const entry = JSON.parse(fs.readFileSync(filePath, "utf-8")); - entry.retryCount = MAX_RETRIES; - fs.writeFileSync(filePath, JSON.stringify(entry), "utf-8"); + setEntryRetryCount(id, MAX_RETRIES); const deliver = vi.fn(); const { result } = await runRecovery({ deliver }); @@ -336,10 +339,7 @@ describe("delivery-queue", () => { { channel: "whatsapp", to: "+1", payloads: [{ text: "a" }] }, tmpDir, ); - const filePath = path.join(tmpDir, "delivery-queue", `${id}.json`); - const entry = JSON.parse(fs.readFileSync(filePath, "utf-8")); - entry.retryCount = 3; - fs.writeFileSync(filePath, JSON.stringify(entry), "utf-8"); + setEntryRetryCount(id, 3); const deliver = vi.fn().mockResolvedValue([]); const delay = vi.fn(async () => {}); @@ -422,30 +422,15 @@ describe("DirectoryCache", () => { }); describe("buildOutboundResultEnvelope", () => { - it("flattens delivery-only payloads by default", () => { - const delivery: OutboundDeliveryJson = { + it("formats envelope variants", () => { + const whatsappDelivery: OutboundDeliveryJson = { channel: "whatsapp", via: "gateway", to: "+1", messageId: "m1", mediaUrl: null, }; - expect(buildOutboundResultEnvelope({ delivery })).toEqual(delivery); - }); - - it("keeps payloads and meta in the envelope", () => { - const envelope = buildOutboundResultEnvelope({ - payloads: [{ text: "hi", mediaUrl: null, mediaUrls: undefined }], - meta: { foo: "bar" }, - }); - expect(envelope).toEqual({ - payloads: [{ text: "hi", mediaUrl: null, mediaUrls: undefined }], - meta: { foo: "bar" }, - }); - }); - - it("includes delivery when payloads are present", () => { - const delivery: OutboundDeliveryJson = { + const telegramDelivery: OutboundDeliveryJson = { channel: "telegram", via: "direct", to: "123", @@ -453,20 +438,7 @@ describe("buildOutboundResultEnvelope", () => { mediaUrl: null, chatId: "c1", }; - const envelope = buildOutboundResultEnvelope({ - payloads: [], - delivery, - meta: { ok: true }, - }); - expect(envelope).toEqual({ - payloads: [], - meta: { ok: true }, - delivery, - }); - }); - - it("can keep delivery wrapped when requested", () => { - const delivery: OutboundDeliveryJson = { + const discordDelivery: OutboundDeliveryJson = { channel: "discord", via: "gateway", to: "channel:C1", @@ -474,11 +446,41 @@ describe("buildOutboundResultEnvelope", () => { mediaUrl: null, channelId: "C1", }; - const envelope = buildOutboundResultEnvelope({ - delivery, - flattenDelivery: false, - }); - expect(envelope).toEqual({ delivery }); + const cases = [ + { + name: "flatten delivery by default", + input: { delivery: whatsappDelivery }, + expected: whatsappDelivery, + }, + { + name: "keep payloads + meta", + input: { + payloads: [{ text: "hi", mediaUrl: null, mediaUrls: undefined }], + meta: { foo: "bar" }, + }, + expected: { + payloads: [{ text: "hi", mediaUrl: null, mediaUrls: undefined }], + meta: { foo: "bar" }, + }, + }, + { + name: "include delivery when payloads exist", + input: { payloads: [], delivery: telegramDelivery, meta: { ok: true } }, + expected: { + payloads: [], + meta: { ok: true }, + delivery: telegramDelivery, + }, + }, + { + name: "keep wrapped delivery when flatten disabled", + input: { delivery: discordDelivery, flattenDelivery: false }, + expected: { delivery: discordDelivery }, + }, + ] as const; + for (const testCase of cases) { + expect(buildOutboundResultEnvelope(testCase.input), testCase.name).toEqual(testCase.expected); + } }); }); @@ -668,50 +670,9 @@ describe("outbound policy", () => { describe("resolveOutboundSessionRoute", () => { const baseConfig = {} as OpenClawConfig; - it("builds Slack thread session keys", async () => { - const route = await resolveOutboundSessionRoute({ - cfg: baseConfig, - channel: "slack", - agentId: "main", - target: "channel:C123", - replyToId: "456", - }); - - expect(route?.sessionKey).toBe("agent:main:slack:channel:c123:thread:456"); - expect(route?.from).toBe("slack:channel:C123"); - expect(route?.to).toBe("channel:C123"); - expect(route?.threadId).toBe("456"); - }); - - it("uses Telegram topic ids in group session keys", async () => { - const route = await resolveOutboundSessionRoute({ - cfg: baseConfig, - channel: "telegram", - agentId: "main", - target: "-100123456:topic:42", - }); - - expect(route?.sessionKey).toBe("agent:main:telegram:group:-100123456:topic:42"); - expect(route?.from).toBe("telegram:group:-100123456:topic:42"); - expect(route?.to).toBe("telegram:-100123456"); - expect(route?.threadId).toBe(42); - }); - - it("treats Telegram usernames as DMs when unresolved", async () => { - const cfg = { session: { dmScope: "per-channel-peer" } } as OpenClawConfig; - const route = await resolveOutboundSessionRoute({ - cfg, - channel: "telegram", - agentId: "main", - target: "@alice", - }); - - expect(route?.sessionKey).toBe("agent:main:telegram:direct:@alice"); - expect(route?.chatType).toBe("direct"); - }); - - it("honors dmScope identity links", async () => { - const cfg = { + it("resolves provider-specific session routes", async () => { + const perChannelPeerCfg = { session: { dmScope: "per-channel-peer" } } as OpenClawConfig; + const identityLinksCfg = { session: { dmScope: "per-peer", identityLinks: { @@ -719,44 +680,7 @@ describe("resolveOutboundSessionRoute", () => { }, }, } as OpenClawConfig; - - const route = await resolveOutboundSessionRoute({ - cfg, - channel: "discord", - agentId: "main", - target: "user:123", - }); - - expect(route?.sessionKey).toBe("agent:main:direct:alice"); - }); - - it("strips chat_* prefixes for BlueBubbles group session keys", async () => { - const route = await resolveOutboundSessionRoute({ - cfg: baseConfig, - channel: "bluebubbles", - agentId: "main", - target: "chat_guid:ABC123", - }); - - expect(route?.sessionKey).toBe("agent:main:bluebubbles:group:abc123"); - expect(route?.from).toBe("group:ABC123"); - }); - - it("treats Zalo Personal DM targets as direct sessions", async () => { - const cfg = { session: { dmScope: "per-channel-peer" } } as OpenClawConfig; - const route = await resolveOutboundSessionRoute({ - cfg, - channel: "zalouser", - agentId: "main", - target: "123456", - }); - - expect(route?.sessionKey).toBe("agent:main:zalouser:direct:123456"); - expect(route?.chatType).toBe("direct"); - }); - - it("uses group session keys for Slack mpim allowlist entries", async () => { - const cfg = { + const slackMpimCfg = { channels: { slack: { dm: { @@ -765,16 +689,118 @@ describe("resolveOutboundSessionRoute", () => { }, }, } as OpenClawConfig; + const cases: Array<{ + name: string; + cfg: OpenClawConfig; + channel: string; + target: string; + replyToId?: string; + expected: { + sessionKey: string; + from?: string; + to?: string; + threadId?: string | number; + chatType?: "direct" | "group"; + }; + }> = [ + { + name: "Slack thread", + cfg: baseConfig, + channel: "slack", + target: "channel:C123", + replyToId: "456", + expected: { + sessionKey: "agent:main:slack:channel:c123:thread:456", + from: "slack:channel:C123", + to: "channel:C123", + threadId: "456", + }, + }, + { + name: "Telegram topic group", + cfg: baseConfig, + channel: "telegram", + target: "-100123456:topic:42", + expected: { + sessionKey: "agent:main:telegram:group:-100123456:topic:42", + from: "telegram:group:-100123456:topic:42", + to: "telegram:-100123456", + threadId: 42, + }, + }, + { + name: "Telegram unresolved username DM", + cfg: perChannelPeerCfg, + channel: "telegram", + target: "@alice", + expected: { + sessionKey: "agent:main:telegram:direct:@alice", + chatType: "direct", + }, + }, + { + name: "identity-links per-peer", + cfg: identityLinksCfg, + channel: "discord", + target: "user:123", + expected: { + sessionKey: "agent:main:direct:alice", + }, + }, + { + name: "BlueBubbles chat_* prefix stripping", + cfg: baseConfig, + channel: "bluebubbles", + target: "chat_guid:ABC123", + expected: { + sessionKey: "agent:main:bluebubbles:group:abc123", + from: "group:ABC123", + }, + }, + { + name: "Zalo Personal DM target", + cfg: perChannelPeerCfg, + channel: "zalouser", + target: "123456", + expected: { + sessionKey: "agent:main:zalouser:direct:123456", + chatType: "direct", + }, + }, + { + name: "Slack mpim allowlist -> group key", + cfg: slackMpimCfg, + channel: "slack", + target: "channel:G123", + expected: { + sessionKey: "agent:main:slack:group:g123", + from: "slack:group:G123", + }, + }, + ]; - const route = await resolveOutboundSessionRoute({ - cfg, - channel: "slack", - agentId: "main", - target: "channel:G123", - }); - - expect(route?.sessionKey).toBe("agent:main:slack:group:g123"); - expect(route?.from).toBe("slack:group:G123"); + for (const testCase of cases) { + const route = await resolveOutboundSessionRoute({ + cfg: testCase.cfg, + channel: testCase.channel, + agentId: "main", + target: testCase.target, + replyToId: testCase.replyToId, + }); + expect(route?.sessionKey, testCase.name).toBe(testCase.expected.sessionKey); + if (testCase.expected.from !== undefined) { + expect(route?.from, testCase.name).toBe(testCase.expected.from); + } + if (testCase.expected.to !== undefined) { + expect(route?.to, testCase.name).toBe(testCase.expected.to); + } + if (testCase.expected.threadId !== undefined) { + expect(route?.threadId, testCase.name).toBe(testCase.expected.threadId); + } + if (testCase.expected.chatType !== undefined) { + expect(route?.chatType, testCase.name).toBe(testCase.expected.chatType); + } + } }); });