From 34ea33f0574f9eea6253a0c2fa76df3c023a8101 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 17:11:34 +0000 Subject: [PATCH] refactor: dedupe core config and runtime helpers --- .../auth-choice.apply-helpers.test.ts | 80 ++++--- ...doctor-config-flow.include-warning.test.ts | 14 +- .../doctor-config-flow.safe-bins.test.ts | 25 +-- src/commands/doctor-config-flow.test-utils.ts | 26 +++ src/commands/doctor-config-flow.test.ts | 85 +++---- src/config/config.compaction-settings.test.ts | 143 +++++------- src/config/config.discord.test.ts | 87 ++++---- ...etection.accepts-imessage-dmpolicy.test.ts | 209 +++++++----------- ...ig.multi-agent-agentdir-validation.test.ts | 46 ++-- .../config.nix-integration-u3-u5-u9.test.ts | 92 ++++---- src/config/env-preserve-io.test.ts | 86 +++---- src/config/redact-snapshot.test.ts | 43 ++-- .../store.pruning.integration.test.ts | 19 +- src/config/test-helpers.ts | 19 ++ src/config/zod-schema.agent-runtime.ts | 20 +- src/config/zod-schema.core.ts | 26 +-- src/cron/isolated-agent/run.ts | 19 +- src/cron/service/timer.ts | 66 +++--- src/daemon/service-env.ts | 23 +- src/infra/device-pairing.test.ts | 73 +++--- src/infra/gateway-lock.test.ts | 106 ++++----- .../heartbeat-runner.ghost-reminder.test.ts | 106 ++++----- src/infra/outbound/deliver.test.ts | 53 ++--- src/infra/path-safety.ts | 13 +- src/infra/update-runner.test.ts | 65 +++--- src/process/supervisor/adapters/child.ts | 12 +- src/process/supervisor/adapters/pty.ts | 12 +- src/process/supervisor/types.ts | 10 + src/routing/session-key.ts | 16 +- 29 files changed, 720 insertions(+), 874 deletions(-) create mode 100644 src/commands/doctor-config-flow.test-utils.ts diff --git a/src/commands/auth-choice.apply-helpers.test.ts b/src/commands/auth-choice.apply-helpers.test.ts index 0318a3a417a..c122fe197ca 100644 --- a/src/commands/auth-choice.apply-helpers.test.ts +++ b/src/commands/auth-choice.apply-helpers.test.ts @@ -35,6 +35,36 @@ function createPrompter(params?: { } as unknown as WizardPrompter; } +function createPromptSpies(params?: { confirmResult?: boolean; textResult?: string }) { + const confirm = vi.fn(async () => params?.confirmResult ?? true); + const note = vi.fn(async () => undefined); + const text = vi.fn(async () => params?.textResult ?? "prompt-key"); + return { confirm, note, text }; +} + +async function runEnsureMinimaxApiKeyFlow(params: { confirmResult: boolean; textResult: string }) { + process.env.MINIMAX_API_KEY = "env-key"; + delete process.env.MINIMAX_OAUTH_TOKEN; + + const { confirm, text } = createPromptSpies({ + confirmResult: params.confirmResult, + textResult: params.textResult, + }); + const setCredential = vi.fn(async () => undefined); + + const result = await ensureApiKeyFromEnvOrPrompt({ + provider: "minimax", + envLabel: "MINIMAX_API_KEY", + promptMessage: "Enter key", + normalize: (value) => value.trim(), + validate: () => undefined, + prompter: createPrompter({ confirm, text }), + setCredential, + }); + + return { result, setCredential, confirm, text }; +} + afterEach(() => { restoreMinimaxEnv(); vi.restoreAllMocks(); @@ -96,21 +126,9 @@ describe("maybeApplyApiKeyFromOption", () => { describe("ensureApiKeyFromEnvOrPrompt", () => { it("uses env credential when user confirms", async () => { - process.env.MINIMAX_API_KEY = "env-key"; - delete process.env.MINIMAX_OAUTH_TOKEN; - - const confirm = vi.fn(async () => true); - const text = vi.fn(async () => "prompt-key"); - const setCredential = vi.fn(async () => undefined); - - const result = await ensureApiKeyFromEnvOrPrompt({ - provider: "minimax", - envLabel: "MINIMAX_API_KEY", - promptMessage: "Enter key", - normalize: (value) => value.trim(), - validate: () => undefined, - prompter: createPrompter({ confirm, text }), - setCredential, + const { result, setCredential, text } = await runEnsureMinimaxApiKeyFlow({ + confirmResult: true, + textResult: "prompt-key", }); expect(result).toBe("env-key"); @@ -119,21 +137,9 @@ describe("ensureApiKeyFromEnvOrPrompt", () => { }); it("falls back to prompt when env is declined", async () => { - process.env.MINIMAX_API_KEY = "env-key"; - delete process.env.MINIMAX_OAUTH_TOKEN; - - const confirm = vi.fn(async () => false); - const text = vi.fn(async () => " prompted-key "); - const setCredential = vi.fn(async () => undefined); - - const result = await ensureApiKeyFromEnvOrPrompt({ - provider: "minimax", - envLabel: "MINIMAX_API_KEY", - promptMessage: "Enter key", - normalize: (value) => value.trim(), - validate: () => undefined, - prompter: createPrompter({ confirm, text }), - setCredential, + const { result, setCredential, text } = await runEnsureMinimaxApiKeyFlow({ + confirmResult: false, + textResult: " prompted-key ", }); expect(result).toBe("prompted-key"); @@ -148,9 +154,10 @@ describe("ensureApiKeyFromEnvOrPrompt", () => { describe("ensureApiKeyFromOptionEnvOrPrompt", () => { it("uses opts token and skips note/env/prompt", async () => { - const confirm = vi.fn(async () => true); - const note = vi.fn(async () => undefined); - const text = vi.fn(async () => "prompt-key"); + const { confirm, note, text } = createPromptSpies({ + confirmResult: true, + textResult: "prompt-key", + }); const setCredential = vi.fn(async () => undefined); const result = await ensureApiKeyFromOptionEnvOrPrompt({ @@ -179,9 +186,10 @@ describe("ensureApiKeyFromOptionEnvOrPrompt", () => { delete process.env.MINIMAX_OAUTH_TOKEN; process.env.MINIMAX_API_KEY = "env-key"; - const confirm = vi.fn(async () => true); - const note = vi.fn(async () => undefined); - const text = vi.fn(async () => "prompt-key"); + const { confirm, note, text } = createPromptSpies({ + confirmResult: true, + textResult: "prompt-key", + }); const setCredential = vi.fn(async () => undefined); const result = await ensureApiKeyFromOptionEnvOrPrompt({ diff --git a/src/commands/doctor-config-flow.include-warning.test.ts b/src/commands/doctor-config-flow.include-warning.test.ts index 504a6d84b89..79ed3148406 100644 --- a/src/commands/doctor-config-flow.include-warning.test.ts +++ b/src/commands/doctor-config-flow.include-warning.test.ts @@ -1,7 +1,5 @@ -import fs from "node:fs/promises"; -import path from "node:path"; import { describe, expect, it, vi } from "vitest"; -import { withTempHome } from "../../test/helpers/temp-home.js"; +import { withTempHomeConfig } from "../config/test-helpers.js"; const { noteSpy } = vi.hoisted(() => ({ noteSpy: vi.fn(), @@ -15,15 +13,7 @@ import { loadAndMaybeMigrateDoctorConfig } from "./doctor-config-flow.js"; describe("doctor include warning", () => { it("surfaces include confinement hint for escaped include paths", async () => { - await withTempHome(async (home) => { - const configDir = path.join(home, ".openclaw"); - await fs.mkdir(configDir, { recursive: true }); - await fs.writeFile( - path.join(configDir, "openclaw.json"), - JSON.stringify({ $include: "/etc/passwd" }, null, 2), - "utf-8", - ); - + await withTempHomeConfig({ $include: "/etc/passwd" }, async () => { await loadAndMaybeMigrateDoctorConfig({ options: { nonInteractive: true }, confirm: async () => false, diff --git a/src/commands/doctor-config-flow.safe-bins.test.ts b/src/commands/doctor-config-flow.safe-bins.test.ts index 5d1651ce591..3d7a646a8dd 100644 --- a/src/commands/doctor-config-flow.safe-bins.test.ts +++ b/src/commands/doctor-config-flow.safe-bins.test.ts @@ -1,7 +1,5 @@ -import fs from "node:fs/promises"; -import path from "node:path"; import { beforeEach, describe, expect, it, vi } from "vitest"; -import { withTempHome } from "../../test/helpers/temp-home.js"; +import { runDoctorConfigWithInput } from "./doctor-config-flow.test-utils.js"; const { noteSpy } = vi.hoisted(() => ({ noteSpy: vi.fn(), @@ -13,25 +11,6 @@ vi.mock("../terminal/note.js", () => ({ import { loadAndMaybeMigrateDoctorConfig } from "./doctor-config-flow.js"; -async function runDoctorConfigWithInput(params: { - config: Record; - repair?: boolean; -}) { - return withTempHome(async (home) => { - const configDir = path.join(home, ".openclaw"); - await fs.mkdir(configDir, { recursive: true }); - await fs.writeFile( - path.join(configDir, "openclaw.json"), - JSON.stringify(params.config, null, 2), - "utf-8", - ); - return loadAndMaybeMigrateDoctorConfig({ - options: { nonInteractive: true, repair: params.repair }, - confirm: async () => false, - }); - }); -} - describe("doctor config flow safe bins", () => { beforeEach(() => { noteSpy.mockClear(); @@ -59,6 +38,7 @@ describe("doctor config flow safe bins", () => { ], }, }, + run: loadAndMaybeMigrateDoctorConfig, }); const cfg = result.cfg as { @@ -94,6 +74,7 @@ describe("doctor config flow safe bins", () => { }, }, }, + run: loadAndMaybeMigrateDoctorConfig, }); expect(noteSpy).toHaveBeenCalledWith( diff --git a/src/commands/doctor-config-flow.test-utils.ts b/src/commands/doctor-config-flow.test-utils.ts new file mode 100644 index 00000000000..ef363620e68 --- /dev/null +++ b/src/commands/doctor-config-flow.test-utils.ts @@ -0,0 +1,26 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { withTempHome } from "../../test/helpers/temp-home.js"; + +export async function runDoctorConfigWithInput(params: { + config: Record; + repair?: boolean; + run: (args: { + options: { nonInteractive: boolean; repair?: boolean }; + confirm: () => Promise; + }) => Promise; +}) { + return withTempHome(async (home) => { + const configDir = path.join(home, ".openclaw"); + await fs.mkdir(configDir, { recursive: true }); + await fs.writeFile( + path.join(configDir, "openclaw.json"), + JSON.stringify(params.config, null, 2), + "utf-8", + ); + return params.run({ + options: { nonInteractive: true, repair: params.repair }, + confirm: async () => false, + }); + }); +} diff --git a/src/commands/doctor-config-flow.test.ts b/src/commands/doctor-config-flow.test.ts index f1d8bf307a4..001bf5f7031 100644 --- a/src/commands/doctor-config-flow.test.ts +++ b/src/commands/doctor-config-flow.test.ts @@ -3,25 +3,7 @@ import path from "node:path"; import { describe, expect, it, vi } from "vitest"; import { withTempHome } from "../../test/helpers/temp-home.js"; import { loadAndMaybeMigrateDoctorConfig } from "./doctor-config-flow.js"; - -async function runDoctorConfigWithInput(params: { - config: Record; - repair?: boolean; -}) { - return withTempHome(async (home) => { - const configDir = path.join(home, ".openclaw"); - await fs.mkdir(configDir, { recursive: true }); - await fs.writeFile( - path.join(configDir, "openclaw.json"), - JSON.stringify(params.config, null, 2), - "utf-8", - ); - return loadAndMaybeMigrateDoctorConfig({ - options: { nonInteractive: true, repair: params.repair }, - confirm: async () => false, - }); - }); -} +import { runDoctorConfigWithInput } from "./doctor-config-flow.test-utils.js"; function expectGoogleChatDmAllowFromRepaired(cfg: unknown) { const typed = cfg as { @@ -36,6 +18,27 @@ function expectGoogleChatDmAllowFromRepaired(cfg: unknown) { expect(typed.channels.googlechat.allowFrom).toBeUndefined(); } +type DiscordGuildRule = { + users: string[]; + roles: string[]; + channels: Record; +}; + +type DiscordAccountRule = { + allowFrom: string[]; + dm: { allowFrom: string[]; groupChannels: string[] }; + execApprovals: { approvers: string[] }; + guilds: Record; +}; + +type RepairedDiscordPolicy = { + allowFrom: string[]; + dm: { allowFrom: string[]; groupChannels: string[] }; + execApprovals: { approvers: string[] }; + guilds: Record; + accounts: Record; +}; + describe("doctor config flow", () => { it("preserves invalid config for doctor repairs", async () => { const result = await runDoctorConfigWithInput({ @@ -43,6 +46,7 @@ describe("doctor config flow", () => { gateway: { auth: { mode: "token", token: 123 } }, agents: { list: [{ id: "pi" }] }, }, + run: loadAndMaybeMigrateDoctorConfig, }); expect((result.cfg as Record).gateway).toEqual({ @@ -58,6 +62,7 @@ describe("doctor config flow", () => { gateway: { auth: { mode: "token", token: "ok", extra: true } }, agents: { list: [{ id: "pi" }] }, }, + run: loadAndMaybeMigrateDoctorConfig, }); const cfg = result.cfg as Record; @@ -88,6 +93,7 @@ describe("doctor config flow", () => { }, }, }, + run: loadAndMaybeMigrateDoctorConfig, }); const cfg = result.cfg as { @@ -145,6 +151,7 @@ describe("doctor config flow", () => { }, }, }, + run: loadAndMaybeMigrateDoctorConfig, }); const cfg = result.cfg as unknown as { @@ -223,37 +230,7 @@ describe("doctor config flow", () => { }); const cfg = result.cfg as unknown as { - channels: { - discord: { - allowFrom: string[]; - dm: { allowFrom: string[]; groupChannels: string[] }; - execApprovals: { approvers: string[] }; - guilds: Record< - string, - { - users: string[]; - roles: string[]; - channels: Record; - } - >; - accounts: Record< - string, - { - allowFrom: string[]; - dm: { allowFrom: string[]; groupChannels: string[] }; - execApprovals: { approvers: string[] }; - guilds: Record< - string, - { - users: string[]; - roles: string[]; - channels: Record; - } - >; - } - >; - }; - }; + channels: { discord: RepairedDiscordPolicy }; }; expect(cfg.channels.discord.allowFrom).toEqual(["123"]); @@ -291,6 +268,7 @@ describe("doctor config flow", () => { }, }, }, + run: loadAndMaybeMigrateDoctorConfig, }); const cfg = result.cfg as unknown as { @@ -313,6 +291,7 @@ describe("doctor config flow", () => { }, }, }, + run: loadAndMaybeMigrateDoctorConfig, }); const cfg = result.cfg as unknown as { @@ -334,6 +313,7 @@ describe("doctor config flow", () => { }, }, }, + run: loadAndMaybeMigrateDoctorConfig, }); const cfg = result.cfg as unknown as { @@ -362,6 +342,7 @@ describe("doctor config flow", () => { }, }, }, + run: loadAndMaybeMigrateDoctorConfig, }); const cfg = result.cfg as unknown as { @@ -386,6 +367,7 @@ describe("doctor config flow", () => { }, }, }, + run: loadAndMaybeMigrateDoctorConfig, }); const cfg = result.cfg as unknown as { @@ -408,6 +390,7 @@ describe("doctor config flow", () => { }, }, }, + run: loadAndMaybeMigrateDoctorConfig, }); expectGoogleChatDmAllowFromRepaired(result.cfg); @@ -429,6 +412,7 @@ describe("doctor config flow", () => { }, }, }, + run: loadAndMaybeMigrateDoctorConfig, }); const cfg = result.cfg as unknown as { @@ -464,6 +448,7 @@ describe("doctor config flow", () => { }, }, }, + run: loadAndMaybeMigrateDoctorConfig, }); expectGoogleChatDmAllowFromRepaired(result.cfg); diff --git a/src/config/config.compaction-settings.test.ts b/src/config/config.compaction-settings.test.ts index 289748ccc11..2503b4dbef5 100644 --- a/src/config/config.compaction-settings.test.ts +++ b/src/config/config.compaction-settings.test.ts @@ -1,107 +1,80 @@ -import fs from "node:fs/promises"; -import path from "node:path"; import { describe, expect, it } from "vitest"; import { loadConfig } from "./config.js"; -import { withTempHome } from "./test-helpers.js"; +import { withTempHomeConfig } from "./test-helpers.js"; describe("config compaction settings", () => { it("preserves memory flush config values", async () => { - await withTempHome(async (home) => { - const configDir = path.join(home, ".openclaw"); - await fs.mkdir(configDir, { recursive: true }); - await fs.writeFile( - path.join(configDir, "openclaw.json"), - JSON.stringify( - { - agents: { - defaults: { - compaction: { - mode: "safeguard", - reserveTokensFloor: 12_345, - memoryFlush: { - enabled: false, - softThresholdTokens: 1234, - prompt: "Write notes.", - systemPrompt: "Flush memory now.", - }, - }, + await withTempHomeConfig( + { + agents: { + defaults: { + compaction: { + mode: "safeguard", + reserveTokensFloor: 12_345, + memoryFlush: { + enabled: false, + softThresholdTokens: 1234, + prompt: "Write notes.", + systemPrompt: "Flush memory now.", }, }, }, - null, - 2, - ), - "utf-8", - ); + }, + }, + async () => { + const cfg = loadConfig(); - const cfg = loadConfig(); - - expect(cfg.agents?.defaults?.compaction?.reserveTokensFloor).toBe(12_345); - expect(cfg.agents?.defaults?.compaction?.mode).toBe("safeguard"); - expect(cfg.agents?.defaults?.compaction?.reserveTokens).toBeUndefined(); - expect(cfg.agents?.defaults?.compaction?.keepRecentTokens).toBeUndefined(); - expect(cfg.agents?.defaults?.compaction?.memoryFlush?.enabled).toBe(false); - expect(cfg.agents?.defaults?.compaction?.memoryFlush?.softThresholdTokens).toBe(1234); - expect(cfg.agents?.defaults?.compaction?.memoryFlush?.prompt).toBe("Write notes."); - expect(cfg.agents?.defaults?.compaction?.memoryFlush?.systemPrompt).toBe("Flush memory now."); - }); + expect(cfg.agents?.defaults?.compaction?.reserveTokensFloor).toBe(12_345); + expect(cfg.agents?.defaults?.compaction?.mode).toBe("safeguard"); + expect(cfg.agents?.defaults?.compaction?.reserveTokens).toBeUndefined(); + expect(cfg.agents?.defaults?.compaction?.keepRecentTokens).toBeUndefined(); + expect(cfg.agents?.defaults?.compaction?.memoryFlush?.enabled).toBe(false); + expect(cfg.agents?.defaults?.compaction?.memoryFlush?.softThresholdTokens).toBe(1234); + expect(cfg.agents?.defaults?.compaction?.memoryFlush?.prompt).toBe("Write notes."); + expect(cfg.agents?.defaults?.compaction?.memoryFlush?.systemPrompt).toBe( + "Flush memory now.", + ); + }, + ); }); it("preserves pi compaction override values", async () => { - await withTempHome(async (home) => { - const configDir = path.join(home, ".openclaw"); - await fs.mkdir(configDir, { recursive: true }); - await fs.writeFile( - path.join(configDir, "openclaw.json"), - JSON.stringify( - { - agents: { - defaults: { - compaction: { - reserveTokens: 15_000, - keepRecentTokens: 12_000, - }, - }, + await withTempHomeConfig( + { + agents: { + defaults: { + compaction: { + reserveTokens: 15_000, + keepRecentTokens: 12_000, }, }, - null, - 2, - ), - "utf-8", - ); - - const cfg = loadConfig(); - expect(cfg.agents?.defaults?.compaction?.reserveTokens).toBe(15_000); - expect(cfg.agents?.defaults?.compaction?.keepRecentTokens).toBe(12_000); - }); + }, + }, + async () => { + const cfg = loadConfig(); + expect(cfg.agents?.defaults?.compaction?.reserveTokens).toBe(15_000); + expect(cfg.agents?.defaults?.compaction?.keepRecentTokens).toBe(12_000); + }, + ); }); it("defaults compaction mode to safeguard", async () => { - await withTempHome(async (home) => { - const configDir = path.join(home, ".openclaw"); - await fs.mkdir(configDir, { recursive: true }); - await fs.writeFile( - path.join(configDir, "openclaw.json"), - JSON.stringify( - { - agents: { - defaults: { - compaction: { - reserveTokensFloor: 9000, - }, - }, + await withTempHomeConfig( + { + agents: { + defaults: { + compaction: { + reserveTokensFloor: 9000, }, }, - null, - 2, - ), - "utf-8", - ); + }, + }, + async () => { + const cfg = loadConfig(); - const cfg = loadConfig(); - - expect(cfg.agents?.defaults?.compaction?.mode).toBe("safeguard"); - expect(cfg.agents?.defaults?.compaction?.reserveTokensFloor).toBe(9000); - }); + expect(cfg.agents?.defaults?.compaction?.mode).toBe("safeguard"); + expect(cfg.agents?.defaults?.compaction?.reserveTokensFloor).toBe(9000); + }, + ); }); }); diff --git a/src/config/config.discord.test.ts b/src/config/config.discord.test.ts index bd0ac31822e..8afde31b9e3 100644 --- a/src/config/config.discord.test.ts +++ b/src/config/config.discord.test.ts @@ -1,8 +1,6 @@ -import fs from "node:fs/promises"; -import path from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { loadConfig, validateConfigObject } from "./config.js"; -import { withTempHome } from "./test-helpers.js"; +import { withTempHomeConfig } from "./test-helpers.js"; describe("config discord", () => { let previousHome: string | undefined; @@ -16,57 +14,48 @@ describe("config discord", () => { }); it("loads discord guild map + dm group settings", async () => { - await withTempHome(async (home) => { - const configDir = path.join(home, ".openclaw"); - await fs.mkdir(configDir, { recursive: true }); - await fs.writeFile( - path.join(configDir, "openclaw.json"), - JSON.stringify( - { - channels: { - discord: { - enabled: true, - dm: { - enabled: true, - allowFrom: ["steipete"], - groupEnabled: true, - groupChannels: ["openclaw-dm"], - }, - actions: { - emojiUploads: true, - stickerUploads: false, - channels: true, - }, - guilds: { - "123": { - slug: "friends-of-openclaw", - requireMention: false, - users: ["steipete"], - channels: { - general: { allow: true }, - }, - }, + await withTempHomeConfig( + { + channels: { + discord: { + enabled: true, + dm: { + enabled: true, + allowFrom: ["steipete"], + groupEnabled: true, + groupChannels: ["openclaw-dm"], + }, + actions: { + emojiUploads: true, + stickerUploads: false, + channels: true, + }, + guilds: { + "123": { + slug: "friends-of-openclaw", + requireMention: false, + users: ["steipete"], + channels: { + general: { allow: true }, }, }, }, }, - null, - 2, - ), - "utf-8", - ); + }, + }, + async () => { + const cfg = loadConfig(); - const cfg = loadConfig(); - - expect(cfg.channels?.discord?.enabled).toBe(true); - expect(cfg.channels?.discord?.dm?.groupEnabled).toBe(true); - expect(cfg.channels?.discord?.dm?.groupChannels).toEqual(["openclaw-dm"]); - expect(cfg.channels?.discord?.actions?.emojiUploads).toBe(true); - expect(cfg.channels?.discord?.actions?.stickerUploads).toBe(false); - expect(cfg.channels?.discord?.actions?.channels).toBe(true); - expect(cfg.channels?.discord?.guilds?.["123"]?.slug).toBe("friends-of-openclaw"); - expect(cfg.channels?.discord?.guilds?.["123"]?.channels?.general?.allow).toBe(true); - }); + expect(cfg.channels?.discord?.enabled).toBe(true); + expect(cfg.channels?.discord?.dm?.groupEnabled).toBe(true); + expect(cfg.channels?.discord?.dm?.groupChannels).toEqual(["openclaw-dm"]); + expect(cfg.channels?.discord?.actions?.emojiUploads).toBe(true); + expect(cfg.channels?.discord?.actions?.stickerUploads).toBe(false); + expect(cfg.channels?.discord?.actions?.channels).toBe(true); + expect(cfg.channels?.discord?.guilds?.["123"]?.slug).toBe("friends-of-openclaw"); + expect(cfg.channels?.discord?.guilds?.["123"]?.channels?.general?.allow).toBe(true); + }, + ); }); it("rejects numeric discord allowlist entries", () => { diff --git a/src/config/config.legacy-config-detection.accepts-imessage-dmpolicy.test.ts b/src/config/config.legacy-config-detection.accepts-imessage-dmpolicy.test.ts index e4a5ddcfdba..1fec5ba6d60 100644 --- a/src/config/config.legacy-config-detection.accepts-imessage-dmpolicy.test.ts +++ b/src/config/config.legacy-config-detection.accepts-imessage-dmpolicy.test.ts @@ -26,6 +26,22 @@ async function expectLoadRejectionPreservesField(params: { }); } +type ConfigSnapshot = Awaited>; + +async function withSnapshotForConfig( + config: unknown, + run: (params: { snapshot: ConfigSnapshot; parsed: unknown; configPath: string }) => Promise, +) { + await withTempHome(async (home) => { + const configPath = path.join(home, ".openclaw", "openclaw.json"); + await fs.mkdir(path.dirname(configPath), { recursive: true }); + await fs.writeFile(configPath, JSON.stringify(config, null, 2), "utf-8"); + const snapshot = await readConfigFileSnapshot(); + const parsed = JSON.parse(await fs.readFile(configPath, "utf-8")) as unknown; + await run({ snapshot, parsed, configPath }); + }); +} + function expectValidConfigValue(params: { config: unknown; readValue: (config: unknown) => unknown; @@ -47,6 +63,20 @@ function expectInvalidIssuePath(config: unknown, expectedPath: string) { } } +function expectRoutingAllowFromLegacySnapshot( + ctx: { snapshot: ConfigSnapshot; parsed: unknown }, + expectedAllowFrom: string[], +) { + expect(ctx.snapshot.valid).toBe(false); + expect(ctx.snapshot.legacyIssues.some((issue) => issue.path === "routing.allowFrom")).toBe(true); + const parsed = ctx.parsed as { + routing?: { allowFrom?: string[] }; + channels?: unknown; + }; + expect(parsed.routing?.allowFrom).toEqual(expectedAllowFrom); + expect(parsed.channels).toBeUndefined(); +} + describe("legacy config detection", () => { it('accepts imessage.dmPolicy="open" with allowFrom "*"', async () => { const res = validateConfigObject({ @@ -224,43 +254,30 @@ describe("legacy config detection", () => { expect((res.config as { agent?: unknown } | undefined)?.agent).toBeUndefined(); }); it("flags legacy config in snapshot", async () => { - await withTempHome(async (home) => { - const configPath = path.join(home, ".openclaw", "openclaw.json"); - await fs.mkdir(path.dirname(configPath), { recursive: true }); - await fs.writeFile( - configPath, - JSON.stringify({ routing: { allowFrom: ["+15555550123"] } }), - "utf-8", - ); - - const snap = await readConfigFileSnapshot(); - - expect(snap.valid).toBe(false); - expect(snap.legacyIssues.some((issue) => issue.path === "routing.allowFrom")).toBe(true); - - const raw = await fs.readFile(configPath, "utf-8"); - const parsed = JSON.parse(raw) as { - routing?: { allowFrom?: string[] }; - channels?: unknown; - }; - expect(parsed.routing?.allowFrom).toEqual(["+15555550123"]); - expect(parsed.channels).toBeUndefined(); + await withSnapshotForConfig({ routing: { allowFrom: ["+15555550123"] } }, async (ctx) => { + expectRoutingAllowFromLegacySnapshot(ctx, ["+15555550123"]); }); }); it("flags top-level memorySearch as legacy in snapshot", async () => { - await withTempHome(async (home) => { - const configPath = path.join(home, ".openclaw", "openclaw.json"); - await fs.mkdir(path.dirname(configPath), { recursive: true }); - await fs.writeFile( - configPath, - JSON.stringify({ memorySearch: { provider: "local", fallback: "none" } }), - "utf-8", - ); + await withSnapshotForConfig( + { memorySearch: { provider: "local", fallback: "none" } }, + async (ctx) => { + expect(ctx.snapshot.valid).toBe(false); + expect(ctx.snapshot.legacyIssues.some((issue) => issue.path === "memorySearch")).toBe(true); + }, + ); + }); + it("flags legacy provider sections in snapshot", async () => { + await withSnapshotForConfig({ whatsapp: { allowFrom: ["+1555"] } }, async (ctx) => { + expect(ctx.snapshot.valid).toBe(false); + expect(ctx.snapshot.legacyIssues.some((issue) => issue.path === "whatsapp")).toBe(true); - const snap = await readConfigFileSnapshot(); - - expect(snap.valid).toBe(false); - expect(snap.legacyIssues.some((issue) => issue.path === "memorySearch")).toBe(true); + const parsed = ctx.parsed as { + channels?: unknown; + whatsapp?: unknown; + }; + expect(parsed.channels).toBeUndefined(); + expect(parsed.whatsapp).toBeTruthy(); }); }); it("does not auto-migrate claude-cli auth profile mode on load", async () => { @@ -293,52 +310,9 @@ describe("legacy config detection", () => { expect(parsed.auth?.profiles?.["anthropic:claude-cli"]?.mode).toBe("token"); }); }); - it("flags legacy provider sections in snapshot", async () => { - await withTempHome(async (home) => { - const configPath = path.join(home, ".openclaw", "openclaw.json"); - await fs.mkdir(path.dirname(configPath), { recursive: true }); - await fs.writeFile( - configPath, - JSON.stringify({ whatsapp: { allowFrom: ["+1555"] } }, null, 2), - "utf-8", - ); - - const snap = await readConfigFileSnapshot(); - - expect(snap.valid).toBe(false); - expect(snap.legacyIssues.some((issue) => issue.path === "whatsapp")).toBe(true); - - const raw = await fs.readFile(configPath, "utf-8"); - const parsed = JSON.parse(raw) as { - channels?: unknown; - whatsapp?: unknown; - }; - expect(parsed.channels).toBeUndefined(); - expect(parsed.whatsapp).toBeTruthy(); - }); - }); it("flags routing.allowFrom in snapshot", async () => { - await withTempHome(async (home) => { - const configPath = path.join(home, ".openclaw", "openclaw.json"); - await fs.mkdir(path.dirname(configPath), { recursive: true }); - await fs.writeFile( - configPath, - JSON.stringify({ routing: { allowFrom: ["+1666"] } }, null, 2), - "utf-8", - ); - - const snap = await readConfigFileSnapshot(); - - expect(snap.valid).toBe(false); - expect(snap.legacyIssues.some((issue) => issue.path === "routing.allowFrom")).toBe(true); - - const raw = await fs.readFile(configPath, "utf-8"); - const parsed = JSON.parse(raw) as { - channels?: unknown; - routing?: { allowFrom?: string[] }; - }; - expect(parsed.channels).toBeUndefined(); - expect(parsed.routing?.allowFrom).toEqual(["+1666"]); + await withSnapshotForConfig({ routing: { allowFrom: ["+1666"] } }, async (ctx) => { + expectRoutingAllowFromLegacySnapshot(ctx, ["+1666"]); }); }); it("rejects bindings[].match.provider on load", async () => { @@ -374,61 +348,40 @@ describe("legacy config detection", () => { }); }); it("rejects session.sendPolicy.rules[].match.provider on load", async () => { - await withTempHome(async (home) => { - const configPath = path.join(home, ".openclaw", "openclaw.json"); - await fs.mkdir(path.dirname(configPath), { recursive: true }); - await fs.writeFile( - configPath, - JSON.stringify( - { - session: { - sendPolicy: { - rules: [{ action: "deny", match: { provider: "telegram" } }], - }, - }, + await withSnapshotForConfig( + { + session: { + sendPolicy: { + rules: [{ action: "deny", match: { provider: "telegram" } }], }, - null, - 2, - ), - "utf-8", - ); - - const snap = await readConfigFileSnapshot(); - - expect(snap.valid).toBe(false); - expect(snap.issues.length).toBeGreaterThan(0); - - const raw = await fs.readFile(configPath, "utf-8"); - const parsed = JSON.parse(raw) as { - session?: { sendPolicy?: { rules?: Array<{ match?: { provider?: string } }> } }; - }; - expect(parsed.session?.sendPolicy?.rules?.[0]?.match?.provider).toBe("telegram"); - }); + }, + }, + async (ctx) => { + expect(ctx.snapshot.valid).toBe(false); + expect(ctx.snapshot.issues.length).toBeGreaterThan(0); + const parsed = ctx.parsed as { + session?: { sendPolicy?: { rules?: Array<{ match?: { provider?: string } }> } }; + }; + expect(parsed.session?.sendPolicy?.rules?.[0]?.match?.provider).toBe("telegram"); + }, + ); }); it("rejects messages.queue.byProvider on load", async () => { - await withTempHome(async (home) => { - const configPath = path.join(home, ".openclaw", "openclaw.json"); - await fs.mkdir(path.dirname(configPath), { recursive: true }); - await fs.writeFile( - configPath, - JSON.stringify({ messages: { queue: { byProvider: { whatsapp: "queue" } } } }, null, 2), - "utf-8", - ); + await withSnapshotForConfig( + { messages: { queue: { byProvider: { whatsapp: "queue" } } } }, + async (ctx) => { + expect(ctx.snapshot.valid).toBe(false); + expect(ctx.snapshot.issues.length).toBeGreaterThan(0); - const snap = await readConfigFileSnapshot(); - - expect(snap.valid).toBe(false); - expect(snap.issues.length).toBeGreaterThan(0); - - const raw = await fs.readFile(configPath, "utf-8"); - const parsed = JSON.parse(raw) as { - messages?: { - queue?: { - byProvider?: Record; + const parsed = ctx.parsed as { + messages?: { + queue?: { + byProvider?: Record; + }; }; }; - }; - expect(parsed.messages?.queue?.byProvider?.whatsapp).toBe("queue"); - }); + expect(parsed.messages?.queue?.byProvider?.whatsapp).toBe("queue"); + }, + ); }); }); diff --git a/src/config/config.multi-agent-agentdir-validation.test.ts b/src/config/config.multi-agent-agentdir-validation.test.ts index 5a49d1e285f..efe534fe14e 100644 --- a/src/config/config.multi-agent-agentdir-validation.test.ts +++ b/src/config/config.multi-agent-agentdir-validation.test.ts @@ -1,9 +1,8 @@ -import fs from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; import { describe, expect, it, vi } from "vitest"; import { loadConfig, validateConfigObject } from "./config.js"; -import { withTempHome } from "./test-helpers.js"; +import { withTempHomeConfig } from "./test-helpers.js"; describe("multi-agent agentDir validation", () => { it("rejects shared agents.list agentDir", async () => { @@ -24,31 +23,22 @@ describe("multi-agent agentDir validation", () => { }); it("throws on shared agentDir during loadConfig()", async () => { - await withTempHome(async (home) => { - const configDir = path.join(home, ".openclaw"); - await fs.mkdir(configDir, { recursive: true }); - await fs.writeFile( - path.join(configDir, "openclaw.json"), - JSON.stringify( - { - agents: { - list: [ - { id: "a", agentDir: "~/.openclaw/agents/shared/agent" }, - { id: "b", agentDir: "~/.openclaw/agents/shared/agent" }, - ], - }, - bindings: [{ agentId: "a", match: { channel: "telegram" } }], - }, - null, - 2, - ), - "utf-8", - ); - - const spy = vi.spyOn(console, "error").mockImplementation(() => {}); - expect(() => loadConfig()).toThrow(/duplicate agentDir/i); - expect(spy.mock.calls.flat().join(" ")).toMatch(/Duplicate agentDir/i); - spy.mockRestore(); - }); + await withTempHomeConfig( + { + agents: { + list: [ + { id: "a", agentDir: "~/.openclaw/agents/shared/agent" }, + { id: "b", agentDir: "~/.openclaw/agents/shared/agent" }, + ], + }, + bindings: [{ agentId: "a", match: { channel: "telegram" } }], + }, + async () => { + const spy = vi.spyOn(console, "error").mockImplementation(() => {}); + expect(() => loadConfig()).toThrow(/duplicate agentDir/i); + expect(spy.mock.calls.flat().join(" ")).toMatch(/Duplicate agentDir/i); + spy.mockRestore(); + }, + ); }); }); diff --git a/src/config/config.nix-integration-u3-u5-u9.test.ts b/src/config/config.nix-integration-u3-u5-u9.test.ts index 371b1da121c..5e843607ddb 100644 --- a/src/config/config.nix-integration-u3-u5-u9.test.ts +++ b/src/config/config.nix-integration-u3-u5-u9.test.ts @@ -9,7 +9,7 @@ import { resolveIsNixMode, resolveStateDir, } from "./config.js"; -import { withTempHome } from "./test-helpers.js"; +import { withTempHome, withTempHomeConfig } from "./test-helpers.js"; function envWith(overrides: Record): NodeJS.ProcessEnv { // Hermetic env: don't inherit process.env because other tests may mutate it. @@ -23,6 +23,16 @@ function loadConfigForHome(home: string) { }).loadConfig(); } +async function withLoadedConfigForHome( + config: unknown, + run: (cfg: ReturnType) => Promise | void, +) { + await withTempHomeConfig(config, async ({ home }) => { + const cfg = loadConfigForHome(home); + await run(cfg); + }); +} + describe("Nix integration (U3, U5, U9)", () => { describe("U3: isNixMode env var detection", () => { it("isNixMode is false when OPENCLAW_NIX_MODE is not set", () => { @@ -211,62 +221,44 @@ describe("Nix integration (U3, U5, U9)", () => { describe("U9: telegram.tokenFile schema validation", () => { it("accepts config with only botToken", async () => { - await withTempHome(async (home) => { - const configDir = path.join(home, ".openclaw"); - await fs.mkdir(configDir, { recursive: true }); - await fs.writeFile( - path.join(configDir, "openclaw.json"), - JSON.stringify({ - channels: { telegram: { botToken: "123:ABC" } }, - }), - "utf-8", - ); - - const cfg = loadConfigForHome(home); - expect(cfg.channels?.telegram?.botToken).toBe("123:ABC"); - expect(cfg.channels?.telegram?.tokenFile).toBeUndefined(); - }); + await withLoadedConfigForHome( + { + channels: { telegram: { botToken: "123:ABC" } }, + }, + async (cfg) => { + expect(cfg.channels?.telegram?.botToken).toBe("123:ABC"); + expect(cfg.channels?.telegram?.tokenFile).toBeUndefined(); + }, + ); }); it("accepts config with only tokenFile", async () => { - await withTempHome(async (home) => { - const configDir = path.join(home, ".openclaw"); - await fs.mkdir(configDir, { recursive: true }); - await fs.writeFile( - path.join(configDir, "openclaw.json"), - JSON.stringify({ - channels: { telegram: { tokenFile: "/run/agenix/telegram-token" } }, - }), - "utf-8", - ); - - const cfg = loadConfigForHome(home); - expect(cfg.channels?.telegram?.tokenFile).toBe("/run/agenix/telegram-token"); - expect(cfg.channels?.telegram?.botToken).toBeUndefined(); - }); + await withLoadedConfigForHome( + { + channels: { telegram: { tokenFile: "/run/agenix/telegram-token" } }, + }, + async (cfg) => { + expect(cfg.channels?.telegram?.tokenFile).toBe("/run/agenix/telegram-token"); + expect(cfg.channels?.telegram?.botToken).toBeUndefined(); + }, + ); }); it("accepts config with both botToken and tokenFile", async () => { - await withTempHome(async (home) => { - const configDir = path.join(home, ".openclaw"); - await fs.mkdir(configDir, { recursive: true }); - await fs.writeFile( - path.join(configDir, "openclaw.json"), - JSON.stringify({ - channels: { - telegram: { - botToken: "fallback:token", - tokenFile: "/run/agenix/telegram-token", - }, + await withLoadedConfigForHome( + { + channels: { + telegram: { + botToken: "fallback:token", + tokenFile: "/run/agenix/telegram-token", }, - }), - "utf-8", - ); - - const cfg = loadConfigForHome(home); - expect(cfg.channels?.telegram?.botToken).toBe("fallback:token"); - expect(cfg.channels?.telegram?.tokenFile).toBe("/run/agenix/telegram-token"); - }); + }, + }, + async (cfg) => { + expect(cfg.channels?.telegram?.botToken).toBe("fallback:token"); + expect(cfg.channels?.telegram?.tokenFile).toBe("/run/agenix/telegram-token"); + }, + ); }); }); }); diff --git a/src/config/env-preserve-io.test.ts b/src/config/env-preserve-io.test.ts index 9e94a704091..ce6a215f611 100644 --- a/src/config/env-preserve-io.test.ts +++ b/src/config/env-preserve-io.test.ts @@ -62,6 +62,28 @@ async function withWrapperEnvContext(configPath: string, run: () => Promise { + return { MY_API_KEY: initialValue }; +} + +async function withGatewayTokenTempConfig( + run: (configPath: string) => Promise, +): Promise { + await withTempConfig(createGatewayTokenConfigJson(), run); +} + +async function withWrapperGatewayTokenContext( + run: (configPath: string) => Promise, +): Promise { + await withGatewayTokenTempConfig(async (configPath) => { + await withWrapperEnvContext(configPath, async () => run(configPath)); + }); +} + async function readGatewayToken(configPath: string): Promise { const written = await fs.readFile(configPath, "utf-8"); const parsed = JSON.parse(written) as { gateway: { remote: { token: string } } }; @@ -70,13 +92,8 @@ async function readGatewayToken(configPath: string): Promise { describe("env snapshot TOCTOU via createConfigIO", () => { it("restores env refs using read-time env even after env mutation", async () => { - const env: Record = { - MY_API_KEY: "original-key-123", - }; - - const configJson = JSON.stringify({ gateway: { remote: { token: "${MY_API_KEY}" } } }, null, 2); - - await withTempConfig(configJson, async (configPath) => { + const env = createMutableApiKeyEnv(); + await withGatewayTokenTempConfig(async (configPath) => { // Instance A: read config (captures env snapshot) const ioA = createConfigIO({ configPath, env: env as unknown as NodeJS.ProcessEnv }); const firstRead = await ioA.readConfigFileSnapshotForWrite(); @@ -99,13 +116,8 @@ describe("env snapshot TOCTOU via createConfigIO", () => { }); it("without snapshot bridging, mutated env causes incorrect restoration", async () => { - const env: Record = { - MY_API_KEY: "original-key-123", - }; - - const configJson = JSON.stringify({ gateway: { remote: { token: "${MY_API_KEY}" } } }, null, 2); - - await withTempConfig(configJson, async (configPath) => { + const env = createMutableApiKeyEnv(); + await withGatewayTokenTempConfig(async (configPath) => { // Instance A: read config const ioA = createConfigIO({ configPath, env: env as unknown as NodeJS.ProcessEnv }); const snapshot = await ioA.readConfigFileSnapshot(); @@ -132,40 +144,34 @@ describe("env snapshot TOCTOU via createConfigIO", () => { describe("env snapshot TOCTOU via wrapper APIs", () => { it("uses explicit read context even if another read interleaves", async () => { - const configJson = JSON.stringify({ gateway: { remote: { token: "${MY_API_KEY}" } } }, null, 2); - await withTempConfig(configJson, async (configPath) => { - await withWrapperEnvContext(configPath, async () => { - const firstRead = await readConfigFileSnapshotForWrite(); - expect(firstRead.snapshot.config.gateway?.remote?.token).toBe("original-key-123"); + await withWrapperGatewayTokenContext(async (configPath) => { + const firstRead = await readConfigFileSnapshotForWrite(); + expect(firstRead.snapshot.config.gateway?.remote?.token).toBe("original-key-123"); - // Interleaving read from another request context with a different env value. - process.env.MY_API_KEY = "mutated-key-456"; - const secondRead = await readConfigFileSnapshotForWrite(); - expect(secondRead.snapshot.config.gateway?.remote?.token).toBe("mutated-key-456"); + // Interleaving read from another request context with a different env value. + process.env.MY_API_KEY = "mutated-key-456"; + const secondRead = await readConfigFileSnapshotForWrite(); + expect(secondRead.snapshot.config.gateway?.remote?.token).toBe("mutated-key-456"); - // Write using the first read's explicit context. - await writeConfigFileViaWrapper(firstRead.snapshot.config, firstRead.writeOptions); - expect(await readGatewayToken(configPath)).toBe("${MY_API_KEY}"); - }); + // Write using the first read's explicit context. + await writeConfigFileViaWrapper(firstRead.snapshot.config, firstRead.writeOptions); + expect(await readGatewayToken(configPath)).toBe("${MY_API_KEY}"); }); }); it("ignores read context when expected config path does not match", async () => { - const configJson = JSON.stringify({ gateway: { remote: { token: "${MY_API_KEY}" } } }, null, 2); - await withTempConfig(configJson, async (configPath) => { - await withWrapperEnvContext(configPath, async () => { - const firstRead = await readConfigFileSnapshotForWrite(); - expect(firstRead.snapshot.config.gateway?.remote?.token).toBe("original-key-123"); - expect(firstRead.writeOptions.expectedConfigPath).toBe(configPath); + await withWrapperGatewayTokenContext(async (configPath) => { + const firstRead = await readConfigFileSnapshotForWrite(); + expect(firstRead.snapshot.config.gateway?.remote?.token).toBe("original-key-123"); + expect(firstRead.writeOptions.expectedConfigPath).toBe(configPath); - process.env.MY_API_KEY = "mutated-key-456"; - await writeConfigFileViaWrapper(firstRead.snapshot.config, { - ...firstRead.writeOptions, - expectedConfigPath: `${configPath}.different`, - }); - - expect(await readGatewayToken(configPath)).toBe("original-key-123"); + process.env.MY_API_KEY = "mutated-key-456"; + await writeConfigFileViaWrapper(firstRead.snapshot.config, { + ...firstRead.writeOptions, + expectedConfigPath: `${configPath}.different`, }); + + expect(await readGatewayToken(configPath)).toBe("original-key-123"); }); }); }); diff --git a/src/config/redact-snapshot.test.ts b/src/config/redact-snapshot.test.ts index 95b26ecaebf..0973560c68b 100644 --- a/src/config/redact-snapshot.test.ts +++ b/src/config/redact-snapshot.test.ts @@ -46,6 +46,27 @@ function restoreRedactedValues( return result.result as TOriginal; } +function expectNestedLevelPairValue( + source: Record>>, + field: string, + expected: readonly [unknown, unknown], +): void { + const values = source.nested.level[field] as unknown[]; + expect(values[0]).toBe(expected[0]); + expect(values[1]).toBe(expected[1]); +} + +function expectGatewayAuthFieldValue( + result: ReturnType, + field: "token" | "password", + expected: string, +): void { + const gateway = result.config.gateway as Record>; + const resolved = result.resolved as Record>>; + expect(gateway.auth[field]).toBe(expected); + expect(resolved.gateway.auth[field]).toBe(expected); +} + describe("redactConfigSnapshot", () => { it("redacts common secret field patterns across config sections", () => { const snapshot = makeSnapshot({ @@ -560,12 +581,10 @@ describe("redactConfigSnapshot", () => { }), assert: ({ redacted, restored }) => { const cfg = redacted as Record>>; - expect((cfg.nested.level.token as unknown[])[0]).toBe(42); - expect((cfg.nested.level.token as unknown[])[1]).toBe(815); + expectNestedLevelPairValue(cfg, "token", [42, 815]); const out = restored as Record>>; - expect((out.nested.level.token as unknown[])[0]).toBe(42); - expect((out.nested.level.token as unknown[])[1]).toBe(815); + expectNestedLevelPairValue(out, "token", [42, 815]); }, }, { @@ -604,12 +623,10 @@ describe("redactConfigSnapshot", () => { }), assert: ({ redacted, restored }) => { const cfg = redacted as Record>>; - expect((cfg.nested.level.custom as unknown[])[0]).toBe(42); - expect((cfg.nested.level.custom as unknown[])[1]).toBe(815); + expectNestedLevelPairValue(cfg, "custom", [42, 815]); const out = restored as Record>>; - expect((out.nested.level.custom as unknown[])[0]).toBe(42); - expect((out.nested.level.custom as unknown[])[1]).toBe(815); + expectNestedLevelPairValue(out, "custom", [42, 815]); }, }, ]; @@ -636,10 +653,7 @@ describe("redactConfigSnapshot", () => { gateway: { auth: { token: "not-actually-secret-value" } }, }); const result = redactConfigSnapshot(snapshot, hints); - const gw = result.config.gateway as Record>; - const resolved = result.resolved as Record>>; - expect(gw.auth.token).toBe("not-actually-secret-value"); - expect(resolved.gateway.auth.token).toBe("not-actually-secret-value"); + expectGatewayAuthFieldValue(result, "token", "not-actually-secret-value"); }); it("does not redact paths absent from uiHints (schema is single source of truth)", () => { @@ -650,10 +664,7 @@ describe("redactConfigSnapshot", () => { gateway: { auth: { password: "not-in-hints-value" } }, }); const result = redactConfigSnapshot(snapshot, hints); - const gw = result.config.gateway as Record>; - const resolved = result.resolved as Record>>; - expect(gw.auth.password).toBe("not-in-hints-value"); - expect(resolved.gateway.auth.password).toBe("not-in-hints-value"); + expectGatewayAuthFieldValue(result, "password", "not-in-hints-value"); }); it("uses wildcard hints for array items", () => { diff --git a/src/config/sessions/store.pruning.integration.test.ts b/src/config/sessions/store.pruning.integration.test.ts index a8c3ed41325..f1ef11e7cd3 100644 --- a/src/config/sessions/store.pruning.integration.test.ts +++ b/src/config/sessions/store.pruning.integration.test.ts @@ -43,6 +43,13 @@ async function createCaseDir(prefix: string): Promise { return dir; } +function createStaleAndFreshStore(now = Date.now()): Record { + return { + stale: makeEntry(now - 30 * DAY_MS), + fresh: makeEntry(now), + }; +} + describe("Integration: saveSessionStore with pruning", () => { let testDir: string; let storePath: string; @@ -78,11 +85,7 @@ describe("Integration: saveSessionStore with pruning", () => { it("saveSessionStore prunes stale entries on write", async () => { applyEnforcedMaintenanceConfig(mockLoadConfig); - const now = Date.now(); - const store: Record = { - stale: makeEntry(now - 30 * DAY_MS), - fresh: makeEntry(now), - }; + const store = createStaleAndFreshStore(); await saveSessionStore(storePath, store); @@ -168,11 +171,7 @@ describe("Integration: saveSessionStore with pruning", () => { }, }); - const now = Date.now(); - const store: Record = { - stale: makeEntry(now - 30 * DAY_MS), - fresh: makeEntry(now), - }; + const store = createStaleAndFreshStore(); await saveSessionStore(storePath, store); diff --git a/src/config/test-helpers.ts b/src/config/test-helpers.ts index b1a229a6ea5..14e62ddfd74 100644 --- a/src/config/test-helpers.ts +++ b/src/config/test-helpers.ts @@ -1,9 +1,28 @@ +import fs from "node:fs/promises"; +import path from "node:path"; import { withTempHome as withTempHomeBase } from "../../test/helpers/temp-home.js"; export async function withTempHome(fn: (home: string) => Promise): Promise { return withTempHomeBase(fn, { prefix: "openclaw-config-" }); } +export async function writeOpenClawConfig(home: string, config: unknown): Promise { + const configPath = path.join(home, ".openclaw", "openclaw.json"); + await fs.mkdir(path.dirname(configPath), { recursive: true }); + await fs.writeFile(configPath, JSON.stringify(config, null, 2), "utf-8"); + return configPath; +} + +export async function withTempHomeConfig( + config: unknown, + fn: (params: { home: string; configPath: string }) => Promise, +): Promise { + return withTempHome(async (home) => { + const configPath = await writeOpenClawConfig(home, config); + return fn({ home, configPath }); + }); +} + /** * Helper to test env var overrides. Saves/restores env vars for a callback. */ diff --git a/src/config/zod-schema.agent-runtime.ts b/src/config/zod-schema.agent-runtime.ts index f3f5a8b7a60..507ec4c0fc1 100644 --- a/src/config/zod-schema.agent-runtime.ts +++ b/src/config/zod-schema.agent-runtime.ts @@ -440,13 +440,17 @@ export const AgentSandboxSchema = z .strict() .optional(); +const CommonToolPolicyFields = { + profile: ToolProfileSchema, + allow: z.array(z.string()).optional(), + alsoAllow: z.array(z.string()).optional(), + deny: z.array(z.string()).optional(), + byProvider: z.record(z.string(), ToolPolicyWithProfileSchema).optional(), +}; + export const AgentToolsSchema = z .object({ - profile: ToolProfileSchema, - allow: z.array(z.string()).optional(), - alsoAllow: z.array(z.string()).optional(), - deny: z.array(z.string()).optional(), - byProvider: z.record(z.string(), ToolPolicyWithProfileSchema).optional(), + ...CommonToolPolicyFields, elevated: z .object({ enabled: z.boolean().optional(), @@ -641,11 +645,7 @@ export const AgentEntrySchema = z export const ToolsSchema = z .object({ - profile: ToolProfileSchema, - allow: z.array(z.string()).optional(), - alsoAllow: z.array(z.string()).optional(), - deny: z.array(z.string()).optional(), - byProvider: z.record(z.string(), ToolPolicyWithProfileSchema).optional(), + ...CommonToolPolicyFields, web: ToolsWebSchema, media: ToolsMediaSchema, links: ToolsLinksSchema, diff --git a/src/config/zod-schema.core.ts b/src/config/zod-schema.core.ts index 4431a51c790..2b4dab28c48 100644 --- a/src/config/zod-schema.core.ts +++ b/src/config/zod-schema.core.ts @@ -430,6 +430,16 @@ const ProviderOptionsSchema = z .record(z.string(), z.record(z.string(), ProviderOptionValueSchema)) .optional(); +const MediaUnderstandingRuntimeFields = { + prompt: z.string().optional(), + timeoutSeconds: z.number().int().positive().optional(), + language: z.string().optional(), + providerOptions: ProviderOptionsSchema, + deepgram: DeepgramAudioSchema, + baseUrl: z.string().optional(), + headers: z.record(z.string(), z.string()).optional(), +}; + export const MediaUnderstandingModelSchema = z .object({ provider: z.string().optional(), @@ -438,15 +448,9 @@ export const MediaUnderstandingModelSchema = z type: z.union([z.literal("provider"), z.literal("cli")]).optional(), command: z.string().optional(), args: z.array(z.string()).optional(), - prompt: z.string().optional(), maxChars: z.number().int().positive().optional(), maxBytes: z.number().int().positive().optional(), - timeoutSeconds: z.number().int().positive().optional(), - language: z.string().optional(), - providerOptions: ProviderOptionsSchema, - deepgram: DeepgramAudioSchema, - baseUrl: z.string().optional(), - headers: z.record(z.string(), z.string()).optional(), + ...MediaUnderstandingRuntimeFields, profile: z.string().optional(), preferredProfile: z.string().optional(), }) @@ -459,13 +463,7 @@ export const ToolsMediaUnderstandingSchema = z scope: MediaUnderstandingScopeSchema, maxBytes: z.number().int().positive().optional(), maxChars: z.number().int().positive().optional(), - prompt: z.string().optional(), - timeoutSeconds: z.number().int().positive().optional(), - language: z.string().optional(), - providerOptions: ProviderOptionsSchema, - deepgram: DeepgramAudioSchema, - baseUrl: z.string().optional(), - headers: z.record(z.string(), z.string()).optional(), + ...MediaUnderstandingRuntimeFields, attachments: MediaUnderstandingAttachmentsSchema, models: z.array(MediaUnderstandingModelSchema).optional(), }) diff --git a/src/cron/isolated-agent/run.ts b/src/cron/isolated-agent/run.ts index bb8c2f67833..8ba4c051b8b 100644 --- a/src/cron/isolated-agent/run.ts +++ b/src/cron/isolated-agent/run.ts @@ -611,8 +611,7 @@ export async function runCronIsolatedAgentTurn(params: { logWarn(`[cron:${params.job.id}] ${resolvedDelivery.error.message}`); return withRunSession({ status: "ok", summary, outputText, ...telemetry }); } - if (!resolvedDelivery.channel) { - const message = "cron delivery channel is missing"; + const failOrWarnMissingDeliveryField = (message: string) => { if (!deliveryBestEffort) { return withRunSession({ status: "error", @@ -624,20 +623,12 @@ export async function runCronIsolatedAgentTurn(params: { } logWarn(`[cron:${params.job.id}] ${message}`); return withRunSession({ status: "ok", summary, outputText, ...telemetry }); + }; + if (!resolvedDelivery.channel) { + return failOrWarnMissingDeliveryField("cron delivery channel is missing"); } if (!resolvedDelivery.to) { - const message = "cron delivery target is missing"; - if (!deliveryBestEffort) { - return withRunSession({ - status: "error", - error: message, - summary, - outputText, - ...telemetry, - }); - } - logWarn(`[cron:${params.job.id}] ${message}`); - return withRunSession({ status: "ok", summary, outputText, ...telemetry }); + return failOrWarnMissingDeliveryField("cron delivery target is missing"); } const identity = resolveAgentOutboundIdentity(cfgWithAgentDefaults, agentId); diff --git a/src/cron/service/timer.ts b/src/cron/service/timer.ts index 206c82d439f..8b80dbd9070 100644 --- a/src/cron/service/timer.ts +++ b/src/cron/service/timer.ts @@ -153,6 +153,33 @@ function applyJobResult( return shouldDelete; } +function applyOutcomeToStoredJob(state: CronServiceState, result: TimedCronRunOutcome): void { + const store = state.store; + if (!store) { + return; + } + const jobs = store.jobs; + const job = jobs.find((entry) => entry.id === result.jobId); + if (!job) { + return; + } + + const shouldDelete = applyJobResult(state, job, { + status: result.status, + error: result.error, + delivered: result.delivered, + startedAt: result.startedAt, + endedAt: result.endedAt, + }); + + emitJobFinished(state, job, result, result.startedAt); + + if (shouldDelete) { + store.jobs = jobs.filter((entry) => entry.id !== job.id); + emit(state, { jobId: job.id, action: "removed" }); + } +} + export function armTimer(state: CronServiceState) { if (state.timer) { clearTimeout(state.timer); @@ -333,25 +360,7 @@ export async function onTimer(state: CronServiceState) { await ensureLoaded(state, { forceReload: true, skipRecompute: true }); for (const result of completedResults) { - const job = state.store?.jobs.find((j) => j.id === result.jobId); - if (!job) { - continue; - } - - const shouldDelete = applyJobResult(state, job, { - status: result.status, - error: result.error, - delivered: result.delivered, - startedAt: result.startedAt, - endedAt: result.endedAt, - }); - - emitJobFinished(state, job, result, result.startedAt); - - if (shouldDelete && state.store) { - state.store.jobs = state.store.jobs.filter((j) => j.id !== job.id); - emit(state, { jobId: job.id, action: "removed" }); - } + applyOutcomeToStoredJob(state, result); } // Use maintenance-only recompute to avoid advancing past-due @@ -525,24 +534,7 @@ export async function runMissedJobs( } for (const result of outcomes) { - const job = state.store.jobs.find((entry) => entry.id === result.jobId); - if (!job) { - continue; - } - const shouldDelete = applyJobResult(state, job, { - status: result.status, - error: result.error, - delivered: result.delivered, - startedAt: result.startedAt, - endedAt: result.endedAt, - }); - - emitJobFinished(state, job, result, result.startedAt); - - if (shouldDelete) { - state.store.jobs = state.store.jobs.filter((entry) => entry.id !== job.id); - emit(state, { jobId: job.id, action: "removed" }); - } + applyOutcomeToStoredJob(state, result); } // Preserve any new past-due nextRunAtMs values that became due while diff --git a/src/daemon/service-env.ts b/src/daemon/service-env.ts index 10fd4223c8f..4925a337611 100644 --- a/src/daemon/service-env.ts +++ b/src/daemon/service-env.ts @@ -47,6 +47,17 @@ function addCommonUserBinDirs(dirs: string[], home: string): void { dirs.push(`${home}/.bun/bin`); } +function addCommonEnvConfiguredBinDirs( + dirs: string[], + env: Record | undefined, +): void { + addNonEmptyDir(dirs, env?.PNPM_HOME); + addNonEmptyDir(dirs, appendSubdir(env?.NPM_CONFIG_PREFIX, "bin")); + addNonEmptyDir(dirs, appendSubdir(env?.BUN_INSTALL, "bin")); + addNonEmptyDir(dirs, appendSubdir(env?.VOLTA_HOME, "bin")); + addNonEmptyDir(dirs, appendSubdir(env?.ASDF_DATA_DIR, "shims")); +} + function resolveSystemPathDirs(platform: NodeJS.Platform): string[] { if (platform === "darwin") { return ["/opt/homebrew/bin", "/usr/local/bin", "/usr/bin", "/bin"]; @@ -78,11 +89,7 @@ export function resolveDarwinUserBinDirs( // Env-configured bin roots (override defaults when present). // Note: FNM_DIR on macOS defaults to ~/Library/Application Support/fnm // Note: PNPM_HOME on macOS defaults to ~/Library/pnpm - addNonEmptyDir(dirs, env?.PNPM_HOME); - addNonEmptyDir(dirs, appendSubdir(env?.NPM_CONFIG_PREFIX, "bin")); - addNonEmptyDir(dirs, appendSubdir(env?.BUN_INSTALL, "bin")); - addNonEmptyDir(dirs, appendSubdir(env?.VOLTA_HOME, "bin")); - addNonEmptyDir(dirs, appendSubdir(env?.ASDF_DATA_DIR, "shims")); + addCommonEnvConfiguredBinDirs(dirs, env); // nvm: no stable default path, relies on env or user's shell config // User must set NVM_DIR and source nvm.sh for it to work addNonEmptyDir(dirs, env?.NVM_DIR); @@ -120,11 +127,7 @@ export function resolveLinuxUserBinDirs( const dirs: string[] = []; // Env-configured bin roots (override defaults when present). - addNonEmptyDir(dirs, env?.PNPM_HOME); - addNonEmptyDir(dirs, appendSubdir(env?.NPM_CONFIG_PREFIX, "bin")); - addNonEmptyDir(dirs, appendSubdir(env?.BUN_INSTALL, "bin")); - addNonEmptyDir(dirs, appendSubdir(env?.VOLTA_HOME, "bin")); - addNonEmptyDir(dirs, appendSubdir(env?.ASDF_DATA_DIR, "shims")); + addCommonEnvConfiguredBinDirs(dirs, env); addNonEmptyDir(dirs, appendSubdir(env?.NVM_DIR, "current/bin")); addNonEmptyDir(dirs, appendSubdir(env?.FNM_DIR, "current/bin")); diff --git a/src/infra/device-pairing.test.ts b/src/infra/device-pairing.test.ts index 04b0d995e42..c76b44b323d 100644 --- a/src/infra/device-pairing.test.ts +++ b/src/infra/device-pairing.test.ts @@ -25,6 +25,24 @@ async function setupPairedOperatorDevice(baseDir: string, scopes: string[]) { await approveDevicePairing(request.request.requestId, baseDir); } +async function setupOperatorToken(scopes: string[]) { + const baseDir = await mkdtemp(join(tmpdir(), "openclaw-device-pairing-")); + await setupPairedOperatorDevice(baseDir, scopes); + const paired = await getPairedDevice("device-1", baseDir); + const token = requireToken(paired?.tokens?.operator?.token); + return { baseDir, token }; +} + +function verifyOperatorToken(params: { baseDir: string; token: string; scopes: string[] }) { + return verifyDeviceToken({ + deviceId: "device-1", + token: params.token, + role: "operator", + scopes: params.scopes, + baseDir: params.baseDir, + }); +} + function requireToken(token: string | undefined): string { expect(typeof token).toBe("string"); if (typeof token !== "string") { @@ -163,71 +181,52 @@ describe("device pairing tokens", () => { }); test("verifies token and rejects mismatches", async () => { - const baseDir = await mkdtemp(join(tmpdir(), "openclaw-device-pairing-")); - await setupPairedOperatorDevice(baseDir, ["operator.read"]); - const paired = await getPairedDevice("device-1", baseDir); - const token = requireToken(paired?.tokens?.operator?.token); + const { baseDir, token } = await setupOperatorToken(["operator.read"]); - const ok = await verifyDeviceToken({ - deviceId: "device-1", - token, - role: "operator", - scopes: ["operator.read"], + const ok = await verifyOperatorToken({ baseDir, + token, + scopes: ["operator.read"], }); expect(ok.ok).toBe(true); - const mismatch = await verifyDeviceToken({ - deviceId: "device-1", - token: "x".repeat(token.length), - role: "operator", - scopes: ["operator.read"], + const mismatch = await verifyOperatorToken({ baseDir, + token: "x".repeat(token.length), + scopes: ["operator.read"], }); expect(mismatch.ok).toBe(false); expect(mismatch.reason).toBe("token-mismatch"); }); test("accepts operator.read/operator.write requests with an operator.admin token scope", async () => { - const baseDir = await mkdtemp(join(tmpdir(), "openclaw-device-pairing-")); - await setupPairedOperatorDevice(baseDir, ["operator.admin"]); - const paired = await getPairedDevice("device-1", baseDir); - const token = requireToken(paired?.tokens?.operator?.token); + const { baseDir, token } = await setupOperatorToken(["operator.admin"]); - const readOk = await verifyDeviceToken({ - deviceId: "device-1", - token, - role: "operator", - scopes: ["operator.read"], + const readOk = await verifyOperatorToken({ baseDir, + token, + scopes: ["operator.read"], }); expect(readOk.ok).toBe(true); - const writeOk = await verifyDeviceToken({ - deviceId: "device-1", - token, - role: "operator", - scopes: ["operator.write"], + const writeOk = await verifyOperatorToken({ baseDir, + token, + scopes: ["operator.write"], }); expect(writeOk.ok).toBe(true); }); test("treats multibyte same-length token input as mismatch without throwing", async () => { - const baseDir = await mkdtemp(join(tmpdir(), "openclaw-device-pairing-")); - await setupPairedOperatorDevice(baseDir, ["operator.read"]); - const paired = await getPairedDevice("device-1", baseDir); - const token = requireToken(paired?.tokens?.operator?.token); + const { baseDir, token } = await setupOperatorToken(["operator.read"]); const multibyteToken = "é".repeat(token.length); expect(Buffer.from(multibyteToken).length).not.toBe(Buffer.from(token).length); await expect( - verifyDeviceToken({ - deviceId: "device-1", - token: multibyteToken, - role: "operator", - scopes: ["operator.read"], + verifyOperatorToken({ baseDir, + token: multibyteToken, + scopes: ["operator.read"], }), ).resolves.toEqual({ ok: false, reason: "token-mismatch" }); }); diff --git a/src/infra/gateway-lock.test.ts b/src/infra/gateway-lock.test.ts index 195a242defc..6f3826bfcec 100644 --- a/src/infra/gateway-lock.test.ts +++ b/src/infra/gateway-lock.test.ts @@ -91,6 +91,44 @@ function mockProcStatRead(params: { onProcRead: () => string }) { }); } +async function writeLockFile( + env: NodeJS.ProcessEnv, + params: { startTime: number; createdAt?: string } = { startTime: 111 }, +) { + const { lockPath, configPath } = resolveLockPath(env); + const payload = createLockPayload({ + configPath, + startTime: params.startTime, + createdAt: params.createdAt, + }); + await fs.writeFile(lockPath, JSON.stringify(payload), "utf8"); + return { lockPath, configPath }; +} + +function createEaccesProcStatSpy() { + return mockProcStatRead({ + onProcRead: () => { + throw new Error("EACCES"); + }, + }); +} + +async function acquireStaleLinuxLock(env: NodeJS.ProcessEnv) { + await writeLockFile(env, { + startTime: 111, + createdAt: new Date(0).toISOString(), + }); + const staleProcSpy = createEaccesProcStatSpy(); + const lock = await acquireForTest(env, { + staleMs: 1, + platform: "linux", + }); + expect(lock).not.toBeNull(); + + await lock?.release(); + staleProcSpy.mockRestore(); +} + describe("gateway lock", () => { beforeAll(async () => { fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-gateway-lock-")); @@ -154,15 +192,8 @@ describe("gateway lock", () => { it("keeps lock on linux when proc access fails unless stale", async () => { vi.useRealTimers(); const env = await makeEnv(); - const { lockPath, configPath } = resolveLockPath(env); - const payload = createLockPayload({ configPath, startTime: 111 }); - await fs.writeFile(lockPath, JSON.stringify(payload), "utf8"); - - const spy = mockProcStatRead({ - onProcRead: () => { - throw new Error("EACCES"); - }, - }); + await writeLockFile(env); + const spy = createEaccesProcStatSpy(); const pending = acquireForTest(env, { timeoutMs: 15, @@ -172,42 +203,14 @@ describe("gateway lock", () => { await expect(pending).rejects.toBeInstanceOf(GatewayLockError); spy.mockRestore(); - - const stalePayload = createLockPayload({ - configPath, - startTime: 111, - createdAt: new Date(0).toISOString(), - }); - await fs.writeFile(lockPath, JSON.stringify(stalePayload), "utf8"); - - const staleSpy = mockProcStatRead({ - onProcRead: () => { - throw new Error("EACCES"); - }, - }); - - const lock = await acquireForTest(env, { - staleMs: 1, - platform: "linux", - }); - expect(lock).not.toBeNull(); - - await lock?.release(); - staleSpy.mockRestore(); + await acquireStaleLinuxLock(env); }); it("keeps lock when fs.stat fails until payload is stale", async () => { vi.useRealTimers(); const env = await makeEnv(); - const { lockPath, configPath } = resolveLockPath(env); - const payload = createLockPayload({ configPath, startTime: 111 }); - await fs.writeFile(lockPath, JSON.stringify(payload), "utf8"); - - const procSpy = mockProcStatRead({ - onProcRead: () => { - throw new Error("EACCES"); - }, - }); + await writeLockFile(env); + const procSpy = createEaccesProcStatSpy(); const statSpy = vi .spyOn(fs, "stat") .mockRejectedValue(Object.assign(new Error("EPERM"), { code: "EPERM" })); @@ -220,28 +223,7 @@ describe("gateway lock", () => { await expect(pending).rejects.toBeInstanceOf(GatewayLockError); procSpy.mockRestore(); - - const stalePayload = createLockPayload({ - configPath, - startTime: 111, - createdAt: new Date(0).toISOString(), - }); - await fs.writeFile(lockPath, JSON.stringify(stalePayload), "utf8"); - - const staleProcSpy = mockProcStatRead({ - onProcRead: () => { - throw new Error("EACCES"); - }, - }); - - const lock = await acquireForTest(env, { - staleMs: 1, - platform: "linux", - }); - expect(lock).not.toBeNull(); - - await lock?.release(); - staleProcSpy.mockRestore(); + await acquireStaleLinuxLock(env); statSpy.mockRestore(); }); diff --git a/src/infra/heartbeat-runner.ghost-reminder.test.ts b/src/infra/heartbeat-runner.ghost-reminder.test.ts index 5df817b53b4..8ef1348de06 100644 --- a/src/infra/heartbeat-runner.ghost-reminder.test.ts +++ b/src/infra/heartbeat-runner.ghost-reminder.test.ts @@ -87,16 +87,35 @@ describe("Ghost reminder bug (issue #13317)", () => { result: Awaited>; sendTelegram: ReturnType; calledCtx: { Provider?: string; Body?: string } | null; + }> => { + return runHeartbeatCase({ + tmpPrefix, + replyText: "Relay this reminder now", + reason: "cron:reminder-job", + enqueue, + }); + }; + + const runHeartbeatCase = async (params: { + tmpPrefix: string; + replyText: string; + reason: string; + enqueue: (sessionKey: string) => void; + }): Promise<{ + result: Awaited>; + sendTelegram: ReturnType; + calledCtx: { Provider?: string; Body?: string } | null; + replyCallCount: number; }> => { return withTempHeartbeatSandbox( async ({ tmpDir, storePath }) => { - const { sendTelegram, getReplySpy } = createHeartbeatDeps("Relay this reminder now"); + const { sendTelegram, getReplySpy } = createHeartbeatDeps(params.replyText); const { cfg, sessionKey } = await createConfig({ tmpDir, storePath }); - enqueue(sessionKey); + params.enqueue(sessionKey); const result = await runHeartbeatOnce({ cfg, agentId: "main", - reason: "cron:reminder-job", + reason: params.reason, deps: { sendTelegram, }, @@ -105,38 +124,32 @@ describe("Ghost reminder bug (issue #13317)", () => { Provider?: string; Body?: string; } | null; - return { result, sendTelegram, calledCtx }; + return { + result, + sendTelegram, + calledCtx, + replyCallCount: getReplySpy.mock.calls.length, + }; }, - { prefix: tmpPrefix }, + { prefix: params.tmpPrefix }, ); }; it("does not use CRON_EVENT_PROMPT when only a HEARTBEAT_OK event is present", async () => { - await withTempHeartbeatSandbox( - async ({ tmpDir, storePath }) => { - const { sendTelegram, getReplySpy } = createHeartbeatDeps("Heartbeat check-in"); - const { cfg, sessionKey } = await createConfig({ tmpDir, storePath }); + const { result, sendTelegram, calledCtx, replyCallCount } = await runHeartbeatCase({ + tmpPrefix: "openclaw-ghost-", + replyText: "Heartbeat check-in", + reason: "cron:test-job", + enqueue: (sessionKey) => { enqueueSystemEvent("HEARTBEAT_OK", { sessionKey }); - - const result = await runHeartbeatOnce({ - cfg, - agentId: "main", - reason: "cron:test-job", - deps: { - sendTelegram, - }, - }); - - expect(result.status).toBe("ran"); - expect(getReplySpy).toHaveBeenCalledTimes(1); - const calledCtx = getReplySpy.mock.calls[0]?.[0]; - expect(calledCtx?.Provider).toBe("heartbeat"); - expect(calledCtx?.Body).not.toContain("scheduled reminder has been triggered"); - expect(calledCtx?.Body).not.toContain("relay this reminder"); - expect(sendTelegram).toHaveBeenCalled(); }, - { prefix: "openclaw-ghost-" }, - ); + }); + expect(result.status).toBe("ran"); + expect(replyCallCount).toBe(1); + expect(calledCtx?.Provider).toBe("heartbeat"); + expect(calledCtx?.Body).not.toContain("scheduled reminder has been triggered"); + expect(calledCtx?.Body).not.toContain("relay this reminder"); + expect(sendTelegram).toHaveBeenCalled(); }); it("uses CRON_EVENT_PROMPT when an actionable cron event exists", async () => { @@ -165,34 +178,23 @@ describe("Ghost reminder bug (issue #13317)", () => { }); it("uses CRON_EVENT_PROMPT for tagged cron events on interval wake", async () => { - await withTempHeartbeatSandbox( - async ({ tmpDir, storePath }) => { - const { sendTelegram, getReplySpy } = createHeartbeatDeps("Relay this cron update now"); - const { cfg, sessionKey } = await createConfig({ tmpDir, storePath }); + const { result, sendTelegram, calledCtx, replyCallCount } = await runHeartbeatCase({ + tmpPrefix: "openclaw-cron-interval-", + replyText: "Relay this cron update now", + reason: "interval", + enqueue: (sessionKey) => { enqueueSystemEvent("Cron: QMD maintenance completed", { sessionKey, contextKey: "cron:qmd-maintenance", }); - - const result = await runHeartbeatOnce({ - cfg, - agentId: "main", - reason: "interval", - deps: { - sendTelegram, - }, - }); - - expect(result.status).toBe("ran"); - expect(getReplySpy).toHaveBeenCalledTimes(1); - const calledCtx = getReplySpy.mock.calls[0]?.[0]; - expect(calledCtx?.Provider).toBe("cron-event"); - expect(calledCtx?.Body).toContain("scheduled reminder has been triggered"); - expect(calledCtx?.Body).toContain("Cron: QMD maintenance completed"); - expect(calledCtx?.Body).not.toContain("Read HEARTBEAT.md"); - expect(sendTelegram).toHaveBeenCalled(); }, - { prefix: "openclaw-cron-interval-" }, - ); + }); + expect(result.status).toBe("ran"); + expect(replyCallCount).toBe(1); + expect(calledCtx?.Provider).toBe("cron-event"); + expect(calledCtx?.Body).toContain("scheduled reminder has been triggered"); + expect(calledCtx?.Body).toContain("Cron: QMD maintenance completed"); + expect(calledCtx?.Body).not.toContain("Read HEARTBEAT.md"); + expect(sendTelegram).toHaveBeenCalled(); }); }); diff --git a/src/infra/outbound/deliver.test.ts b/src/infra/outbound/deliver.test.ts index 074cf3e213b..0927de7df99 100644 --- a/src/infra/outbound/deliver.test.ts +++ b/src/infra/outbound/deliver.test.ts @@ -79,6 +79,27 @@ async function deliverWhatsAppPayload(params: { }); } +async function runChunkedWhatsAppDelivery(params?: { + mirror?: Parameters[0]["mirror"]; +}) { + const sendWhatsApp = vi + .fn() + .mockResolvedValueOnce({ messageId: "w1", toJid: "jid" }) + .mockResolvedValueOnce({ messageId: "w2", toJid: "jid" }); + const cfg: OpenClawConfig = { + channels: { whatsapp: { textChunkLimit: 2 } }, + }; + const results = await deliverOutboundPayloads({ + cfg, + channel: "whatsapp", + to: "+1555", + payloads: [{ text: "abcd" }], + deps: { sendWhatsApp }, + ...(params?.mirror ? { mirror: params.mirror } : {}), + }); + return { sendWhatsApp, results }; +} + describe("deliverOutboundPayloads", () => { beforeEach(() => { setActivePluginRegistry(defaultRegistry); @@ -238,21 +259,7 @@ describe("deliverOutboundPayloads", () => { }); it("chunks WhatsApp text and returns all results", async () => { - const sendWhatsApp = vi - .fn() - .mockResolvedValueOnce({ messageId: "w1", toJid: "jid" }) - .mockResolvedValueOnce({ messageId: "w2", toJid: "jid" }); - const cfg: OpenClawConfig = { - channels: { whatsapp: { textChunkLimit: 2 } }, - }; - - const results = await deliverOutboundPayloads({ - cfg, - channel: "whatsapp", - to: "+1555", - payloads: [{ text: "abcd" }], - deps: { sendWhatsApp }, - }); + const { sendWhatsApp, results } = await runChunkedWhatsAppDelivery(); expect(sendWhatsApp).toHaveBeenCalledTimes(2); expect(results.map((r) => r.messageId)).toEqual(["w1", "w2"]); @@ -447,24 +454,12 @@ describe("deliverOutboundPayloads", () => { }); it("emits internal message:sent hook with success=true for chunked payload delivery", async () => { - const sendWhatsApp = vi - .fn() - .mockResolvedValueOnce({ messageId: "w1", toJid: "jid" }) - .mockResolvedValueOnce({ messageId: "w2", toJid: "jid" }); - const cfg: OpenClawConfig = { - channels: { whatsapp: { textChunkLimit: 2 } }, - }; - - await deliverOutboundPayloads({ - cfg, - channel: "whatsapp", - to: "+1555", - payloads: [{ text: "abcd" }], - deps: { sendWhatsApp }, + const { sendWhatsApp } = await runChunkedWhatsAppDelivery({ mirror: { sessionKey: "agent:main:main", }, }); + expect(sendWhatsApp).toHaveBeenCalledTimes(2); expect(internalHookMocks.createInternalHookEvent).toHaveBeenCalledTimes(1); expect(internalHookMocks.createInternalHookEvent).toHaveBeenCalledWith( diff --git a/src/infra/path-safety.ts b/src/infra/path-safety.ts index df05097b312..40a72d81b13 100644 --- a/src/infra/path-safety.ts +++ b/src/infra/path-safety.ts @@ -1,4 +1,5 @@ import path from "node:path"; +import { isPathInside } from "./path-guards.js"; export function resolveSafeBaseDir(rootDir: string): string { const resolved = path.resolve(rootDir); @@ -6,15 +7,5 @@ export function resolveSafeBaseDir(rootDir: string): string { } export function isWithinDir(rootDir: string, targetPath: string): boolean { - const resolvedRoot = path.resolve(rootDir); - const resolvedTarget = path.resolve(targetPath); - - // Windows paths are effectively case-insensitive; normalize to avoid false negatives. - if (process.platform === "win32") { - const relative = path.win32.relative(resolvedRoot.toLowerCase(), resolvedTarget.toLowerCase()); - return relative === "" || (!relative.startsWith("..") && !path.win32.isAbsolute(relative)); - } - - const relative = path.relative(resolvedRoot, resolvedTarget); - return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative)); + return isPathInside(rootDir, targetPath); } diff --git a/src/infra/update-runner.test.ts b/src/infra/update-runner.test.ts index bb301c563ca..2ad84305794 100644 --- a/src/infra/update-runner.test.ts +++ b/src/infra/update-runner.test.ts @@ -123,32 +123,34 @@ describe("runGatewayUpdate", () => { return uiIndexPath; } - function buildStableTagResponses(stableTag: string): Record { + function buildStableTagResponses( + stableTag: string, + options?: { additionalTags?: string[] }, + ): Record { + const tagOutput = [stableTag, ...(options?.additionalTags ?? [])].join("\n"); return { [`git -C ${tempDir} rev-parse --show-toplevel`]: { stdout: tempDir }, [`git -C ${tempDir} rev-parse HEAD`]: { stdout: "abc123" }, [`git -C ${tempDir} status --porcelain -- :!dist/control-ui/`]: { stdout: "" }, [`git -C ${tempDir} fetch --all --prune --tags`]: { stdout: "" }, - [`git -C ${tempDir} tag --list v* --sort=-v:refname`]: { stdout: `${stableTag}\n` }, + [`git -C ${tempDir} tag --list v* --sort=-v:refname`]: { stdout: `${tagOutput}\n` }, [`git -C ${tempDir} checkout --detach ${stableTag}`]: { stdout: "" }, }; } - async function removeControlUiAssets() { - await fs.rm(path.join(tempDir, "dist", "control-ui"), { recursive: true, force: true }); + function buildGitWorktreeProbeResponses(options?: { status?: string; branch?: string }) { + return { + [`git -C ${tempDir} rev-parse --show-toplevel`]: { stdout: tempDir }, + [`git -C ${tempDir} rev-parse HEAD`]: { stdout: "abc123" }, + [`git -C ${tempDir} rev-parse --abbrev-ref HEAD`]: { stdout: options?.branch ?? "main" }, + [`git -C ${tempDir} status --porcelain -- :!dist/control-ui/`]: { + stdout: options?.status ?? "", + }, + } satisfies Record; } - async function runWithRunner( - runner: (argv: string[]) => Promise, - options?: { channel?: "stable" | "beta"; tag?: string; cwd?: string }, - ) { - return runGatewayUpdate({ - cwd: options?.cwd ?? tempDir, - runCommand: async (argv, _runOptions) => runner(argv), - timeoutMs: 5000, - ...(options?.channel ? { channel: options.channel } : {}), - ...(options?.tag ? { tag: options.tag } : {}), - }); + async function removeControlUiAssets() { + await fs.rm(path.join(tempDir, "dist", "control-ui"), { recursive: true, force: true }); } async function runWithCommand( @@ -164,6 +166,13 @@ describe("runGatewayUpdate", () => { }); } + async function runWithRunner( + runner: (argv: string[]) => Promise, + options?: { channel?: "stable" | "beta"; tag?: string; cwd?: string }, + ) { + return runWithCommand(runner, options); + } + async function seedGlobalPackageRoot(pkgRoot: string, version = "1.0.0") { await fs.mkdir(pkgRoot, { recursive: true }); await fs.writeFile( @@ -176,10 +185,7 @@ describe("runGatewayUpdate", () => { it("skips git update when worktree is dirty", async () => { await setupGitCheckout(); const { runner, calls } = createRunner({ - [`git -C ${tempDir} rev-parse --show-toplevel`]: { stdout: tempDir }, - [`git -C ${tempDir} rev-parse HEAD`]: { stdout: "abc123" }, - [`git -C ${tempDir} rev-parse --abbrev-ref HEAD`]: { stdout: "main" }, - [`git -C ${tempDir} status --porcelain -- :!dist/control-ui/`]: { stdout: " M README.md" }, + ...buildGitWorktreeProbeResponses({ status: " M README.md" }), }); const result = await runWithRunner(runner); @@ -192,10 +198,7 @@ describe("runGatewayUpdate", () => { it("aborts rebase on failure", async () => { await setupGitCheckout(); const { runner, calls } = createRunner({ - [`git -C ${tempDir} rev-parse --show-toplevel`]: { stdout: tempDir }, - [`git -C ${tempDir} rev-parse HEAD`]: { stdout: "abc123" }, - [`git -C ${tempDir} rev-parse --abbrev-ref HEAD`]: { stdout: "main" }, - [`git -C ${tempDir} status --porcelain -- :!dist/control-ui/`]: { stdout: "" }, + ...buildGitWorktreeProbeResponses(), [`git -C ${tempDir} rev-parse --abbrev-ref --symbolic-full-name @{upstream}`]: { stdout: "origin/main", }, @@ -252,14 +255,7 @@ describe("runGatewayUpdate", () => { const stableTag = "v1.0.1-1"; const betaTag = "v1.0.0-beta.2"; const { runner, calls } = createRunner({ - [`git -C ${tempDir} rev-parse --show-toplevel`]: { stdout: tempDir }, - [`git -C ${tempDir} rev-parse HEAD`]: { stdout: "abc123" }, - [`git -C ${tempDir} status --porcelain -- :!dist/control-ui/`]: { stdout: "" }, - [`git -C ${tempDir} fetch --all --prune --tags`]: { stdout: "" }, - [`git -C ${tempDir} tag --list v* --sort=-v:refname`]: { - stdout: `${stableTag}\n${betaTag}\n`, - }, - [`git -C ${tempDir} checkout --detach ${stableTag}`]: { stdout: "" }, + ...buildStableTagResponses(stableTag, { additionalTags: [betaTag] }), "pnpm install": { stdout: "" }, "pnpm build": { stdout: "" }, "pnpm ui:build": { stdout: "" }, @@ -472,12 +468,7 @@ describe("runGatewayUpdate", () => { const stableTag = "v1.0.1-1"; const { runner } = createRunner({ - [`git -C ${tempDir} rev-parse --show-toplevel`]: { stdout: tempDir }, - [`git -C ${tempDir} rev-parse HEAD`]: { stdout: "abc123" }, - [`git -C ${tempDir} status --porcelain -- :!dist/control-ui/`]: { stdout: "" }, - [`git -C ${tempDir} fetch --all --prune --tags`]: { stdout: "" }, - [`git -C ${tempDir} tag --list v* --sort=-v:refname`]: { stdout: `${stableTag}\n` }, - [`git -C ${tempDir} checkout --detach ${stableTag}`]: { stdout: "" }, + ...buildStableTagResponses(stableTag), "pnpm install": { stdout: "" }, "pnpm build": { stdout: "" }, "pnpm ui:build": { stdout: "" }, diff --git a/src/process/supervisor/adapters/child.ts b/src/process/supervisor/adapters/child.ts index 3229516b65f..a6db4329336 100644 --- a/src/process/supervisor/adapters/child.ts +++ b/src/process/supervisor/adapters/child.ts @@ -1,7 +1,7 @@ import type { ChildProcessWithoutNullStreams, SpawnOptions } from "node:child_process"; import { killProcessTree } from "../../kill-tree.js"; import { spawnWithFallback } from "../../spawn-utils.js"; -import type { ManagedRunStdin } from "../types.js"; +import type { ManagedRunStdin, SpawnProcessAdapter } from "../types.js"; import { toStringEnv } from "./env.js"; function resolveCommand(command: string): string { @@ -19,15 +19,7 @@ function resolveCommand(command: string): string { return command; } -export type ChildAdapter = { - pid?: number; - stdin?: ManagedRunStdin; - onStdout: (listener: (chunk: string) => void) => void; - onStderr: (listener: (chunk: string) => void) => void; - wait: () => Promise<{ code: number | null; signal: NodeJS.Signals | null }>; - kill: (signal?: NodeJS.Signals) => void; - dispose: () => void; -}; +export type ChildAdapter = SpawnProcessAdapter; export async function createChildAdapter(params: { argv: string[]; diff --git a/src/process/supervisor/adapters/pty.ts b/src/process/supervisor/adapters/pty.ts index 40b0bc2ce72..8226e83a215 100644 --- a/src/process/supervisor/adapters/pty.ts +++ b/src/process/supervisor/adapters/pty.ts @@ -1,5 +1,5 @@ import { killProcessTree } from "../../kill-tree.js"; -import type { ManagedRunStdin } from "../types.js"; +import type { ManagedRunStdin, SpawnProcessAdapter } from "../types.js"; import { toStringEnv } from "./env.js"; const FORCE_KILL_WAIT_FALLBACK_MS = 4000; @@ -32,15 +32,7 @@ type PtyModule = { }; }; -export type PtyAdapter = { - pid?: number; - stdin?: ManagedRunStdin; - onStdout: (listener: (chunk: string) => void) => void; - onStderr: (listener: (chunk: string) => void) => void; - wait: () => Promise<{ code: number | null; signal: NodeJS.Signals | number | null }>; - kill: (signal?: NodeJS.Signals) => void; - dispose: () => void; -}; +export type PtyAdapter = SpawnProcessAdapter; export async function createPtyAdapter(params: { shell: string; diff --git a/src/process/supervisor/types.ts b/src/process/supervisor/types.ts index 04c571b08b2..7bb0f39c1df 100644 --- a/src/process/supervisor/types.ts +++ b/src/process/supervisor/types.ts @@ -54,6 +54,16 @@ export type ManagedRunStdin = { destroyed?: boolean; }; +export type SpawnProcessAdapter = { + pid?: number; + stdin?: ManagedRunStdin; + onStdout: (listener: (chunk: string) => void) => void; + onStderr: (listener: (chunk: string) => void) => void; + wait: () => Promise<{ code: number | null; signal: WaitSignal }>; + kill: (signal?: NodeJS.Signals) => void; + dispose: () => void; +}; + type SpawnBaseInput = { runId?: string; sessionId: string; diff --git a/src/routing/session-key.ts b/src/routing/session-key.ts index 67494ab15ec..77429870797 100644 --- a/src/routing/session-key.ts +++ b/src/routing/session-key.ts @@ -99,21 +99,7 @@ export function normalizeAgentId(value: string | undefined | null): string { } export function sanitizeAgentId(value: string | undefined | null): string { - const trimmed = (value ?? "").trim(); - if (!trimmed) { - return DEFAULT_AGENT_ID; - } - if (VALID_ID_RE.test(trimmed)) { - return trimmed.toLowerCase(); - } - return ( - trimmed - .toLowerCase() - .replace(INVALID_CHARS_RE, "-") - .replace(LEADING_DASH_RE, "") - .replace(TRAILING_DASH_RE, "") - .slice(0, 64) || DEFAULT_AGENT_ID - ); + return normalizeAgentId(value); } export function buildAgentMainSessionKey(params: {