diff --git a/src/config/config.legacy-config-detection.accepts-imessage-dmpolicy.e2e.test.ts b/src/config/config.legacy-config-detection.accepts-imessage-dmpolicy.e2e.test.ts index 2ba26ac6de3..edaf05e362c 100644 --- a/src/config/config.legacy-config-detection.accepts-imessage-dmpolicy.e2e.test.ts +++ b/src/config/config.legacy-config-detection.accepts-imessage-dmpolicy.e2e.test.ts @@ -6,6 +6,26 @@ const { loadConfig, migrateLegacyConfig, readConfigFileSnapshot, validateConfigO await vi.importActual("./config.js"); import { withTempHome } from "./test-helpers.js"; +async function expectLoadRejectionPreservesField(params: { + config: unknown; + readValue: (parsed: unknown) => unknown; + expectedValue: unknown; +}) { + 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(params.config, null, 2), "utf-8"); + + const snap = await readConfigFileSnapshot(); + + expect(snap.valid).toBe(false); + expect(snap.issues.length).toBeGreaterThan(0); + + const parsed = JSON.parse(await fs.readFile(configPath, "utf-8")) as unknown; + expect(params.readValue(parsed)).toBe(params.expectedValue); + }); +} + describe("legacy config detection", () => { it('accepts imessage.dmPolicy="open" with allowFrom "*"', async () => { const res = validateConfigObject({ @@ -296,59 +316,25 @@ describe("legacy config detection", () => { }); }); it("rejects bindings[].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( - { - bindings: [{ agentId: "main", match: { provider: "slack" } }], - }, - 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 { - bindings?: Array<{ match?: { provider?: string } }>; - }; - expect(parsed.bindings?.[0]?.match?.provider).toBe("slack"); + await expectLoadRejectionPreservesField({ + config: { + bindings: [{ agentId: "main", match: { provider: "slack" } }], + }, + readValue: (parsed) => + (parsed as { bindings?: Array<{ match?: { provider?: string } }> }).bindings?.[0]?.match + ?.provider, + expectedValue: "slack", }); }); it("rejects bindings[].match.accountID 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( - { - bindings: [{ agentId: "main", match: { channel: "telegram", accountID: "work" } }], - }, - 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 { - bindings?: Array<{ match?: { accountID?: string } }>; - }; - expect(parsed.bindings?.[0]?.match?.accountID).toBe("work"); + await expectLoadRejectionPreservesField({ + config: { + bindings: [{ agentId: "main", match: { channel: "telegram", accountID: "work" } }], + }, + readValue: (parsed) => + (parsed as { bindings?: Array<{ match?: { accountID?: string } }> }).bindings?.[0]?.match + ?.accountID, + expectedValue: "work", }); }); it("rejects session.sendPolicy.rules[].match.provider on load", async () => { diff --git a/src/config/config.pruning-defaults.test.ts b/src/config/config.pruning-defaults.test.ts index 00320e618c5..b6a0c4563d3 100644 --- a/src/config/config.pruning-defaults.test.ts +++ b/src/config/config.pruning-defaults.test.ts @@ -4,6 +4,16 @@ import { describe, expect, it } from "vitest"; import { loadConfig } from "./config.js"; import { withTempHome } from "./test-helpers.js"; +async function writeConfigForTest(home: string, config: unknown): Promise { + const configDir = path.join(home, ".openclaw"); + await fs.mkdir(configDir, { recursive: true }); + await fs.writeFile( + path.join(configDir, "openclaw.json"), + JSON.stringify(config, null, 2), + "utf-8", + ); +} + describe("config pruning defaults", () => { it("does not enable contextPruning by default", async () => { const prevApiKey = process.env.ANTHROPIC_API_KEY; @@ -11,13 +21,7 @@ describe("config pruning defaults", () => { process.env.ANTHROPIC_API_KEY = ""; process.env.ANTHROPIC_OAUTH_TOKEN = ""; 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: {} } }, null, 2), - "utf-8", - ); + await writeConfigForTest(home, { agents: { defaults: {} } }); const cfg = loadConfig(); @@ -37,24 +41,14 @@ describe("config pruning defaults", () => { it("enables cache-ttl pruning + 1h heartbeat for Anthropic OAuth", 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( - { - auth: { - profiles: { - "anthropic:me": { provider: "anthropic", mode: "oauth", email: "me@example.com" }, - }, - }, - agents: { defaults: {} }, + await writeConfigForTest(home, { + auth: { + profiles: { + "anthropic:me": { provider: "anthropic", mode: "oauth", email: "me@example.com" }, }, - null, - 2, - ), - "utf-8", - ); + }, + agents: { defaults: {} }, + }); const cfg = loadConfig(); @@ -66,28 +60,18 @@ describe("config pruning defaults", () => { it("enables cache-ttl pruning + 1h cache TTL for Anthropic API keys", 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( - { - auth: { - profiles: { - "anthropic:api": { provider: "anthropic", mode: "api_key" }, - }, - }, - agents: { - defaults: { - model: { primary: "anthropic/claude-opus-4-5" }, - }, - }, + await writeConfigForTest(home, { + auth: { + profiles: { + "anthropic:api": { provider: "anthropic", mode: "api_key" }, }, - null, - 2, - ), - "utf-8", - ); + }, + agents: { + defaults: { + model: { primary: "anthropic/claude-opus-4-5" }, + }, + }, + }); const cfg = loadConfig(); @@ -102,13 +86,7 @@ describe("config pruning defaults", () => { it("does not override explicit contextPruning mode", 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: { contextPruning: { mode: "off" } } } }, null, 2), - "utf-8", - ); + await writeConfigForTest(home, { agents: { defaults: { contextPruning: { mode: "off" } } } }); const cfg = loadConfig(); diff --git a/src/config/env-preserve-io.test.ts b/src/config/env-preserve-io.test.ts index 5e2d29a31bf..9e94a704091 100644 --- a/src/config/env-preserve-io.test.ts +++ b/src/config/env-preserve-io.test.ts @@ -51,6 +51,23 @@ async function withEnvOverrides( } } +async function withWrapperEnvContext(configPath: string, run: () => Promise): Promise { + await withEnvOverrides( + { + OPENCLAW_CONFIG_PATH: configPath, + OPENCLAW_DISABLE_CONFIG_CACHE: "1", + MY_API_KEY: "original-key-123", + }, + run, + ); +} + +async function readGatewayToken(configPath: string): Promise { + const written = await fs.readFile(configPath, "utf-8"); + const parsed = JSON.parse(written) as { gateway: { remote: { token: string } } }; + return parsed.gateway.remote.token; +} + describe("env snapshot TOCTOU via createConfigIO", () => { it("restores env refs using read-time env even after env mutation", async () => { const env: Record = { @@ -117,56 +134,38 @@ 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 withEnvOverrides( - { - OPENCLAW_CONFIG_PATH: configPath, - OPENCLAW_DISABLE_CONFIG_CACHE: "1", - MY_API_KEY: "original-key-123", - }, - async () => { - const firstRead = await readConfigFileSnapshotForWrite(); - expect(firstRead.snapshot.config.gateway?.remote?.token).toBe("original-key-123"); + await withWrapperEnvContext(configPath, async () => { + 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); - const written = await fs.readFile(configPath, "utf-8"); - const parsed = JSON.parse(written); - expect(parsed.gateway.remote.token).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 withEnvOverrides( - { - OPENCLAW_CONFIG_PATH: configPath, - OPENCLAW_DISABLE_CONFIG_CACHE: "1", - MY_API_KEY: "original-key-123", - }, - async () => { - const firstRead = await readConfigFileSnapshotForWrite(); - expect(firstRead.snapshot.config.gateway?.remote?.token).toBe("original-key-123"); - expect(firstRead.writeOptions.expectedConfigPath).toBe(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); - process.env.MY_API_KEY = "mutated-key-456"; - await writeConfigFileViaWrapper(firstRead.snapshot.config, { - ...firstRead.writeOptions, - expectedConfigPath: `${configPath}.different`, - }); + process.env.MY_API_KEY = "mutated-key-456"; + await writeConfigFileViaWrapper(firstRead.snapshot.config, { + ...firstRead.writeOptions, + expectedConfigPath: `${configPath}.different`, + }); - const written = await fs.readFile(configPath, "utf-8"); - const parsed = JSON.parse(written); - expect(parsed.gateway.remote.token).toBe("original-key-123"); - }, - ); + expect(await readGatewayToken(configPath)).toBe("original-key-123"); + }); }); }); }); diff --git a/src/config/env-substitution.ts b/src/config/env-substitution.ts index 306be869c05..0c1b7e02603 100644 --- a/src/config/env-substitution.ts +++ b/src/config/env-substitution.ts @@ -36,6 +36,45 @@ export class MissingEnvVarError extends Error { } } +type EnvToken = + | { kind: "escaped"; name: string; end: number } + | { kind: "substitution"; name: string; end: number }; + +function parseEnvTokenAt(value: string, index: number): EnvToken | null { + if (value[index] !== "$") { + return null; + } + + const next = value[index + 1]; + const afterNext = value[index + 2]; + + // Escaped: $${VAR} -> ${VAR} + if (next === "$" && afterNext === "{") { + const start = index + 3; + const end = value.indexOf("}", start); + if (end !== -1) { + const name = value.slice(start, end); + if (ENV_VAR_NAME_PATTERN.test(name)) { + return { kind: "escaped", name, end }; + } + } + } + + // Substitution: ${VAR} -> value + if (next === "{") { + const start = index + 2; + const end = value.indexOf("}", start); + if (end !== -1) { + const name = value.slice(start, end); + if (ENV_VAR_NAME_PATTERN.test(name)) { + return { kind: "substitution", name, end }; + } + } + } + + return null; +} + function substituteString(value: string, env: NodeJS.ProcessEnv, configPath: string): string { if (!value.includes("$")) { return value; @@ -50,39 +89,20 @@ function substituteString(value: string, env: NodeJS.ProcessEnv, configPath: str continue; } - const next = value[i + 1]; - const afterNext = value[i + 2]; - - // Escaped: $${VAR} -> ${VAR} - if (next === "$" && afterNext === "{") { - const start = i + 3; - const end = value.indexOf("}", start); - if (end !== -1) { - const name = value.slice(start, end); - if (ENV_VAR_NAME_PATTERN.test(name)) { - chunks.push(`\${${name}}`); - i = end; - continue; - } - } + const token = parseEnvTokenAt(value, i); + if (token?.kind === "escaped") { + chunks.push(`\${${token.name}}`); + i = token.end; + continue; } - - // Substitution: ${VAR} -> value - if (next === "{") { - const start = i + 2; - const end = value.indexOf("}", start); - if (end !== -1) { - const name = value.slice(start, end); - if (ENV_VAR_NAME_PATTERN.test(name)) { - const envValue = env[name]; - if (envValue === undefined || envValue === "") { - throw new MissingEnvVarError(name, configPath); - } - chunks.push(envValue); - i = end; - continue; - } + if (token?.kind === "substitution") { + const envValue = env[token.name]; + if (envValue === undefined || envValue === "") { + throw new MissingEnvVarError(token.name, configPath); } + chunks.push(envValue); + i = token.end; + continue; } // Leave untouched if not a recognized pattern @@ -103,32 +123,13 @@ export function containsEnvVarReference(value: string): boolean { continue; } - const next = value[i + 1]; - const afterNext = value[i + 2]; - - // Escaped: $${VAR} -> ${VAR} - if (next === "$" && afterNext === "{") { - const start = i + 3; - const end = value.indexOf("}", start); - if (end !== -1) { - const name = value.slice(start, end); - if (ENV_VAR_NAME_PATTERN.test(name)) { - i = end; - continue; - } - } + const token = parseEnvTokenAt(value, i); + if (token?.kind === "escaped") { + i = token.end; + continue; } - - // Substitution: ${VAR} -> value - if (next === "{") { - const start = i + 2; - const end = value.indexOf("}", start); - if (end !== -1) { - const name = value.slice(start, end); - if (ENV_VAR_NAME_PATTERN.test(name)) { - return true; - } - } + if (token?.kind === "substitution") { + return true; } } diff --git a/src/config/io.write-config.test.ts b/src/config/io.write-config.test.ts index 04f5e34a77f..a926869d033 100644 --- a/src/config/io.write-config.test.ts +++ b/src/config/io.write-config.test.ts @@ -1,8 +1,7 @@ import fs from "node:fs/promises"; -import os from "node:os"; import path from "node:path"; -import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; -import { captureEnv } from "../test-utils/env.js"; +import { describe, expect, it, vi } from "vitest"; +import { withTempHome } from "./home-env.test-harness.js"; import { createConfigIO } from "./io.js"; describe("config io write", () => { @@ -11,70 +10,32 @@ describe("config io write", () => { error: () => {}, }; - let fixtureRoot = ""; - let caseId = 0; + async function writeConfigAndCreateIo(params: { + home: string; + initialConfig: Record; + env?: NodeJS.ProcessEnv; + }) { + const configPath = path.join(params.home, ".openclaw", "openclaw.json"); + await fs.mkdir(path.dirname(configPath), { recursive: true }); + await fs.writeFile(configPath, JSON.stringify(params.initialConfig, null, 2), "utf-8"); - async function withTempHome(prefix: string, fn: (home: string) => Promise): Promise { - const safePrefix = prefix.trim().replace(/[^a-zA-Z0-9._-]+/g, "-") || "tmp"; - const home = path.join(fixtureRoot, `${safePrefix}${caseId++}`); - await fs.mkdir(path.join(home, ".openclaw"), { recursive: true }); - - const snapshot = captureEnv([ - "HOME", - "USERPROFILE", - "HOMEDRIVE", - "HOMEPATH", - "OPENCLAW_STATE_DIR", - ]); - process.env.HOME = home; - process.env.USERPROFILE = home; - process.env.OPENCLAW_STATE_DIR = path.join(home, ".openclaw"); - - if (process.platform === "win32") { - const match = home.match(/^([A-Za-z]:)(.*)$/); - if (match) { - process.env.HOMEDRIVE = match[1]; - process.env.HOMEPATH = match[2] || "\\"; - } - } - - try { - await fn(home); - } finally { - snapshot.restore(); - } + const io = createConfigIO({ + env: params.env ?? {}, + homedir: () => params.home, + logger: silentLogger, + }); + const snapshot = await io.readConfigFileSnapshot(); + expect(snapshot.valid).toBe(true); + return { configPath, io, snapshot }; } - beforeAll(async () => { - fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-config-io-")); - }); - - afterAll(async () => { - if (!fixtureRoot) { - return; - } - await fs.rm(fixtureRoot, { recursive: true, force: true }); - }); - it("persists caller changes onto resolved config without leaking runtime defaults", async () => { await withTempHome("openclaw-config-io-", async (home) => { - const configPath = path.join(home, ".openclaw", "openclaw.json"); - await fs.mkdir(path.dirname(configPath), { recursive: true }); - await fs.writeFile( - configPath, - JSON.stringify({ gateway: { port: 18789 } }, null, 2), - "utf-8", - ); - - const io = createConfigIO({ - env: {} as NodeJS.ProcessEnv, - homedir: () => home, - logger: silentLogger, + const { configPath, io, snapshot } = await writeConfigAndCreateIo({ + home, + initialConfig: { gateway: { port: 18789 } }, }); - const snapshot = await io.readConfigFileSnapshot(); - expect(snapshot.valid).toBe(true); - const next = structuredClone(snapshot.config); next.gateway = { ...next.gateway, @@ -99,41 +60,26 @@ describe("config io write", () => { it("preserves env var references when writing", async () => { await withTempHome("openclaw-config-io-", async (home) => { - const configPath = path.join(home, ".openclaw", "openclaw.json"); - await fs.mkdir(path.dirname(configPath), { recursive: true }); - await fs.writeFile( - configPath, - JSON.stringify( - { - agents: { - defaults: { - cliBackends: { - codex: { - command: "codex", - env: { - OPENAI_API_KEY: "${OPENAI_API_KEY}", - }, + const { configPath, io, snapshot } = await writeConfigAndCreateIo({ + home, + env: { OPENAI_API_KEY: "sk-secret" } as NodeJS.ProcessEnv, + initialConfig: { + agents: { + defaults: { + cliBackends: { + codex: { + command: "codex", + env: { + OPENAI_API_KEY: "${OPENAI_API_KEY}", }, }, }, }, - gateway: { port: 18789 }, }, - null, - 2, - ), - "utf-8", - ); - - const io = createConfigIO({ - env: { OPENAI_API_KEY: "sk-secret" } as NodeJS.ProcessEnv, - homedir: () => home, - logger: silentLogger, + gateway: { port: 18789 }, + }, }); - const snapshot = await io.readConfigFileSnapshot(); - expect(snapshot.valid).toBe(true); - const next = structuredClone(snapshot.config); next.gateway = { ...next.gateway, @@ -158,39 +104,23 @@ describe("config io write", () => { it("does not reintroduce Slack/Discord legacy dm.policy defaults when writing", async () => { await withTempHome("openclaw-config-io-", async (home) => { - const configPath = path.join(home, ".openclaw", "openclaw.json"); - await fs.mkdir(path.dirname(configPath), { recursive: true }); - await fs.writeFile( - configPath, - JSON.stringify( - { - channels: { - discord: { - dmPolicy: "pairing", - dm: { enabled: true, policy: "pairing" }, - }, - slack: { - dmPolicy: "pairing", - dm: { enabled: true, policy: "pairing" }, - }, + const { configPath, io, snapshot } = await writeConfigAndCreateIo({ + home, + initialConfig: { + channels: { + discord: { + dmPolicy: "pairing", + dm: { enabled: true, policy: "pairing" }, + }, + slack: { + dmPolicy: "pairing", + dm: { enabled: true, policy: "pairing" }, }, - gateway: { port: 18789 }, }, - null, - 2, - ), - "utf-8", - ); - - const io = createConfigIO({ - env: {} as NodeJS.ProcessEnv, - homedir: () => home, - logger: silentLogger, + gateway: { port: 18789 }, + }, }); - const snapshot = await io.readConfigFileSnapshot(); - expect(snapshot.valid).toBe(true); - const next = structuredClone(snapshot.config); // Simulate doctor removing legacy keys while keeping dm enabled. if (next.channels?.discord?.dm && typeof next.channels.discord.dm === "object") { diff --git a/src/config/merge-patch.test.ts b/src/config/merge-patch.test.ts index d170fedf5ee..4097638eff2 100644 --- a/src/config/merge-patch.test.ts +++ b/src/config/merge-patch.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from "vitest"; import { applyMergePatch } from "./merge-patch.js"; describe("applyMergePatch", () => { - it("replaces arrays by default", () => { + function makeAgentListBaseAndPatch() { const base = { agents: { list: [ @@ -16,6 +16,11 @@ describe("applyMergePatch", () => { list: [{ id: "primary", memorySearch: { extraPaths: ["/tmp/memory.md"] } }], }, }; + return { base, patch }; + } + + it("replaces arrays by default", () => { + const { base, patch } = makeAgentListBaseAndPatch(); const merged = applyMergePatch(base, patch) as { agents?: { list?: Array<{ id?: string; workspace?: string }> }; @@ -26,19 +31,7 @@ describe("applyMergePatch", () => { }); it("merges object arrays by id when enabled", () => { - const base = { - agents: { - list: [ - { id: "primary", workspace: "/tmp/one" }, - { id: "secondary", workspace: "/tmp/two" }, - ], - }, - }; - const patch = { - agents: { - list: [{ id: "primary", memorySearch: { extraPaths: ["/tmp/memory.md"] } }], - }, - }; + const { base, patch } = makeAgentListBaseAndPatch(); const merged = applyMergePatch(base, patch, { mergeObjectArraysById: true, diff --git a/src/config/redact-snapshot.ts b/src/config/redact-snapshot.ts index 02db6d13261..f384e273e5d 100644 --- a/src/config/redact-snapshot.ts +++ b/src/config/redact-snapshot.ts @@ -373,6 +373,18 @@ class RedactionError extends Error { } } +function restoreOriginalValueOrThrow(params: { + key: string; + path: string; + original: Record; +}): unknown { + if (params.key in params.original) { + return params.original[params.key]; + } + log.warn(`Cannot un-redact config key ${params.path} as it doesn't have any value`); + throw new RedactionError(params.path); +} + /** * Worker for restoreRedactedValues(). * Used when there are ConfigUiHints available. @@ -427,12 +439,7 @@ function restoreRedactedValuesWithLookup( if (lookup.has(candidate)) { matched = true; if (value === REDACTED_SENTINEL) { - if (key in orig) { - result[key] = orig[key]; - } else { - log.warn(`Cannot un-redact config key ${candidate} as it doesn't have any value`); - throw new RedactionError(candidate); - } + result[key] = restoreOriginalValueOrThrow({ key, path: candidate, original: orig }); } else if (typeof value === "object" && value !== null) { result[key] = restoreRedactedValuesWithLookup(value, orig[key], lookup, candidate, hints); } @@ -442,12 +449,7 @@ function restoreRedactedValuesWithLookup( if (!matched && isExtensionPath(path)) { const markedNonSensitive = isExplicitlyNonSensitivePath(hints, [path, wildcardPath]); if (!markedNonSensitive && isSensitivePath(path) && value === REDACTED_SENTINEL) { - if (key in orig) { - result[key] = orig[key]; - } else { - log.warn(`Cannot un-redact config key ${path} as it doesn't have any value`); - throw new RedactionError(path); - } + result[key] = restoreOriginalValueOrThrow({ key, path, original: orig }); } else if (typeof value === "object" && value !== null) { result[key] = restoreRedactedValuesGuessing(value, orig[key], path, hints); } @@ -506,12 +508,7 @@ function restoreRedactedValuesGuessing( isSensitivePath(path) && value === REDACTED_SENTINEL ) { - if (key in orig) { - result[key] = orig[key]; - } else { - log.warn(`Cannot un-redact config key ${path} as it doesn't have any value`); - throw new RedactionError(path); - } + result[key] = restoreOriginalValueOrThrow({ key, path, original: orig }); } else if (typeof value === "object" && value !== null) { result[key] = restoreRedactedValuesGuessing(value, orig[key], path, hints); } else { diff --git a/src/config/zod-schema.agent-runtime.ts b/src/config/zod-schema.agent-runtime.ts index 52f768ff24f..61794fc6fb2 100644 --- a/src/config/zod-schema.agent-runtime.ts +++ b/src/config/zod-schema.agent-runtime.ts @@ -276,6 +276,24 @@ export const ToolProfileSchema = z .union([z.literal("minimal"), z.literal("coding"), z.literal("messaging"), z.literal("full")]) .optional(); +type AllowlistPolicy = { + allow?: string[]; + alsoAllow?: string[]; +}; + +function addAllowAlsoAllowConflictIssue( + value: AllowlistPolicy, + ctx: z.RefinementCtx, + message: string, +): void { + if (value.allow && value.allow.length > 0 && value.alsoAllow && value.alsoAllow.length > 0) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message, + }); + } +} + export const ToolPolicyWithProfileSchema = z .object({ allow: z.array(z.string()).optional(), @@ -285,13 +303,11 @@ export const ToolPolicyWithProfileSchema = z }) .strict() .superRefine((value, ctx) => { - if (value.allow && value.allow.length > 0 && value.alsoAllow && value.alsoAllow.length > 0) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: - "tools.byProvider policy cannot set both allow and alsoAllow in the same scope (merge alsoAllow into allow, or remove allow and use profile + alsoAllow)", - }); - } + addAllowAlsoAllowConflictIssue( + value, + ctx, + "tools.byProvider policy cannot set both allow and alsoAllow in the same scope (merge alsoAllow into allow, or remove allow and use profile + alsoAllow)", + ); }); // Provider docking: allowlists keyed by provider id (no schema updates when adding providers). @@ -380,13 +396,11 @@ export const AgentToolsSchema = z }) .strict() .superRefine((value, ctx) => { - if (value.allow && value.allow.length > 0 && value.alsoAllow && value.alsoAllow.length > 0) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: - "agent tools cannot set both allow and alsoAllow in the same scope (merge alsoAllow into allow, or remove allow and use profile + alsoAllow)", - }); - } + addAllowAlsoAllowConflictIssue( + value, + ctx, + "agent tools cannot set both allow and alsoAllow in the same scope (merge alsoAllow into allow, or remove allow and use profile + alsoAllow)", + ); }) .optional(); @@ -615,12 +629,10 @@ export const ToolsSchema = z }) .strict() .superRefine((value, ctx) => { - if (value.allow && value.allow.length > 0 && value.alsoAllow && value.alsoAllow.length > 0) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: - "tools cannot set both allow and alsoAllow in the same scope (merge alsoAllow into allow, or remove allow and use profile + alsoAllow)", - }); - } + addAllowAlsoAllowConflictIssue( + value, + ctx, + "tools cannot set both allow and alsoAllow in the same scope (merge alsoAllow into allow, or remove allow and use profile + alsoAllow)", + ); }) .optional(); diff --git a/src/hooks/bundled/bootstrap-extra-files/handler.test.ts b/src/hooks/bundled/bootstrap-extra-files/handler.test.ts index 2b945ad07a5..85d39a1d644 100644 --- a/src/hooks/bundled/bootstrap-extra-files/handler.test.ts +++ b/src/hooks/bundled/bootstrap-extra-files/handler.test.ts @@ -7,6 +7,47 @@ import { makeTempWorkspace, writeWorkspaceFile } from "../../../test-helpers/wor import { createHookEvent } from "../../hooks.js"; import handler from "./handler.js"; +function createBootstrapExtraConfig(paths: string[]): OpenClawConfig { + return { + hooks: { + internal: { + entries: { + "bootstrap-extra-files": { + enabled: true, + paths, + }, + }, + }, + }, + }; +} + +async function createBootstrapContext(params: { + workspaceDir: string; + cfg: OpenClawConfig; + sessionKey: string; + rootFiles: Array<{ name: string; content: string }>; +}): Promise { + const bootstrapFiles = await Promise.all( + params.rootFiles.map(async (file) => ({ + name: file.name, + path: await writeWorkspaceFile({ + dir: params.workspaceDir, + name: file.name, + content: file.content, + }), + content: file.content, + missing: false, + })), + ); + return { + workspaceDir: params.workspaceDir, + bootstrapFiles, + cfg: params.cfg, + sessionKey: params.sessionKey, + }; +} + describe("bootstrap-extra-files hook", () => { it("appends extra bootstrap files from configured patterns", async () => { const tempDir = await makeTempWorkspace("openclaw-bootstrap-extra-"); @@ -14,36 +55,13 @@ describe("bootstrap-extra-files hook", () => { await fs.mkdir(extraDir, { recursive: true }); await fs.writeFile(path.join(extraDir, "AGENTS.md"), "extra agents", "utf-8"); - const cfg: OpenClawConfig = { - hooks: { - internal: { - entries: { - "bootstrap-extra-files": { - enabled: true, - paths: ["packages/*/AGENTS.md"], - }, - }, - }, - }, - }; - - const context: AgentBootstrapHookContext = { + const cfg = createBootstrapExtraConfig(["packages/*/AGENTS.md"]); + const context = await createBootstrapContext({ workspaceDir: tempDir, - bootstrapFiles: [ - { - name: "AGENTS.md", - path: await writeWorkspaceFile({ - dir: tempDir, - name: "AGENTS.md", - content: "root agents", - }), - content: "root agents", - missing: false, - }, - ], cfg, sessionKey: "agent:main:main", - }; + rootFiles: [{ name: "AGENTS.md", content: "root agents" }], + }); const event = createHookEvent("agent", "bootstrap", "agent:main:main", context); await handler(event); @@ -61,42 +79,16 @@ describe("bootstrap-extra-files hook", () => { await fs.mkdir(extraDir, { recursive: true }); await fs.writeFile(path.join(extraDir, "SOUL.md"), "evil", "utf-8"); - const cfg: OpenClawConfig = { - hooks: { - internal: { - entries: { - "bootstrap-extra-files": { - enabled: true, - paths: ["packages/*/SOUL.md"], - }, - }, - }, - }, - }; - - const context: AgentBootstrapHookContext = { + const cfg = createBootstrapExtraConfig(["packages/*/SOUL.md"]); + const context = await createBootstrapContext({ workspaceDir: tempDir, - bootstrapFiles: [ - { - name: "AGENTS.md", - path: await writeWorkspaceFile({ - dir: tempDir, - name: "AGENTS.md", - content: "root agents", - }), - content: "root agents", - missing: false, - }, - { - name: "TOOLS.md", - path: await writeWorkspaceFile({ dir: tempDir, name: "TOOLS.md", content: "root tools" }), - content: "root tools", - missing: false, - }, - ], cfg, sessionKey: "agent:main:subagent:abc", - }; + rootFiles: [ + { name: "AGENTS.md", content: "root agents" }, + { name: "TOOLS.md", content: "root tools" }, + ], + }); const event = createHookEvent("agent", "bootstrap", "agent:main:subagent:abc", context); await handler(event); diff --git a/src/hooks/frontmatter.ts b/src/hooks/frontmatter.ts index 3cfc1d67e27..de9e49c723e 100644 --- a/src/hooks/frontmatter.ts +++ b/src/hooks/frontmatter.ts @@ -9,6 +9,7 @@ import { parseFrontmatterBlock } from "../markdown/frontmatter.js"; import { getFrontmatterString, normalizeStringList, + parseOpenClawManifestInstallBase, parseFrontmatterBool, resolveOpenClawManifestBlock, resolveOpenClawManifestInstall, @@ -21,30 +22,23 @@ export function parseFrontmatter(content: string): ParsedHookFrontmatter { } function parseInstallSpec(input: unknown): HookInstallSpec | undefined { - if (!input || typeof input !== "object") { + const parsed = parseOpenClawManifestInstallBase(input, ["bundled", "npm", "git"]); + if (!parsed) { return undefined; } - const raw = input as Record; - const kindRaw = - typeof raw.kind === "string" ? raw.kind : typeof raw.type === "string" ? raw.type : ""; - const kind = kindRaw.trim().toLowerCase(); - if (kind !== "bundled" && kind !== "npm" && kind !== "git") { - return undefined; - } - + const { raw } = parsed; const spec: HookInstallSpec = { - kind: kind, + kind: parsed.kind as HookInstallSpec["kind"], }; - if (typeof raw.id === "string") { - spec.id = raw.id; + if (parsed.id) { + spec.id = parsed.id; } - if (typeof raw.label === "string") { - spec.label = raw.label; + if (parsed.label) { + spec.label = parsed.label; } - const bins = normalizeStringList(raw.bins); - if (bins.length > 0) { - spec.bins = bins; + if (parsed.bins) { + spec.bins = parsed.bins; } if (typeof raw.package === "string") { spec.package = raw.package; diff --git a/src/hooks/install.test.ts b/src/hooks/install.test.ts index 4b862a35769..1f4e7d6197a 100644 --- a/src/hooks/install.test.ts +++ b/src/hooks/install.test.ts @@ -3,6 +3,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { afterAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { expectSingleNpmInstallIgnoreScriptsCall } from "../test-utils/exec-assertions.js"; import { isAddressInUseError } from "./gmail-watcher.js"; const fixtureRoot = path.join(os.tmpdir(), `openclaw-hook-install-${randomUUID()}`); @@ -195,16 +196,10 @@ describe("installHooksFromPath", () => { if (!res.ok) { return; } - - const calls = run.mock.calls.filter((c) => Array.isArray(c[0]) && c[0][0] === "npm"); - expect(calls.length).toBe(1); - const first = calls[0]; - if (!first) { - throw new Error("expected npm install call"); - } - const [argv, opts] = first; - expect(argv).toEqual(["npm", "install", "--omit=dev", "--silent", "--ignore-scripts"]); - expect(opts?.cwd).toBe(res.targetDir); + expectSingleNpmInstallIgnoreScriptsCall({ + calls: run.mock.calls as Array<[unknown, { cwd?: string } | undefined]>, + expectedCwd: res.targetDir, + }); }); }); diff --git a/src/hooks/install.ts b/src/hooks/install.ts index da25388d89b..90738629097 100644 --- a/src/hooks/install.ts +++ b/src/hooks/install.ts @@ -81,6 +81,30 @@ async function ensureOpenClawHooks(manifest: HookPackageManifest) { return list; } +function resolveHookInstallModeOptions(params: { + logger?: HookInstallLogger; + mode?: "install" | "update"; + dryRun?: boolean; +}): { logger: HookInstallLogger; mode: "install" | "update"; dryRun: boolean } { + return { + logger: params.logger ?? defaultLogger, + mode: params.mode ?? "install", + dryRun: params.dryRun ?? false, + }; +} + +function resolveTimedHookInstallModeOptions(params: { + logger?: HookInstallLogger; + timeoutMs?: number; + mode?: "install" | "update"; + dryRun?: boolean; +}): { logger: HookInstallLogger; timeoutMs: number; mode: "install" | "update"; dryRun: boolean } { + return { + ...resolveHookInstallModeOptions(params), + timeoutMs: params.timeoutMs ?? 120_000, + }; +} + async function resolveHookNameFromDir(hookDir: string): Promise { const hookMdPath = path.join(hookDir, "HOOK.md"); if (!(await fileExists(hookMdPath))) { @@ -116,10 +140,7 @@ async function installHookPackageFromDir(params: { dryRun?: boolean; expectedHookPackId?: string; }): Promise { - const logger = params.logger ?? defaultLogger; - const timeoutMs = params.timeoutMs ?? 120_000; - const mode = params.mode ?? "install"; - const dryRun = params.dryRun ?? false; + const { logger, timeoutMs, mode, dryRun } = resolveTimedHookInstallModeOptions(params); const manifestPath = path.join(params.packageDir, "package.json"); if (!(await fileExists(manifestPath))) { @@ -222,9 +243,7 @@ async function installHookFromDir(params: { dryRun?: boolean; expectedHookPackId?: string; }): Promise { - const logger = params.logger ?? defaultLogger; - const mode = params.mode ?? "install"; - const dryRun = params.dryRun ?? false; + const { logger, mode, dryRun } = resolveHookInstallModeOptions(params); await validateHookDir(params.hookDir); const hookName = await resolveHookNameFromDir(params.hookDir); @@ -361,10 +380,7 @@ export async function installHooksFromNpmSpec(params: { dryRun?: boolean; expectedHookPackId?: string; }): Promise { - const logger = params.logger ?? defaultLogger; - const timeoutMs = params.timeoutMs ?? 120_000; - const mode = params.mode ?? "install"; - const dryRun = params.dryRun ?? false; + const { logger, timeoutMs, mode, dryRun } = resolveTimedHookInstallModeOptions(params); const expectedHookPackId = params.expectedHookPackId; const spec = params.spec.trim(); const specError = validateRegistryNpmSpec(spec); diff --git a/src/infra/bonjour.test.ts b/src/infra/bonjour.test.ts index a9320e02177..7980ab4bbde 100644 --- a/src/infra/bonjour.test.ts +++ b/src/infra/bonjour.test.ts @@ -14,6 +14,29 @@ const { createService, shutdown, registerUnhandledRejectionHandler, logWarn, log const asString = (value: unknown, fallback: string) => typeof value === "string" && value.trim() ? value : fallback; +function mockCiaoService(params?: { + advertise?: ReturnType; + destroy?: ReturnType; + serviceState?: string; + on?: ReturnType; +}) { + const advertise = params?.advertise ?? vi.fn().mockResolvedValue(undefined); + const destroy = params?.destroy ?? vi.fn().mockResolvedValue(undefined); + const on = params?.on ?? vi.fn(); + createService.mockImplementation((options: Record) => { + return { + advertise, + destroy, + serviceState: params?.serviceState ?? "announced", + on, + getFQDN: () => `${asString(options.type, "service")}.${asString(options.domain, "local")}.`, + getHostname: () => asString(options.hostname, "unknown"), + getPort: () => Number(options.port ?? -1), + }; + }); + return { advertise, destroy, on }; +} + vi.mock("../logger.js", async () => { const actual = await vi.importActual("../logger.js"); return { @@ -96,18 +119,7 @@ describe("gateway bonjour advertiser", () => { setTimeout(resolve, 250); }), ); - - createService.mockImplementation((options: Record) => { - return { - advertise, - destroy, - serviceState: "announced", - on: vi.fn(), - getFQDN: () => `${asString(options.type, "service")}.${asString(options.domain, "local")}.`, - getHostname: () => asString(options.hostname, "unknown"), - getPort: () => Number(options.port ?? -1), - }; - }); + mockCiaoService({ advertise, destroy }); const started = await startGatewayBonjourAdvertiser({ gatewayPort: 18789, @@ -149,18 +161,7 @@ describe("gateway bonjour advertiser", () => { const destroy = vi.fn().mockResolvedValue(undefined); const advertise = vi.fn().mockResolvedValue(undefined); - - createService.mockImplementation((options: Record) => { - return { - advertise, - destroy, - serviceState: "announced", - on: vi.fn(), - getFQDN: () => `${asString(options.type, "service")}.${asString(options.domain, "local")}.`, - getHostname: () => asString(options.hostname, "unknown"), - getPort: () => Number(options.port ?? -1), - }; - }); + mockCiaoService({ advertise, destroy }); const started = await startGatewayBonjourAdvertiser({ gatewayPort: 18789, @@ -188,20 +189,10 @@ describe("gateway bonjour advertiser", () => { const advertise = vi.fn().mockResolvedValue(undefined); const onCalls: Array<{ event: string }> = []; - createService.mockImplementation((options: Record) => { - const on = vi.fn((event: string) => { - onCalls.push({ event }); - }); - return { - advertise, - destroy, - serviceState: "announced", - on, - getFQDN: () => `${asString(options.type, "service")}.${asString(options.domain, "local")}.`, - getHostname: () => asString(options.hostname, "unknown"), - getPort: () => Number(options.port ?? -1), - }; + const on = vi.fn((event: string) => { + onCalls.push({ event }); }); + mockCiaoService({ advertise, destroy, on }); const started = await startGatewayBonjourAdvertiser({ gatewayPort: 18789, @@ -228,18 +219,7 @@ describe("gateway bonjour advertiser", () => { shutdown.mockImplementation(async () => { order.push("shutdown"); }); - - createService.mockImplementation((options: Record) => { - return { - advertise, - destroy, - serviceState: "announced", - on: vi.fn(), - getFQDN: () => `${asString(options.type, "service")}.${asString(options.domain, "local")}.`, - getHostname: () => asString(options.hostname, "unknown"), - getPort: () => Number(options.port ?? -1), - }; - }); + mockCiaoService({ advertise, destroy }); const cleanup = vi.fn(() => { order.push("cleanup"); @@ -272,18 +252,7 @@ describe("gateway bonjour advertiser", () => { .fn() .mockRejectedValueOnce(new Error("boom")) // initial advertise fails .mockResolvedValue(undefined); // watchdog retry succeeds - - createService.mockImplementation((options: Record) => { - return { - advertise, - destroy, - serviceState: "unannounced", - on: vi.fn(), - getFQDN: () => `${asString(options.type, "service")}.${asString(options.domain, "local")}.`, - getHostname: () => asString(options.hostname, "unknown"), - getPort: () => Number(options.port ?? -1), - }; - }); + mockCiaoService({ advertise, destroy, serviceState: "unannounced" }); const started = await startGatewayBonjourAdvertiser({ gatewayPort: 18789, @@ -319,18 +288,7 @@ describe("gateway bonjour advertiser", () => { const advertise = vi.fn(() => { throw new Error("sync-fail"); }); - - createService.mockImplementation((options: Record) => { - return { - advertise, - destroy, - serviceState: "unannounced", - on: vi.fn(), - getFQDN: () => `${asString(options.type, "service")}.${asString(options.domain, "local")}.`, - getHostname: () => asString(options.hostname, "unknown"), - getPort: () => Number(options.port ?? -1), - }; - }); + mockCiaoService({ advertise, destroy, serviceState: "unannounced" }); const started = await startGatewayBonjourAdvertiser({ gatewayPort: 18789, @@ -352,17 +310,7 @@ describe("gateway bonjour advertiser", () => { const destroy = vi.fn().mockResolvedValue(undefined); const advertise = vi.fn().mockResolvedValue(undefined); - createService.mockImplementation((options: Record) => { - return { - advertise, - destroy, - serviceState: "announced", - on: vi.fn(), - getFQDN: () => `${asString(options.type, "service")}.${asString(options.domain, "local")}.`, - getHostname: () => asString(options.hostname, "unknown"), - getPort: () => Number(options.port ?? -1), - }; - }); + mockCiaoService({ advertise, destroy }); const started = await startGatewayBonjourAdvertiser({ gatewayPort: 18789, diff --git a/src/infra/control-ui-assets.test.ts b/src/infra/control-ui-assets.test.ts index 9421cdfb0b7..1d153f5273f 100644 --- a/src/infra/control-ui-assets.test.ts +++ b/src/infra/control-ui-assets.test.ts @@ -28,6 +28,7 @@ vi.mock("node:fs", async (importOriginal) => { const resolved = absInMock(p); return resolved === fixturesRoot.slice(0, -1) || resolved.startsWith(fixturesRoot); }; + const readFixtureEntry = (p: string) => state.entries.get(absInMock(p)); const wrapped = { ...actual, @@ -38,25 +39,25 @@ vi.mock("node:fs", async (importOriginal) => { // eslint-disable-next-line @typescript-eslint/no-explicit-any return actual.readFileSync(p as any, encoding as any) as unknown; } - const entry = state.entries.get(absInMock(p)); - if (!entry || entry.kind !== "file") { - throw new Error(`ENOENT: no such file, open '${p}'`); + const entry = readFixtureEntry(p); + if (entry?.kind === "file") { + return entry.content; } - return entry.content; + throw new Error(`ENOENT: no such file, open '${p}'`); }, statSync: (p: string) => { if (!isFixturePath(p)) { // eslint-disable-next-line @typescript-eslint/no-explicit-any return actual.statSync(p as any) as unknown; } - const entry = state.entries.get(absInMock(p)); - if (!entry) { - throw new Error(`ENOENT: no such file or directory, stat '${p}'`); + const entry = readFixtureEntry(p); + if (entry?.kind === "file") { + return { isFile: () => true, isDirectory: () => false }; } - return { - isFile: () => entry.kind === "file", - isDirectory: () => entry.kind === "dir", - }; + if (entry?.kind === "dir") { + return { isFile: () => false, isDirectory: () => true }; + } + throw new Error(`ENOENT: no such file or directory, stat '${p}'`); }, realpathSync: (p: string) => isFixturePath(p) diff --git a/src/infra/dedupe.ts b/src/infra/dedupe.ts index 850e2145a63..ffb26d295c5 100644 --- a/src/infra/dedupe.ts +++ b/src/infra/dedupe.ts @@ -1,3 +1,5 @@ +import { pruneMapToMaxSize } from "./map-size.js"; + export type DedupeCache = { check: (key: string | undefined | null, now?: number) => boolean; clear: () => void; @@ -32,13 +34,7 @@ export function createDedupeCache(options: DedupeCacheOptions): DedupeCache { cache.clear(); return; } - while (cache.size > maxSize) { - const oldestKey = cache.keys().next().value; - if (!oldestKey) { - break; - } - cache.delete(oldestKey); - } + pruneMapToMaxSize(cache, maxSize); }; return { diff --git a/src/infra/dotenv.test.ts b/src/infra/dotenv.test.ts index c9cab5456b9..e03e8487659 100644 --- a/src/infra/dotenv.test.ts +++ b/src/infra/dotenv.test.ts @@ -9,29 +9,12 @@ async function writeEnvFile(filePath: string, contents: string) { await fs.writeFile(filePath, contents, "utf8"); } -describe("loadDotEnv", () => { - it("loads ~/.openclaw/.env as fallback without overriding CWD .env", async () => { - const prevEnv = { ...process.env }; - const prevCwd = process.cwd(); - - const base = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-dotenv-test-")); - const cwdDir = path.join(base, "cwd"); - const stateDir = path.join(base, "state"); - - process.env.OPENCLAW_STATE_DIR = stateDir; - - await writeEnvFile(path.join(stateDir, ".env"), "FOO=from-global\nBAR=1\n"); - await writeEnvFile(path.join(cwdDir, ".env"), "FOO=from-cwd\n"); - - process.chdir(cwdDir); - delete process.env.FOO; - delete process.env.BAR; - - loadDotEnv({ quiet: true }); - - expect(process.env.FOO).toBe("from-cwd"); - expect(process.env.BAR).toBe("1"); - +async function withIsolatedEnvAndCwd(run: () => Promise) { + const prevEnv = { ...process.env }; + const prevCwd = process.cwd(); + try { + await run(); + } finally { process.chdir(prevCwd); for (const key of Object.keys(process.env)) { if (!(key in prevEnv)) { @@ -45,40 +28,49 @@ describe("loadDotEnv", () => { process.env[key] = value; } } + } +} + +describe("loadDotEnv", () => { + it("loads ~/.openclaw/.env as fallback without overriding CWD .env", async () => { + await withIsolatedEnvAndCwd(async () => { + const base = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-dotenv-test-")); + const cwdDir = path.join(base, "cwd"); + const stateDir = path.join(base, "state"); + + process.env.OPENCLAW_STATE_DIR = stateDir; + + await writeEnvFile(path.join(stateDir, ".env"), "FOO=from-global\nBAR=1\n"); + await writeEnvFile(path.join(cwdDir, ".env"), "FOO=from-cwd\n"); + + process.chdir(cwdDir); + delete process.env.FOO; + delete process.env.BAR; + + loadDotEnv({ quiet: true }); + + expect(process.env.FOO).toBe("from-cwd"); + expect(process.env.BAR).toBe("1"); + }); }); it("does not override an already-set env var from the shell", async () => { - const prevEnv = { ...process.env }; - const prevCwd = process.cwd(); + await withIsolatedEnvAndCwd(async () => { + const base = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-dotenv-test-")); + const cwdDir = path.join(base, "cwd"); + const stateDir = path.join(base, "state"); - const base = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-dotenv-test-")); - const cwdDir = path.join(base, "cwd"); - const stateDir = path.join(base, "state"); + process.env.OPENCLAW_STATE_DIR = stateDir; + process.env.FOO = "from-shell"; - process.env.OPENCLAW_STATE_DIR = stateDir; - process.env.FOO = "from-shell"; + await writeEnvFile(path.join(stateDir, ".env"), "FOO=from-global\n"); + await writeEnvFile(path.join(cwdDir, ".env"), "FOO=from-cwd\n"); - await writeEnvFile(path.join(stateDir, ".env"), "FOO=from-global\n"); - await writeEnvFile(path.join(cwdDir, ".env"), "FOO=from-cwd\n"); + process.chdir(cwdDir); - process.chdir(cwdDir); + loadDotEnv({ quiet: true }); - loadDotEnv({ quiet: true }); - - expect(process.env.FOO).toBe("from-shell"); - - process.chdir(prevCwd); - for (const key of Object.keys(process.env)) { - if (!(key in prevEnv)) { - delete process.env[key]; - } - } - for (const [key, value] of Object.entries(prevEnv)) { - if (value === undefined) { - delete process.env[key]; - } else { - process.env[key] = value; - } - } + expect(process.env.FOO).toBe("from-shell"); + }); }); }); diff --git a/src/infra/exec-approval-forwarder.test.ts b/src/infra/exec-approval-forwarder.test.ts index fc9afb1e537..ca495a9eaa3 100644 --- a/src/infra/exec-approval-forwarder.test.ts +++ b/src/infra/exec-approval-forwarder.test.ts @@ -24,18 +24,40 @@ function getFirstDeliveryText(deliver: ReturnType): string { return firstCall?.payloads?.[0]?.text ?? ""; } +const TARGETS_CFG = { + approvals: { + exec: { + enabled: true, + mode: "targets", + targets: [{ channel: "telegram", to: "123" }], + }, + }, +} as OpenClawConfig; + +function createForwarder(params: { + cfg: OpenClawConfig; + deliver?: ReturnType; + resolveSessionTarget?: () => { channel: string; to: string } | null; +}) { + const deliver = params.deliver ?? vi.fn().mockResolvedValue([]); + const forwarder = createExecApprovalForwarder({ + getConfig: () => params.cfg, + deliver, + nowMs: () => 1000, + resolveSessionTarget: params.resolveSessionTarget ?? (() => null), + }); + return { deliver, forwarder }; +} + describe("exec approval forwarder", () => { it("forwards to session target and resolves", async () => { vi.useFakeTimers(); - const deliver = vi.fn().mockResolvedValue([]); const cfg = { approvals: { exec: { enabled: true, mode: "session" } }, } as OpenClawConfig; - const forwarder = createExecApprovalForwarder({ - getConfig: () => cfg, - deliver, - nowMs: () => 1000, + const { deliver, forwarder } = createForwarder({ + cfg, resolveSessionTarget: () => ({ channel: "slack", to: "U1" }), }); @@ -56,23 +78,7 @@ describe("exec approval forwarder", () => { it("forwards to explicit targets and expires", async () => { vi.useFakeTimers(); - const deliver = vi.fn().mockResolvedValue([]); - const cfg = { - approvals: { - exec: { - enabled: true, - mode: "targets", - targets: [{ channel: "telegram", to: "123" }], - }, - }, - } as OpenClawConfig; - - const forwarder = createExecApprovalForwarder({ - getConfig: () => cfg, - deliver, - nowMs: () => 1000, - resolveSessionTarget: () => null, - }); + const { deliver, forwarder } = createForwarder({ cfg: TARGETS_CFG }); await forwarder.handleRequested(baseRequest); expect(deliver).toHaveBeenCalledTimes(1); @@ -83,23 +89,7 @@ describe("exec approval forwarder", () => { it("formats single-line commands as inline code", async () => { vi.useFakeTimers(); - const deliver = vi.fn().mockResolvedValue([]); - const cfg = { - approvals: { - exec: { - enabled: true, - mode: "targets", - targets: [{ channel: "telegram", to: "123" }], - }, - }, - } as OpenClawConfig; - - const forwarder = createExecApprovalForwarder({ - getConfig: () => cfg, - deliver, - nowMs: () => 1000, - resolveSessionTarget: () => null, - }); + const { deliver, forwarder } = createForwarder({ cfg: TARGETS_CFG }); await forwarder.handleRequested(baseRequest); @@ -108,23 +98,7 @@ describe("exec approval forwarder", () => { it("formats complex commands as fenced code blocks", async () => { vi.useFakeTimers(); - const deliver = vi.fn().mockResolvedValue([]); - const cfg = { - approvals: { - exec: { - enabled: true, - mode: "targets", - targets: [{ channel: "telegram", to: "123" }], - }, - }, - } as OpenClawConfig; - - const forwarder = createExecApprovalForwarder({ - getConfig: () => cfg, - deliver, - nowMs: () => 1000, - resolveSessionTarget: () => null, - }); + const { deliver, forwarder } = createForwarder({ cfg: TARGETS_CFG }); await forwarder.handleRequested({ ...baseRequest, @@ -139,7 +113,6 @@ describe("exec approval forwarder", () => { it("skips discord forwarding when discord exec approvals target channel", async () => { vi.useFakeTimers(); - const deliver = vi.fn().mockResolvedValue([]); const cfg = { approvals: { exec: { enabled: true, mode: "session" } }, channels: { @@ -153,10 +126,8 @@ describe("exec approval forwarder", () => { }, } as OpenClawConfig; - const forwarder = createExecApprovalForwarder({ - getConfig: () => cfg, - deliver, - nowMs: () => 1000, + const { deliver, forwarder } = createForwarder({ + cfg, resolveSessionTarget: () => ({ channel: "discord", to: "channel:123" }), }); @@ -167,23 +138,7 @@ describe("exec approval forwarder", () => { it("uses a longer fence when command already contains triple backticks", async () => { vi.useFakeTimers(); - const deliver = vi.fn().mockResolvedValue([]); - const cfg = { - approvals: { - exec: { - enabled: true, - mode: "targets", - targets: [{ channel: "telegram", to: "123" }], - }, - }, - } as OpenClawConfig; - - const forwarder = createExecApprovalForwarder({ - getConfig: () => cfg, - deliver, - nowMs: () => 1000, - resolveSessionTarget: () => null, - }); + const { deliver, forwarder } = createForwarder({ cfg: TARGETS_CFG }); await forwarder.handleRequested({ ...baseRequest, diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts index ee3a4f822f6..f263e00eaa5 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -34,6 +34,26 @@ function makeTempDir() { return fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-exec-approvals-")); } +function createSafeBinJqCase(params: { command: string; seedFileName?: string }) { + const dir = makeTempDir(); + const binDir = path.join(dir, "bin"); + fs.mkdirSync(binDir, { recursive: true }); + const exeName = process.platform === "win32" ? "jq.exe" : "jq"; + const exe = path.join(binDir, exeName); + fs.writeFileSync(exe, ""); + fs.chmodSync(exe, 0o755); + if (params.seedFileName) { + fs.writeFileSync(path.join(dir, params.seedFileName), "{}"); + } + const res = analyzeShellCommand({ + command: params.command, + cwd: dir, + env: makePathEnv(binDir), + }); + expect(res.ok).toBe(true); + return { dir, segment: res.segments[0] }; +} + describe("exec approvals allowlist matching", () => { it("ignores basename-only patterns", () => { const resolution = { @@ -389,20 +409,7 @@ describe("exec approvals safe bins", () => { if (process.platform === "win32") { return; } - const dir = makeTempDir(); - const binDir = path.join(dir, "bin"); - fs.mkdirSync(binDir, { recursive: true }); - const exeName = process.platform === "win32" ? "jq.exe" : "jq"; - const exe = path.join(binDir, exeName); - fs.writeFileSync(exe, ""); - fs.chmodSync(exe, 0o755); - const res = analyzeShellCommand({ - command: "jq .foo", - cwd: dir, - env: makePathEnv(binDir), - }); - expect(res.ok).toBe(true); - const segment = res.segments[0]; + const { dir, segment } = createSafeBinJqCase({ command: "jq .foo" }); const ok = isSafeBinUsage({ argv: segment.argv, resolution: segment.resolution, @@ -416,22 +423,10 @@ describe("exec approvals safe bins", () => { if (process.platform === "win32") { return; } - const dir = makeTempDir(); - const binDir = path.join(dir, "bin"); - fs.mkdirSync(binDir, { recursive: true }); - const exeName = process.platform === "win32" ? "jq.exe" : "jq"; - const exe = path.join(binDir, exeName); - fs.writeFileSync(exe, ""); - fs.chmodSync(exe, 0o755); - const file = path.join(dir, "secret.json"); - fs.writeFileSync(file, "{}"); - const res = analyzeShellCommand({ + const { dir, segment } = createSafeBinJqCase({ command: "jq .foo secret.json", - cwd: dir, - env: makePathEnv(binDir), + seedFileName: "secret.json", }); - expect(res.ok).toBe(true); - const segment = res.segments[0]; const ok = isSafeBinUsage({ argv: segment.argv, resolution: segment.resolution, diff --git a/src/infra/gateway-lock.test.ts b/src/infra/gateway-lock.test.ts index fd83ef5eab3..f64a03edea3 100644 --- a/src/infra/gateway-lock.test.ts +++ b/src/infra/gateway-lock.test.ts @@ -62,6 +62,25 @@ function makeProcStat(pid: number, startTime: number) { return `${pid} (node) ${fields.join(" ")}`; } +function createLockPayload(params: { configPath: string; startTime: number; createdAt?: string }) { + return { + pid: process.pid, + createdAt: params.createdAt ?? new Date().toISOString(), + configPath: params.configPath, + startTime: params.startTime, + }; +} + +function mockProcStatRead(params: { onProcRead: () => string }) { + const readFileSync = fsSync.readFileSync; + return vi.spyOn(fsSync, "readFileSync").mockImplementation((filePath, encoding) => { + if (filePath === `/proc/${process.pid}/stat`) { + return params.onProcRead(); + } + return readFileSync(filePath as never, encoding as never) as never; + }); +} + describe("gateway lock", () => { beforeAll(async () => { fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-gateway-lock-")); @@ -119,21 +138,12 @@ describe("gateway lock", () => { vi.setSystemTime(new Date("2026-02-06T10:05:00.000Z")); const { env, cleanup } = await makeEnv(); const { lockPath, configPath } = resolveLockPath(env); - const payload = { - pid: process.pid, - createdAt: new Date().toISOString(), - configPath, - startTime: 111, - }; + const payload = createLockPayload({ configPath, startTime: 111 }); await fs.writeFile(lockPath, JSON.stringify(payload), "utf8"); - const readFileSync = fsSync.readFileSync; const statValue = makeProcStat(process.pid, 222); - const spy = vi.spyOn(fsSync, "readFileSync").mockImplementation((filePath, encoding) => { - if (filePath === `/proc/${process.pid}/stat`) { - return statValue; - } - return readFileSync(filePath as never, encoding as never) as never; + const spy = mockProcStatRead({ + onProcRead: () => statValue, }); const lock = await acquireGatewayLock({ @@ -154,20 +164,13 @@ describe("gateway lock", () => { vi.useRealTimers(); const { env, cleanup } = await makeEnv(); const { lockPath, configPath } = resolveLockPath(env); - const payload = { - pid: process.pid, - createdAt: new Date().toISOString(), - configPath, - startTime: 111, - }; + const payload = createLockPayload({ configPath, startTime: 111 }); await fs.writeFile(lockPath, JSON.stringify(payload), "utf8"); - const readFileSync = fsSync.readFileSync; - const spy = vi.spyOn(fsSync, "readFileSync").mockImplementation((filePath, encoding) => { - if (filePath === `/proc/${process.pid}/stat`) { + const spy = mockProcStatRead({ + onProcRead: () => { throw new Error("EACCES"); - } - return readFileSync(filePath as never, encoding as never) as never; + }, }); const pending = acquireGatewayLock({ @@ -182,17 +185,17 @@ describe("gateway lock", () => { spy.mockRestore(); - const stalePayload = { - ...payload, + const stalePayload = createLockPayload({ + configPath, + startTime: 111, createdAt: new Date(0).toISOString(), - }; + }); await fs.writeFile(lockPath, JSON.stringify(stalePayload), "utf8"); - const staleSpy = vi.spyOn(fsSync, "readFileSync").mockImplementation((filePath, encoding) => { - if (filePath === `/proc/${process.pid}/stat`) { + const staleSpy = mockProcStatRead({ + onProcRead: () => { throw new Error("EACCES"); - } - return readFileSync(filePath as never, encoding as never) as never; + }, }); const lock = await acquireGatewayLock({ diff --git a/src/infra/heartbeat-runner.ghost-reminder.test.ts b/src/infra/heartbeat-runner.ghost-reminder.test.ts index f3d613ae960..55aa20a293e 100644 --- a/src/infra/heartbeat-runner.ghost-reminder.test.ts +++ b/src/infra/heartbeat-runner.ghost-reminder.test.ts @@ -86,6 +86,40 @@ describe("Ghost reminder bug (issue #13317)", () => { expect(calledCtx?.Body).not.toContain("heartbeat poll"); }; + const runCronReminderCase = async ( + tmpPrefix: string, + enqueue: (sessionKey: string) => void, + ): Promise<{ + result: Awaited>; + sendTelegram: ReturnType; + getReplySpy: ReturnType>; + }> => { + const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), tmpPrefix)); + const sendTelegram = vi.fn().mockResolvedValue({ + messageId: "m1", + chatId: "155462274", + }); + const getReplySpy = vi + .spyOn(replyModule, "getReplyFromConfig") + .mockResolvedValue({ text: "Relay this reminder now" }); + + try { + const { cfg, sessionKey } = await createConfig(tmpDir); + enqueue(sessionKey); + const result = await runHeartbeatOnce({ + cfg, + agentId: "main", + reason: "cron:reminder-job", + deps: { + sendTelegram, + }, + }); + return { result, sendTelegram, getReplySpy }; + } finally { + await fs.rm(tmpDir, { recursive: true, force: true }); + } + }; + it("does not use CRON_EVENT_PROMPT when only a HEARTBEAT_OK event is present", async () => { const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-ghost-")); const sendTelegram = vi.fn().mockResolvedValue({ @@ -122,68 +156,28 @@ describe("Ghost reminder bug (issue #13317)", () => { }); it("uses CRON_EVENT_PROMPT when an actionable cron event exists", async () => { - const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-cron-")); - const sendTelegram = vi.fn().mockResolvedValue({ - messageId: "m1", - chatId: "155462274", - }); - const getReplySpy = vi - .spyOn(replyModule, "getReplyFromConfig") - .mockResolvedValue({ text: "Relay this reminder now" }); - - try { - const { cfg } = await createConfig(tmpDir); - enqueueSystemEvent("Reminder: Check Base Scout results", { - sessionKey: resolveMainSessionKey(cfg), - }); - - const result = await runHeartbeatOnce({ - cfg, - agentId: "main", - reason: "cron:reminder-job", - deps: { - sendTelegram, - }, - }); - - expect(result.status).toBe("ran"); - expectCronEventPrompt(getReplySpy, "Reminder: Check Base Scout results"); - expect(sendTelegram).toHaveBeenCalled(); - } finally { - await fs.rm(tmpDir, { recursive: true, force: true }); - } + const { result, sendTelegram, getReplySpy } = await runCronReminderCase( + "openclaw-cron-", + (sessionKey) => { + enqueueSystemEvent("Reminder: Check Base Scout results", { sessionKey }); + }, + ); + expect(result.status).toBe("ran"); + expectCronEventPrompt(getReplySpy, "Reminder: Check Base Scout results"); + expect(sendTelegram).toHaveBeenCalled(); }); it("uses CRON_EVENT_PROMPT when cron events are mixed with heartbeat noise", async () => { - const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-cron-mixed-")); - const sendTelegram = vi.fn().mockResolvedValue({ - messageId: "m1", - chatId: "155462274", - }); - const getReplySpy = vi - .spyOn(replyModule, "getReplyFromConfig") - .mockResolvedValue({ text: "Relay this reminder now" }); - - try { - const { cfg, sessionKey } = await createConfig(tmpDir); - enqueueSystemEvent("HEARTBEAT_OK", { sessionKey }); - enqueueSystemEvent("Reminder: Check Base Scout results", { sessionKey }); - - const result = await runHeartbeatOnce({ - cfg, - agentId: "main", - reason: "cron:reminder-job", - deps: { - sendTelegram, - }, - }); - - expect(result.status).toBe("ran"); - expectCronEventPrompt(getReplySpy, "Reminder: Check Base Scout results"); - expect(sendTelegram).toHaveBeenCalled(); - } finally { - await fs.rm(tmpDir, { recursive: true, force: true }); - } + const { result, sendTelegram, getReplySpy } = await runCronReminderCase( + "openclaw-cron-mixed-", + (sessionKey) => { + enqueueSystemEvent("HEARTBEAT_OK", { sessionKey }); + enqueueSystemEvent("Reminder: Check Base Scout results", { sessionKey }); + }, + ); + expect(result.status).toBe("ran"); + expectCronEventPrompt(getReplySpy, "Reminder: Check Base Scout results"); + expect(sendTelegram).toHaveBeenCalled(); }); it("uses CRON_EVENT_PROMPT for tagged cron events on interval wake", async () => { diff --git a/src/infra/heartbeat-runner.respects-ackmaxchars-heartbeat-acks.test.ts b/src/infra/heartbeat-runner.respects-ackmaxchars-heartbeat-acks.test.ts index 83868eec59b..6e698770317 100644 --- a/src/infra/heartbeat-runner.respects-ackmaxchars-heartbeat-acks.test.ts +++ b/src/infra/heartbeat-runner.respects-ackmaxchars-heartbeat-acks.test.ts @@ -1,33 +1,17 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; -import { telegramPlugin } from "../../extensions/telegram/src/channel.js"; -import { setTelegramRuntime } from "../../extensions/telegram/src/runtime.js"; -import { whatsappPlugin } from "../../extensions/whatsapp/src/channel.js"; -import { setWhatsAppRuntime } from "../../extensions/whatsapp/src/runtime.js"; import * as replyModule from "../auto-reply/reply.js"; import { resolveMainSessionKey } from "../config/sessions.js"; -import { setActivePluginRegistry } from "../plugins/runtime.js"; -import { createPluginRuntime } from "../plugins/runtime/index.js"; -import { createTestRegistry } from "../test-utils/channel-plugins.js"; import { runHeartbeatOnce } from "./heartbeat-runner.js"; +import { installHeartbeatRunnerTestRuntime } from "./heartbeat-runner.test-harness.js"; // Avoid pulling optional runtime deps during isolated runs. vi.mock("jiti", () => ({ createJiti: () => () => ({}) })); -beforeEach(() => { - const runtime = createPluginRuntime(); - setTelegramRuntime(runtime); - setWhatsAppRuntime(runtime); - setActivePluginRegistry( - createTestRegistry([ - { pluginId: "whatsapp", plugin: whatsappPlugin, source: "test" }, - { pluginId: "telegram", plugin: telegramPlugin, source: "test" }, - ]), - ); -}); +installHeartbeatRunnerTestRuntime(); describe("resolveHeartbeatIntervalMs", () => { async function seedSessionStore( @@ -82,21 +66,16 @@ describe("resolveHeartbeatIntervalMs", () => { replySpy: ReturnType; }) => Promise, ) { - const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-hb-")); - const storePath = path.join(tmpDir, "sessions.json"); - const replySpy = vi.spyOn(replyModule, "getReplyFromConfig"); const prevTelegramToken = process.env.TELEGRAM_BOT_TOKEN; process.env.TELEGRAM_BOT_TOKEN = ""; try { - return await fn({ tmpDir, storePath, replySpy }); + return await withTempHeartbeatSandbox(fn); } finally { - replySpy.mockRestore(); if (prevTelegramToken === undefined) { delete process.env.TELEGRAM_BOT_TOKEN; } else { process.env.TELEGRAM_BOT_TOKEN = prevTelegramToken; } - await fs.rm(tmpDir, { recursive: true, force: true }); } } diff --git a/src/infra/heartbeat-runner.scheduler.test.ts b/src/infra/heartbeat-runner.scheduler.test.ts index ba560826cfe..23c5cbb8129 100644 --- a/src/infra/heartbeat-runner.scheduler.test.ts +++ b/src/infra/heartbeat-runner.scheduler.test.ts @@ -3,6 +3,15 @@ import type { OpenClawConfig } from "../config/config.js"; import { startHeartbeatRunner } from "./heartbeat-runner.js"; describe("startHeartbeatRunner", () => { + function startDefaultRunner(runOnce: (typeof startHeartbeatRunner)[0]["runOnce"]) { + return startHeartbeatRunner({ + cfg: { + agents: { defaults: { heartbeat: { every: "30m" } } }, + } as OpenClawConfig, + runOnce, + }); + } + afterEach(() => { vi.useRealTimers(); vi.restoreAllMocks(); @@ -14,12 +23,7 @@ describe("startHeartbeatRunner", () => { const runSpy = vi.fn().mockResolvedValue({ status: "ran", durationMs: 1 }); - const runner = startHeartbeatRunner({ - cfg: { - agents: { defaults: { heartbeat: { every: "30m" } } }, - } as OpenClawConfig, - runOnce: runSpy, - }); + const runner = startDefaultRunner(runSpy); await vi.advanceTimersByTimeAsync(30 * 60_000 + 1_000); @@ -69,12 +73,7 @@ describe("startHeartbeatRunner", () => { return { status: "ran", durationMs: 1 }; }); - const runner = startHeartbeatRunner({ - cfg: { - agents: { defaults: { heartbeat: { every: "30m" } } }, - } as OpenClawConfig, - runOnce: runSpy, - }); + const runner = startDefaultRunner(runSpy); // First heartbeat fires and throws await vi.advanceTimersByTimeAsync(30 * 60_000 + 1_000); @@ -124,12 +123,7 @@ describe("startHeartbeatRunner", () => { const runSpy = vi.fn().mockResolvedValue({ status: "ran", durationMs: 1 }); - const runner = startHeartbeatRunner({ - cfg: { - agents: { defaults: { heartbeat: { every: "30m" } } }, - } as OpenClawConfig, - runOnce: runSpy, - }); + const runner = startDefaultRunner(runSpy); runner.stop(); diff --git a/src/infra/heartbeat-runner.sender-prefers-delivery-target.test.ts b/src/infra/heartbeat-runner.sender-prefers-delivery-target.test.ts index 405d41877b8..0c83468a2f9 100644 --- a/src/infra/heartbeat-runner.sender-prefers-delivery-target.test.ts +++ b/src/infra/heartbeat-runner.sender-prefers-delivery-target.test.ts @@ -1,37 +1,17 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; -import { slackPlugin } from "../../extensions/slack/src/channel.js"; -import { setSlackRuntime } from "../../extensions/slack/src/runtime.js"; -import { telegramPlugin } from "../../extensions/telegram/src/channel.js"; -import { setTelegramRuntime } from "../../extensions/telegram/src/runtime.js"; -import { whatsappPlugin } from "../../extensions/whatsapp/src/channel.js"; -import { setWhatsAppRuntime } from "../../extensions/whatsapp/src/runtime.js"; import * as replyModule from "../auto-reply/reply.js"; import { resolveMainSessionKey } from "../config/sessions.js"; -import { setActivePluginRegistry } from "../plugins/runtime.js"; -import { createPluginRuntime } from "../plugins/runtime/index.js"; -import { createTestRegistry } from "../test-utils/channel-plugins.js"; import { runHeartbeatOnce } from "./heartbeat-runner.js"; +import { installHeartbeatRunnerTestRuntime } from "./heartbeat-runner.test-harness.js"; // Avoid pulling optional runtime deps during isolated runs. vi.mock("jiti", () => ({ createJiti: () => () => ({}) })); -beforeEach(() => { - const runtime = createPluginRuntime(); - setSlackRuntime(runtime); - setTelegramRuntime(runtime); - setWhatsAppRuntime(runtime); - setActivePluginRegistry( - createTestRegistry([ - { pluginId: "slack", plugin: slackPlugin, source: "test" }, - { pluginId: "whatsapp", plugin: whatsappPlugin, source: "test" }, - { pluginId: "telegram", plugin: telegramPlugin, source: "test" }, - ]), - ); -}); +installHeartbeatRunnerTestRuntime({ includeSlack: true }); describe("runHeartbeatOnce", () => { it("uses the delivery target as sender when lastTo differs", async () => { diff --git a/src/infra/heartbeat-runner.test-harness.ts b/src/infra/heartbeat-runner.test-harness.ts new file mode 100644 index 00000000000..8227c917571 --- /dev/null +++ b/src/infra/heartbeat-runner.test-harness.ts @@ -0,0 +1,40 @@ +import { beforeEach } from "vitest"; +import type { ChannelPlugin } from "../channels/plugins/types.plugin.js"; +import { slackPlugin } from "../../extensions/slack/src/channel.js"; +import { setSlackRuntime } from "../../extensions/slack/src/runtime.js"; +import { telegramPlugin } from "../../extensions/telegram/src/channel.js"; +import { setTelegramRuntime } from "../../extensions/telegram/src/runtime.js"; +import { whatsappPlugin } from "../../extensions/whatsapp/src/channel.js"; +import { setWhatsAppRuntime } from "../../extensions/whatsapp/src/runtime.js"; +import { setActivePluginRegistry } from "../plugins/runtime.js"; +import { createPluginRuntime } from "../plugins/runtime/index.js"; +import { createTestRegistry } from "../test-utils/channel-plugins.js"; + +const slackChannelPlugin = slackPlugin as unknown as ChannelPlugin; +const telegramChannelPlugin = telegramPlugin as unknown as ChannelPlugin; +const whatsappChannelPlugin = whatsappPlugin as unknown as ChannelPlugin; + +export function installHeartbeatRunnerTestRuntime(params?: { includeSlack?: boolean }): void { + beforeEach(() => { + const runtime = createPluginRuntime(); + setTelegramRuntime(runtime); + setWhatsAppRuntime(runtime); + if (params?.includeSlack) { + setSlackRuntime(runtime); + setActivePluginRegistry( + createTestRegistry([ + { pluginId: "slack", plugin: slackChannelPlugin, source: "test" }, + { pluginId: "whatsapp", plugin: whatsappChannelPlugin, source: "test" }, + { pluginId: "telegram", plugin: telegramChannelPlugin, source: "test" }, + ]), + ); + return; + } + setActivePluginRegistry( + createTestRegistry([ + { pluginId: "whatsapp", plugin: whatsappChannelPlugin, source: "test" }, + { pluginId: "telegram", plugin: telegramChannelPlugin, source: "test" }, + ]), + ); + }); +} diff --git a/src/infra/heartbeat-visibility.test.ts b/src/infra/heartbeat-visibility.test.ts index 1a7ab6df725..f48e37ad68c 100644 --- a/src/infra/heartbeat-visibility.test.ts +++ b/src/infra/heartbeat-visibility.test.ts @@ -3,6 +3,25 @@ import type { OpenClawConfig } from "../config/config.js"; import { resolveHeartbeatVisibility } from "./heartbeat-visibility.js"; describe("resolveHeartbeatVisibility", () => { + function createTelegramAccountHeartbeatConfig(): OpenClawConfig { + return { + channels: { + telegram: { + heartbeat: { + showOk: true, + }, + accounts: { + primary: { + heartbeat: { + showOk: false, + }, + }, + }, + }, + }, + } as OpenClawConfig; + } + it("returns default values when no config is provided", () => { const cfg = {} as OpenClawConfig; const result = resolveHeartbeatVisibility({ cfg, channel: "telegram" }); @@ -136,46 +155,14 @@ describe("resolveHeartbeatVisibility", () => { }); it("handles missing accountId gracefully", () => { - const cfg = { - channels: { - telegram: { - heartbeat: { - showOk: true, - }, - accounts: { - primary: { - heartbeat: { - showOk: false, - }, - }, - }, - }, - }, - } as OpenClawConfig; - + const cfg = createTelegramAccountHeartbeatConfig(); const result = resolveHeartbeatVisibility({ cfg, channel: "telegram" }); expect(result.showOk).toBe(true); }); it("handles non-existent account gracefully", () => { - const cfg = { - channels: { - telegram: { - heartbeat: { - showOk: true, - }, - accounts: { - primary: { - heartbeat: { - showOk: false, - }, - }, - }, - }, - }, - } as OpenClawConfig; - + const cfg = createTelegramAccountHeartbeatConfig(); const result = resolveHeartbeatVisibility({ cfg, channel: "telegram", diff --git a/src/infra/heartbeat-wake.test.ts b/src/infra/heartbeat-wake.test.ts index 63d47523023..85a5f247e16 100644 --- a/src/infra/heartbeat-wake.test.ts +++ b/src/infra/heartbeat-wake.test.ts @@ -8,6 +8,25 @@ import { } from "./heartbeat-wake.js"; describe("heartbeat-wake", () => { + async function expectRetryAfterDefaultDelay(params: { + handler: ReturnType; + initialReason: string; + expectedRetryReason: string; + }) { + setHeartbeatWakeHandler(params.handler); + requestHeartbeatNow({ reason: params.initialReason, coalesceMs: 0 }); + + await vi.advanceTimersByTimeAsync(1); + expect(params.handler).toHaveBeenCalledTimes(1); + + await vi.advanceTimersByTimeAsync(500); + expect(params.handler).toHaveBeenCalledTimes(1); + + await vi.advanceTimersByTimeAsync(500); + expect(params.handler).toHaveBeenCalledTimes(2); + expect(params.handler.mock.calls[1]?.[0]).toEqual({ reason: params.expectedRetryReason }); + } + beforeEach(() => { resetHeartbeatWakeStateForTests(); }); @@ -44,19 +63,11 @@ describe("heartbeat-wake", () => { .fn() .mockResolvedValueOnce({ status: "skipped", reason: "requests-in-flight" }) .mockResolvedValueOnce({ status: "ran", durationMs: 1 }); - setHeartbeatWakeHandler(handler); - - requestHeartbeatNow({ reason: "interval", coalesceMs: 0 }); - - await vi.advanceTimersByTimeAsync(1); - expect(handler).toHaveBeenCalledTimes(1); - - await vi.advanceTimersByTimeAsync(500); - expect(handler).toHaveBeenCalledTimes(1); - - await vi.advanceTimersByTimeAsync(500); - expect(handler).toHaveBeenCalledTimes(2); - expect(handler.mock.calls[1]?.[0]).toEqual({ reason: "interval" }); + await expectRetryAfterDefaultDelay({ + handler, + initialReason: "interval", + expectedRetryReason: "interval", + }); }); it("keeps retry cooldown even when a sooner request arrives", async () => { @@ -87,19 +98,11 @@ describe("heartbeat-wake", () => { .fn() .mockRejectedValueOnce(new Error("boom")) .mockResolvedValueOnce({ status: "skipped", reason: "disabled" }); - setHeartbeatWakeHandler(handler); - - requestHeartbeatNow({ reason: "exec-event", coalesceMs: 0 }); - - await vi.advanceTimersByTimeAsync(1); - expect(handler).toHaveBeenCalledTimes(1); - - await vi.advanceTimersByTimeAsync(500); - expect(handler).toHaveBeenCalledTimes(1); - - await vi.advanceTimersByTimeAsync(500); - expect(handler).toHaveBeenCalledTimes(2); - expect(handler.mock.calls[1]?.[0]).toEqual({ reason: "exec-event" }); + await expectRetryAfterDefaultDelay({ + handler, + initialReason: "exec-event", + expectedRetryReason: "exec-event", + }); }); it("stale disposer does not clear a newer handler", async () => { diff --git a/src/infra/http-body.test.ts b/src/infra/http-body.test.ts index 1cc6994b516..16b63a10e48 100644 --- a/src/infra/http-body.test.ts +++ b/src/infra/http-body.test.ts @@ -1,6 +1,7 @@ -import type { IncomingMessage, ServerResponse } from "node:http"; +import type { IncomingMessage } from "node:http"; import { EventEmitter } from "node:events"; import { describe, expect, it } from "vitest"; +import { createMockServerResponse } from "../test-utils/mock-http-response.js"; import { installRequestBodyLimitGuard, isRequestBodyLimitError, @@ -52,24 +53,6 @@ function createMockRequest(params: { return req; } -function createMockResponse(): ServerResponse & { body?: string } { - const headers: Record = {}; - const res = { - headersSent: false, - statusCode: 200, - setHeader: (key: string, value: string) => { - headers[key.toLowerCase()] = value; - return res; - }, - end: (body?: string) => { - res.headersSent = true; - res.body = body; - return res; - }, - } as unknown as ServerResponse & { body?: string }; - return res; -} - describe("http body limits", () => { it("reads body within max bytes", async () => { const req = createMockRequest({ chunks: ['{"ok":true}'] }); @@ -104,7 +87,7 @@ describe("http body limits", () => { headers: { "content-length": "9999" }, emitEnd: false, }); - const res = createMockResponse(); + const res = createMockServerResponse(); const guard = installRequestBodyLimitGuard(req, res, { maxBytes: 128 }); expect(guard.isTripped()).toBe(true); expect(guard.code()).toBe("PAYLOAD_TOO_LARGE"); @@ -113,7 +96,7 @@ describe("http body limits", () => { it("guard rejects streamed oversized body", async () => { const req = createMockRequest({ chunks: ["small", "x".repeat(256)], emitEnd: false }); - const res = createMockResponse(); + const res = createMockServerResponse(); const guard = installRequestBodyLimitGuard(req, res, { maxBytes: 128, responseFormat: "text" }); await new Promise((resolve) => setTimeout(resolve, 0)); expect(guard.isTripped()).toBe(true); diff --git a/src/infra/infra-runtime.test.ts b/src/infra/infra-runtime.test.ts index 78e6d15f9a3..66a81f7bc06 100644 --- a/src/infra/infra-runtime.test.ts +++ b/src/infra/infra-runtime.test.ts @@ -18,6 +18,21 @@ import { getShellPathFromLoginShell, resetShellPathCacheForTests } from "./shell import { listTailnetAddresses } from "./tailnet.js"; describe("infra runtime", () => { + function setupRestartSignalSuite() { + beforeEach(() => { + __testing.resetSigusr1State(); + vi.useFakeTimers(); + vi.spyOn(process, "kill").mockImplementation(() => true); + }); + + afterEach(async () => { + await vi.runOnlyPendingTimersAsync(); + vi.useRealTimers(); + vi.restoreAllMocks(); + __testing.resetSigusr1State(); + }); + } + describe("ensureBinary", () => { it("passes through when binary exists", async () => { const exec: typeof runExec = vi.fn().mockResolvedValue({ @@ -69,18 +84,7 @@ describe("infra runtime", () => { }); describe("restart authorization", () => { - beforeEach(() => { - __testing.resetSigusr1State(); - vi.useFakeTimers(); - vi.spyOn(process, "kill").mockImplementation(() => true); - }); - - afterEach(async () => { - await vi.runOnlyPendingTimersAsync(); - vi.useRealTimers(); - vi.restoreAllMocks(); - __testing.resetSigusr1State(); - }); + setupRestartSignalSuite(); it("authorizes exactly once when scheduled restart emits", async () => { expect(consumeGatewaySigusr1RestartAuthorization()).toBe(false); @@ -124,18 +128,7 @@ describe("infra runtime", () => { }); describe("pre-restart deferral check", () => { - beforeEach(() => { - __testing.resetSigusr1State(); - vi.useFakeTimers(); - vi.spyOn(process, "kill").mockImplementation(() => true); - }); - - afterEach(async () => { - await vi.runOnlyPendingTimersAsync(); - vi.useRealTimers(); - vi.restoreAllMocks(); - __testing.resetSigusr1State(); - }); + setupRestartSignalSuite(); it("emits SIGUSR1 immediately when no deferral check is registered", async () => { const emitSpy = vi.spyOn(process, "emit"); diff --git a/src/infra/map-size.ts b/src/infra/map-size.ts new file mode 100644 index 00000000000..ff5743a9376 --- /dev/null +++ b/src/infra/map-size.ts @@ -0,0 +1,15 @@ +export function pruneMapToMaxSize(map: Map, maxSize: number): void { + const limit = Math.max(0, Math.floor(maxSize)); + if (limit <= 0) { + map.clear(); + return; + } + + while (map.size > limit) { + const oldest = map.keys().next(); + if (oldest.done) { + break; + } + map.delete(oldest.value); + } +} diff --git a/src/infra/openclaw-root.test.ts b/src/infra/openclaw-root.test.ts index 5f2a4e94d65..efdad7c4304 100644 --- a/src/infra/openclaw-root.test.ts +++ b/src/infra/openclaw-root.test.ts @@ -14,6 +14,11 @@ const state = vi.hoisted(() => ({ const abs = (p: string) => path.resolve(p); const fx = (...parts: string[]) => path.join(FIXTURE_BASE, ...parts); +const vitestRootWithSep = `${abs(VITEST_FS_BASE)}${path.sep}`; +const isFixturePath = (p: string) => { + const resolved = abs(p); + return resolved === vitestRootWithSep.slice(0, -1) || resolved.startsWith(vitestRootWithSep); +}; function setFile(p: string, content = "") { state.entries.set(abs(p), { kind: "file", content }); @@ -21,23 +26,16 @@ function setFile(p: string, content = "") { vi.mock("node:fs", async (importOriginal) => { const actual = await importOriginal(); - const pathMod = await import("node:path"); - const absInMock = (p: string) => pathMod.resolve(p); - const vitestRoot = `${absInMock(VITEST_FS_BASE)}${pathMod.sep}`; - const isFixturePath = (p: string) => { - const resolved = absInMock(p); - return resolved === vitestRoot.slice(0, -1) || resolved.startsWith(vitestRoot); - }; const wrapped = { ...actual, existsSync: (p: string) => - isFixturePath(p) ? state.entries.has(absInMock(p)) : actual.existsSync(p), + isFixturePath(p) ? state.entries.has(abs(p)) : actual.existsSync(p), readFileSync: (p: string, encoding?: unknown) => { if (!isFixturePath(p)) { // eslint-disable-next-line @typescript-eslint/no-explicit-any return actual.readFileSync(p as any, encoding as any) as unknown; } - const entry = state.entries.get(absInMock(p)); + const entry = state.entries.get(abs(p)); if (!entry || entry.kind !== "file") { throw new Error(`ENOENT: no such file, open '${p}'`); } @@ -48,7 +46,7 @@ vi.mock("node:fs", async (importOriginal) => { // eslint-disable-next-line @typescript-eslint/no-explicit-any return actual.statSync(p as any) as unknown; } - const entry = state.entries.get(absInMock(p)); + const entry = state.entries.get(abs(p)); if (!entry) { throw new Error(`ENOENT: no such file or directory, stat '${p}'`); } @@ -58,22 +56,13 @@ vi.mock("node:fs", async (importOriginal) => { }; }, realpathSync: (p: string) => - isFixturePath(p) - ? (state.realpaths.get(absInMock(p)) ?? absInMock(p)) - : actual.realpathSync(p), + isFixturePath(p) ? (state.realpaths.get(abs(p)) ?? abs(p)) : actual.realpathSync(p), }; return { ...wrapped, default: wrapped }; }); vi.mock("node:fs/promises", async (importOriginal) => { const actual = await importOriginal(); - const pathMod = await import("node:path"); - const absInMock = (p: string) => pathMod.resolve(p); - const vitestRoot = `${absInMock(VITEST_FS_BASE)}${pathMod.sep}`; - const isFixturePath = (p: string) => { - const resolved = absInMock(p); - return resolved === vitestRoot.slice(0, -1) || resolved.startsWith(vitestRoot); - }; const wrapped = { ...actual, readFile: async (p: string, encoding?: unknown) => { @@ -81,7 +70,7 @@ vi.mock("node:fs/promises", async (importOriginal) => { // eslint-disable-next-line @typescript-eslint/no-explicit-any return (await actual.readFile(p as any, encoding as any)) as unknown; } - const entry = state.entries.get(absInMock(p)); + const entry = state.entries.get(abs(p)); if (!entry || entry.kind !== "file") { throw new Error(`ENOENT: no such file, open '${p}'`); } diff --git a/src/infra/outbound/message.e2e.test.ts b/src/infra/outbound/message.e2e.test.ts index 531671d893e..cd2212928ac 100644 --- a/src/infra/outbound/message.e2e.test.ts +++ b/src/infra/outbound/message.e2e.test.ts @@ -89,7 +89,7 @@ describe("sendMessage replyToId threading", () => { setRegistry(emptyRegistry); }); - it("passes replyToId through to the outbound adapter", async () => { + const setupMattermostCapture = () => { const capturedCtx: Record[] = []; const plugin = createMattermostLikePlugin({ onSendText: (ctx) => { @@ -97,6 +97,11 @@ describe("sendMessage replyToId threading", () => { }, }); setRegistry(createTestRegistry([{ pluginId: "mattermost", source: "test", plugin }])); + return capturedCtx; + }; + + it("passes replyToId through to the outbound adapter", async () => { + const capturedCtx = setupMattermostCapture(); await sendMessage({ cfg: {}, @@ -111,13 +116,7 @@ describe("sendMessage replyToId threading", () => { }); it("passes threadId through to the outbound adapter", async () => { - const capturedCtx: Record[] = []; - const plugin = createMattermostLikePlugin({ - onSendText: (ctx) => { - capturedCtx.push(ctx); - }, - }); - setRegistry(createTestRegistry([{ pluginId: "mattermost", source: "test", plugin }])); + const capturedCtx = setupMattermostCapture(); await sendMessage({ cfg: {}, diff --git a/src/infra/provider-usage.fetch.antigravity.test.ts b/src/infra/provider-usage.fetch.antigravity.test.ts index a3c1080214a..c8ece1ad273 100644 --- a/src/infra/provider-usage.fetch.antigravity.test.ts +++ b/src/infra/provider-usage.fetch.antigravity.test.ts @@ -7,12 +7,22 @@ const makeResponse = (status: number, body: unknown): Response => { return new Response(payload, { status, headers }); }; +const toRequestUrl = (input: Parameters[0]): string => + typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; + +const createAntigravityFetch = ( + handler: (url: string, init?: Parameters[1]) => Promise | Response, +) => + vi.fn, ReturnType>(async (input, init) => + handler(toRequestUrl(input), init), + ); + +const getRequestBody = (init?: Parameters[1]) => + typeof init?.body === "string" ? init.body : undefined; + describe("fetchAntigravityUsage", () => { it("returns 3 windows when both endpoints succeed", async () => { - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; - + const mockFetch = createAntigravityFetch(async (url) => { if (url.includes("loadCodeAssist")) { return makeResponse(200, { availablePromptCredits: 750, @@ -69,10 +79,7 @@ describe("fetchAntigravityUsage", () => { }); it("returns Credits only when loadCodeAssist succeeds but fetchAvailableModels fails", async () => { - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; - + const mockFetch = createAntigravityFetch(async (url) => { if (url.includes("loadCodeAssist")) { return makeResponse(200, { availablePromptCredits: 250, @@ -103,10 +110,7 @@ describe("fetchAntigravityUsage", () => { }); it("returns model IDs when fetchAvailableModels succeeds but loadCodeAssist fails", async () => { - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; - + const mockFetch = createAntigravityFetch(async (url) => { if (url.includes("loadCodeAssist")) { return makeResponse(500, "Internal server error"); } @@ -144,27 +148,22 @@ describe("fetchAntigravityUsage", () => { it("uses cloudaicompanionProject string as project id", async () => { let capturedBody: string | undefined; - const mockFetch = vi.fn, ReturnType>( - async (input, init) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; + const mockFetch = createAntigravityFetch(async (url, init) => { + if (url.includes("loadCodeAssist")) { + return makeResponse(200, { + availablePromptCredits: 900, + planInfo: { monthlyPromptCredits: 1000 }, + cloudaicompanionProject: "projects/alpha", + }); + } - if (url.includes("loadCodeAssist")) { - return makeResponse(200, { - availablePromptCredits: 900, - planInfo: { monthlyPromptCredits: 1000 }, - cloudaicompanionProject: "projects/alpha", - }); - } + if (url.includes("fetchAvailableModels")) { + capturedBody = getRequestBody(init); + return makeResponse(200, { models: {} }); + } - if (url.includes("fetchAvailableModels")) { - capturedBody = init?.body?.toString(); - return makeResponse(200, { models: {} }); - } - - return makeResponse(404, "not found"); - }, - ); + return makeResponse(404, "not found"); + }); await fetchAntigravityUsage("token-123", 5000, mockFetch); @@ -173,27 +172,22 @@ describe("fetchAntigravityUsage", () => { it("uses cloudaicompanionProject object id when present", async () => { let capturedBody: string | undefined; - const mockFetch = vi.fn, ReturnType>( - async (input, init) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; + const mockFetch = createAntigravityFetch(async (url, init) => { + if (url.includes("loadCodeAssist")) { + return makeResponse(200, { + availablePromptCredits: 900, + planInfo: { monthlyPromptCredits: 1000 }, + cloudaicompanionProject: { id: "projects/beta" }, + }); + } - if (url.includes("loadCodeAssist")) { - return makeResponse(200, { - availablePromptCredits: 900, - planInfo: { monthlyPromptCredits: 1000 }, - cloudaicompanionProject: { id: "projects/beta" }, - }); - } + if (url.includes("fetchAvailableModels")) { + capturedBody = getRequestBody(init); + return makeResponse(200, { models: {} }); + } - if (url.includes("fetchAvailableModels")) { - capturedBody = init?.body?.toString(); - return makeResponse(200, { models: {} }); - } - - return makeResponse(404, "not found"); - }, - ); + return makeResponse(404, "not found"); + }); await fetchAntigravityUsage("token-123", 5000, mockFetch); @@ -201,10 +195,7 @@ describe("fetchAntigravityUsage", () => { }); it("returns error snapshot when both endpoints fail", async () => { - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; - + const mockFetch = createAntigravityFetch(async (url) => { if (url.includes("loadCodeAssist")) { return makeResponse(403, { error: { message: "Access denied" } }); } @@ -226,10 +217,7 @@ describe("fetchAntigravityUsage", () => { }); it("returns Token expired when fetchAvailableModels returns 401 and no windows", async () => { - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; - + const mockFetch = createAntigravityFetch(async (url) => { if (url.includes("loadCodeAssist")) { return makeResponse(500, "Boom"); } @@ -248,10 +236,7 @@ describe("fetchAntigravityUsage", () => { }); it("extracts plan info from currentTier.name", async () => { - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; - + const mockFetch = createAntigravityFetch(async (url) => { if (url.includes("loadCodeAssist")) { return makeResponse(200, { availablePromptCredits: 500, @@ -274,10 +259,7 @@ describe("fetchAntigravityUsage", () => { }); it("falls back to planType when currentTier.name is missing", async () => { - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; - + const mockFetch = createAntigravityFetch(async (url) => { if (url.includes("loadCodeAssist")) { return makeResponse(200, { availablePromptCredits: 500, @@ -300,10 +282,7 @@ describe("fetchAntigravityUsage", () => { it("includes reset times in model windows", async () => { const resetTime = "2026-01-10T12:00:00Z"; - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; - + const mockFetch = createAntigravityFetch(async (url) => { if (url.includes("loadCodeAssist")) { return makeResponse(500, "Error"); } @@ -328,10 +307,7 @@ describe("fetchAntigravityUsage", () => { }); it("parses string numbers correctly", async () => { - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; - + const mockFetch = createAntigravityFetch(async (url) => { if (url.includes("loadCodeAssist")) { return makeResponse(200, { availablePromptCredits: "600", @@ -364,10 +340,7 @@ describe("fetchAntigravityUsage", () => { }); it("skips internal models", async () => { - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; - + const mockFetch = createAntigravityFetch(async (url) => { if (url.includes("loadCodeAssist")) { return makeResponse(200, { availablePromptCredits: 500, @@ -395,10 +368,7 @@ describe("fetchAntigravityUsage", () => { }); it("sorts models by usage and shows individual model IDs", async () => { - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; - + const mockFetch = createAntigravityFetch(async (url) => { if (url.includes("loadCodeAssist")) { return makeResponse(500, "Error"); } @@ -440,10 +410,7 @@ describe("fetchAntigravityUsage", () => { }); it("returns Token expired error on 401 from loadCodeAssist", async () => { - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; - + const mockFetch = createAntigravityFetch(async (url) => { if (url.includes("loadCodeAssist")) { return makeResponse(401, { error: { message: "Unauthorized" } }); } @@ -459,10 +426,7 @@ describe("fetchAntigravityUsage", () => { }); it("handles empty models array gracefully", async () => { - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; - + const mockFetch = createAntigravityFetch(async (url) => { if (url.includes("loadCodeAssist")) { return makeResponse(200, { availablePromptCredits: 800, @@ -486,10 +450,7 @@ describe("fetchAntigravityUsage", () => { }); it("handles missing credits fields gracefully", async () => { - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; - + const mockFetch = createAntigravityFetch(async (url) => { if (url.includes("loadCodeAssist")) { return makeResponse(200, { planType: "Free" }); } @@ -517,10 +478,7 @@ describe("fetchAntigravityUsage", () => { }); it("handles invalid reset time gracefully", async () => { - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; - + const mockFetch = createAntigravityFetch(async (url) => { if (url.includes("loadCodeAssist")) { return makeResponse(500, "Error"); } @@ -546,10 +504,7 @@ describe("fetchAntigravityUsage", () => { }); it("handles network errors with graceful degradation", async () => { - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; - + const mockFetch = createAntigravityFetch(async (url) => { if (url.includes("loadCodeAssist")) { throw new Error("Network failure"); } diff --git a/src/infra/provider-usage.test.ts b/src/infra/provider-usage.test.ts index 43e543a8682..c03022ef5e0 100644 --- a/src/infra/provider-usage.test.ts +++ b/src/infra/provider-usage.test.ts @@ -10,6 +10,48 @@ import { type UsageSummary, } from "./provider-usage.js"; +const minimaxRemainsEndpoint = "api.minimaxi.com/v1/api/openplatform/coding_plan/remains"; + +function makeResponse(status: number, body: unknown): Response { + const payload = typeof body === "string" ? body : JSON.stringify(body); + const headers = typeof body === "string" ? undefined : { "Content-Type": "application/json" }; + return new Response(payload, { status, headers }); +} + +function toRequestUrl(input: Parameters[0]): string { + return typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; +} + +function createMinimaxOnlyFetch(payload: unknown) { + return vi.fn, ReturnType>(async (input) => { + if (toRequestUrl(input).includes(minimaxRemainsEndpoint)) { + return makeResponse(200, payload); + } + return makeResponse(404, "not found"); + }); +} + +async function expectMinimaxUsage( + payload: unknown, + expectedUsedPercent: number, + expectedPlan?: string, +) { + const mockFetch = createMinimaxOnlyFetch(payload); + + const summary = await loadProviderUsageSummary({ + now: Date.UTC(2026, 0, 7, 0, 0, 0), + auth: [{ provider: "minimax", token: "token-1b" }], + fetch: mockFetch, + }); + + const minimax = summary.providers.find((p) => p.provider === "minimax"); + expect(minimax?.windows[0]?.usedPercent).toBe(expectedUsedPercent); + if (expectedPlan !== undefined) { + expect(minimax?.plan).toBe(expectedPlan); + } + expect(mockFetch).toHaveBeenCalled(); +} + describe("provider usage formatting", () => { it("returns null when no usage is available", () => { const summary: UsageSummary = { updatedAt: 0, providers: [] }; @@ -71,15 +113,8 @@ describe("provider usage formatting", () => { describe("provider usage loading", () => { it("loads usage snapshots with injected auth", async () => { - const makeResponse = (status: number, body: unknown): Response => { - const payload = typeof body === "string" ? body : JSON.stringify(body); - const headers = typeof body === "string" ? undefined : { "Content-Type": "application/json" }; - return new Response(payload, { status, headers }); - }; - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; + const url = toRequestUrl(input); if (url.includes("api.anthropic.com")) { return makeResponse(200, { five_hour: { utilization: 20, resets_at: "2026-01-07T01:00:00Z" }, @@ -103,7 +138,7 @@ describe("provider usage loading", () => { }, }); } - if (url.includes("api.minimaxi.com/v1/api/openplatform/coding_plan/remains")) { + if (url.includes(minimaxRemainsEndpoint)) { return makeResponse(200, { base_resp: { status_code: 0, status_msg: "ok" }, data: { @@ -138,115 +173,55 @@ describe("provider usage loading", () => { }); it("handles nested MiniMax usage payloads", async () => { - const makeResponse = (status: number, body: unknown): Response => { - const payload = typeof body === "string" ? body : JSON.stringify(body); - const headers = typeof body === "string" ? undefined : { "Content-Type": "application/json" }; - return new Response(payload, { status, headers }); - }; - - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; - if (url.includes("api.minimaxi.com/v1/api/openplatform/coding_plan/remains")) { - return makeResponse(200, { - base_resp: { status_code: 0, status_msg: "ok" }, - data: { - plan_name: "Coding Plan", - usage: { - prompt_limit: 200, - prompt_remain: 50, - next_reset_time: "2026-01-07T05:00:00Z", - }, + await expectMinimaxUsage( + { + base_resp: { status_code: 0, status_msg: "ok" }, + data: { + plan_name: "Coding Plan", + usage: { + prompt_limit: 200, + prompt_remain: 50, + next_reset_time: "2026-01-07T05:00:00Z", }, - }); - } - return makeResponse(404, "not found"); - }); - - const summary = await loadProviderUsageSummary({ - now: Date.UTC(2026, 0, 7, 0, 0, 0), - auth: [{ provider: "minimax", token: "token-1b" }], - fetch: mockFetch, - }); - - const minimax = summary.providers.find((p) => p.provider === "minimax"); - expect(minimax?.windows[0]?.usedPercent).toBe(75); - expect(minimax?.plan).toBe("Coding Plan"); - expect(mockFetch).toHaveBeenCalled(); + }, + }, + 75, + "Coding Plan", + ); }); it("prefers MiniMax count-based usage when percent looks inverted", async () => { - const makeResponse = (status: number, body: unknown): Response => { - const payload = typeof body === "string" ? body : JSON.stringify(body); - const headers = typeof body === "string" ? undefined : { "Content-Type": "application/json" }; - return new Response(payload, { status, headers }); - }; - - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; - if (url.includes("api.minimaxi.com/v1/api/openplatform/coding_plan/remains")) { - return makeResponse(200, { - base_resp: { status_code: 0, status_msg: "ok" }, - data: { - prompt_limit: 200, - prompt_remain: 150, - usage_percent: 75, - next_reset_time: "2026-01-07T05:00:00Z", - }, - }); - } - return makeResponse(404, "not found"); - }); - - const summary = await loadProviderUsageSummary({ - now: Date.UTC(2026, 0, 7, 0, 0, 0), - auth: [{ provider: "minimax", token: "token-1b" }], - fetch: mockFetch, - }); - - const minimax = summary.providers.find((p) => p.provider === "minimax"); - expect(minimax?.windows[0]?.usedPercent).toBe(25); - expect(mockFetch).toHaveBeenCalled(); + await expectMinimaxUsage( + { + base_resp: { status_code: 0, status_msg: "ok" }, + data: { + prompt_limit: 200, + prompt_remain: 150, + usage_percent: 75, + next_reset_time: "2026-01-07T05:00:00Z", + }, + }, + 25, + ); }); it("handles MiniMax model_remains usage payloads", async () => { - const makeResponse = (status: number, body: unknown): Response => { - const payload = typeof body === "string" ? body : JSON.stringify(body); - const headers = typeof body === "string" ? undefined : { "Content-Type": "application/json" }; - return new Response(payload, { status, headers }); - }; - - const mockFetch = vi.fn, ReturnType>(async (input) => { - const url = - typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; - if (url.includes("api.minimaxi.com/v1/api/openplatform/coding_plan/remains")) { - return makeResponse(200, { - base_resp: { status_code: 0, status_msg: "ok" }, - model_remains: [ - { - start_time: 1736217600, - end_time: 1736235600, - remains_time: 600, - current_interval_total_count: 120, - current_interval_usage_count: 30, - model_name: "MiniMax-M2.1", - }, - ], - }); - } - return makeResponse(404, "not found"); - }); - - const summary = await loadProviderUsageSummary({ - now: Date.UTC(2026, 0, 7, 0, 0, 0), - auth: [{ provider: "minimax", token: "token-1b" }], - fetch: mockFetch, - }); - - const minimax = summary.providers.find((p) => p.provider === "minimax"); - expect(minimax?.windows[0]?.usedPercent).toBe(25); - expect(mockFetch).toHaveBeenCalled(); + await expectMinimaxUsage( + { + base_resp: { status_code: 0, status_msg: "ok" }, + model_remains: [ + { + start_time: 1736217600, + end_time: 1736235600, + remains_time: 600, + current_interval_total_count: 120, + current_interval_usage_count: 30, + model_name: "MiniMax-M2.1", + }, + ], + }, + 25, + ); }); it("discovers Claude usage from token auth profiles", async () => { diff --git a/src/infra/runtime-status.ts b/src/infra/runtime-status.ts new file mode 100644 index 00000000000..110a81084ff --- /dev/null +++ b/src/infra/runtime-status.ts @@ -0,0 +1,28 @@ +type RuntimeStatusFormatInput = { + status?: string; + pid?: number; + state?: string; + details?: string[]; +}; + +export function formatRuntimeStatusWithDetails({ + status, + pid, + state, + details = [], +}: RuntimeStatusFormatInput): string { + const runtimeStatus = status ?? "unknown"; + const fullDetails: string[] = []; + if (pid) { + fullDetails.push(`pid ${pid}`); + } + if (state && state.toLowerCase() !== runtimeStatus) { + fullDetails.push(`state ${state}`); + } + for (const detail of details) { + if (detail) { + fullDetails.push(detail); + } + } + return fullDetails.length > 0 ? `${runtimeStatus} (${fullDetails.join(", ")})` : runtimeStatus; +} diff --git a/src/infra/ssh-config.test.ts b/src/infra/ssh-config.test.ts index 48a8bf310a2..7ea70fb8b8b 100644 --- a/src/infra/ssh-config.test.ts +++ b/src/infra/ssh-config.test.ts @@ -2,20 +2,25 @@ import { spawn } from "node:child_process"; import { EventEmitter } from "node:events"; import { describe, expect, it, vi } from "vitest"; +type MockSpawnChild = EventEmitter & { + stdout?: EventEmitter & { setEncoding?: (enc: string) => void }; + kill?: (signal?: string) => void; +}; + +function createMockSpawnChild() { + const child = new EventEmitter() as MockSpawnChild; + const stdout = new EventEmitter() as MockSpawnChild["stdout"]; + stdout!.setEncoding = vi.fn(); + child.stdout = stdout; + child.kill = vi.fn(); + return { child, stdout }; +} + vi.mock("node:child_process", () => { const spawn = vi.fn(() => { - const child = new EventEmitter() as EventEmitter & { - stdout?: EventEmitter & { setEncoding?: (enc: string) => void }; - kill?: (signal?: string) => void; - }; - const stdout = new EventEmitter() as EventEmitter & { - setEncoding?: (enc: string) => void; - }; - stdout.setEncoding = vi.fn(); - child.stdout = stdout; - child.kill = vi.fn(); + const { child, stdout } = createMockSpawnChild(); process.nextTick(() => { - stdout.emit( + stdout?.emit( "data", [ "user steipete", @@ -60,16 +65,7 @@ describe("ssh-config", () => { it("returns null when ssh -G fails", async () => { spawnMock.mockImplementationOnce(() => { - const child = new EventEmitter() as EventEmitter & { - stdout?: EventEmitter & { setEncoding?: (enc: string) => void }; - kill?: (signal?: string) => void; - }; - const stdout = new EventEmitter() as EventEmitter & { - setEncoding?: (enc: string) => void; - }; - stdout.setEncoding = vi.fn(); - child.stdout = stdout; - child.kill = vi.fn(); + const { child } = createMockSpawnChild(); process.nextTick(() => { child.emit("exit", 1); }); diff --git a/src/infra/tmp-openclaw-dir.test.ts b/src/infra/tmp-openclaw-dir.test.ts index 7dc134af6ee..d4f0d2a2559 100644 --- a/src/infra/tmp-openclaw-dir.test.ts +++ b/src/infra/tmp-openclaw-dir.test.ts @@ -2,26 +2,39 @@ import path from "node:path"; import { describe, expect, it, vi } from "vitest"; import { POSIX_OPENCLAW_TMP_DIR, resolvePreferredOpenClawTmpDir } from "./tmp-openclaw-dir.js"; +function fallbackTmp(uid = 501) { + return path.join("/var/fallback", `openclaw-${uid}`); +} + +function resolveWithMocks(params: { + lstatSync: ReturnType; + accessSync?: ReturnType; + uid?: number; + tmpdirPath?: string; +}) { + const accessSync = params.accessSync ?? vi.fn(); + const mkdirSync = vi.fn(); + const getuid = vi.fn(() => params.uid ?? 501); + const tmpdir = vi.fn(() => params.tmpdirPath ?? "/var/fallback"); + const resolved = resolvePreferredOpenClawTmpDir({ + accessSync, + lstatSync: params.lstatSync, + mkdirSync, + getuid, + tmpdir, + }); + return { resolved, accessSync, lstatSync: params.lstatSync, mkdirSync, tmpdir }; +} + describe("resolvePreferredOpenClawTmpDir", () => { it("prefers /tmp/openclaw when it already exists and is writable", () => { - const accessSync = vi.fn(); const lstatSync = vi.fn(() => ({ isDirectory: () => true, isSymbolicLink: () => false, uid: 501, mode: 0o40700, })); - const mkdirSync = vi.fn(); - const getuid = vi.fn(() => 501); - const tmpdir = vi.fn(() => "/var/fallback"); - - const resolved = resolvePreferredOpenClawTmpDir({ - accessSync, - lstatSync, - mkdirSync, - getuid, - tmpdir, - }); + const { resolved, accessSync, tmpdir } = resolveWithMocks({ lstatSync }); expect(lstatSync).toHaveBeenCalledTimes(1); expect(accessSync).toHaveBeenCalledTimes(1); @@ -30,15 +43,11 @@ describe("resolvePreferredOpenClawTmpDir", () => { }); it("prefers /tmp/openclaw when it does not exist but /tmp is writable", () => { - const accessSync = vi.fn(); const lstatSync = vi.fn(() => { const err = new Error("missing") as Error & { code?: string }; err.code = "ENOENT"; throw err; }); - const mkdirSync = vi.fn(); - const getuid = vi.fn(() => 501); - const tmpdir = vi.fn(() => "/var/fallback"); // second lstat call (after mkdir) should succeed lstatSync.mockImplementationOnce(() => { @@ -53,13 +62,7 @@ describe("resolvePreferredOpenClawTmpDir", () => { mode: 0o40700, })); - const resolved = resolvePreferredOpenClawTmpDir({ - accessSync, - lstatSync, - mkdirSync, - getuid, - tmpdir, - }); + const { resolved, accessSync, mkdirSync, tmpdir } = resolveWithMocks({ lstatSync }); expect(resolved).toBe(POSIX_OPENCLAW_TMP_DIR); expect(accessSync).toHaveBeenCalledWith("/tmp", expect.any(Number)); @@ -68,26 +71,15 @@ describe("resolvePreferredOpenClawTmpDir", () => { }); it("falls back to os.tmpdir()/openclaw when /tmp/openclaw is not a directory", () => { - const accessSync = vi.fn(); const lstatSync = vi.fn(() => ({ isDirectory: () => false, isSymbolicLink: () => false, uid: 501, mode: 0o100644, })); - const mkdirSync = vi.fn(); - const getuid = vi.fn(() => 501); - const tmpdir = vi.fn(() => "/var/fallback"); + const { resolved, tmpdir } = resolveWithMocks({ lstatSync }); - const resolved = resolvePreferredOpenClawTmpDir({ - accessSync, - lstatSync, - mkdirSync, - getuid, - tmpdir, - }); - - expect(resolved).toBe(path.join("/var/fallback", "openclaw-501")); + expect(resolved).toBe(fallbackTmp()); expect(tmpdir).toHaveBeenCalledTimes(1); }); @@ -102,91 +94,53 @@ describe("resolvePreferredOpenClawTmpDir", () => { err.code = "ENOENT"; throw err; }); - const mkdirSync = vi.fn(); - const getuid = vi.fn(() => 501); - const tmpdir = vi.fn(() => "/var/fallback"); - - const resolved = resolvePreferredOpenClawTmpDir({ + const { resolved, tmpdir } = resolveWithMocks({ accessSync, lstatSync, - mkdirSync, - getuid, - tmpdir, }); - expect(resolved).toBe(path.join("/var/fallback", "openclaw-501")); + expect(resolved).toBe(fallbackTmp()); expect(tmpdir).toHaveBeenCalledTimes(1); }); it("falls back when /tmp/openclaw is a symlink", () => { - const accessSync = vi.fn(); const lstatSync = vi.fn(() => ({ isDirectory: () => true, isSymbolicLink: () => true, uid: 501, mode: 0o120777, })); - const mkdirSync = vi.fn(); - const getuid = vi.fn(() => 501); - const tmpdir = vi.fn(() => "/var/fallback"); - const resolved = resolvePreferredOpenClawTmpDir({ - accessSync, - lstatSync, - mkdirSync, - getuid, - tmpdir, - }); + const { resolved, tmpdir } = resolveWithMocks({ lstatSync }); - expect(resolved).toBe(path.join("/var/fallback", "openclaw-501")); + expect(resolved).toBe(fallbackTmp()); expect(tmpdir).toHaveBeenCalledTimes(1); }); it("falls back when /tmp/openclaw is not owned by the current user", () => { - const accessSync = vi.fn(); const lstatSync = vi.fn(() => ({ isDirectory: () => true, isSymbolicLink: () => false, uid: 0, mode: 0o40700, })); - const mkdirSync = vi.fn(); - const getuid = vi.fn(() => 501); - const tmpdir = vi.fn(() => "/var/fallback"); - const resolved = resolvePreferredOpenClawTmpDir({ - accessSync, - lstatSync, - mkdirSync, - getuid, - tmpdir, - }); + const { resolved, tmpdir } = resolveWithMocks({ lstatSync }); - expect(resolved).toBe(path.join("/var/fallback", "openclaw-501")); + expect(resolved).toBe(fallbackTmp()); expect(tmpdir).toHaveBeenCalledTimes(1); }); it("falls back when /tmp/openclaw is group/other writable", () => { - const accessSync = vi.fn(); const lstatSync = vi.fn(() => ({ isDirectory: () => true, isSymbolicLink: () => false, uid: 501, mode: 0o40777, })); - const mkdirSync = vi.fn(); - const getuid = vi.fn(() => 501); - const tmpdir = vi.fn(() => "/var/fallback"); + const { resolved, tmpdir } = resolveWithMocks({ lstatSync }); - const resolved = resolvePreferredOpenClawTmpDir({ - accessSync, - lstatSync, - mkdirSync, - getuid, - tmpdir, - }); - - expect(resolved).toBe(path.join("/var/fallback", "openclaw-501")); + expect(resolved).toBe(fallbackTmp()); expect(tmpdir).toHaveBeenCalledTimes(1); }); }); diff --git a/src/infra/update-runner.test.ts b/src/infra/update-runner.test.ts index ac2b8741428..912a67a1439 100644 --- a/src/infra/update-runner.test.ts +++ b/src/infra/update-runner.test.ts @@ -105,13 +105,28 @@ describe("runGatewayUpdate", () => { }; } - it("skips git update when worktree is dirty", async () => { + async function setupGitCheckout(options?: { packageManager?: string }) { await fs.mkdir(path.join(tempDir, ".git")); - await fs.writeFile( - path.join(tempDir, "package.json"), - JSON.stringify({ name: "openclaw", version: "1.0.0" }), - "utf-8", - ); + const pkg: Record = { name: "openclaw", version: "1.0.0" }; + if (options?.packageManager) { + pkg.packageManager = options.packageManager; + } + await fs.writeFile(path.join(tempDir, "package.json"), JSON.stringify(pkg), "utf-8"); + } + + async function setupUiIndex() { + const uiIndexPath = path.join(tempDir, "dist", "control-ui", "index.html"); + await fs.mkdir(path.dirname(uiIndexPath), { recursive: true }); + await fs.writeFile(uiIndexPath, "", "utf-8"); + return uiIndexPath; + } + + async function removeControlUiAssets() { + await fs.rm(path.join(tempDir, "dist", "control-ui"), { recursive: true, force: true }); + } + + 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" }, @@ -131,12 +146,7 @@ describe("runGatewayUpdate", () => { }); it("aborts rebase on failure", async () => { - await fs.mkdir(path.join(tempDir, ".git")); - await fs.writeFile( - path.join(tempDir, "package.json"), - JSON.stringify({ name: "openclaw", version: "1.0.0" }), - "utf-8", - ); + await setupGitCheckout(); const { runner, calls } = createRunner({ [`git -C ${tempDir} rev-parse --show-toplevel`]: { stdout: tempDir }, [`git -C ${tempDir} rev-parse HEAD`]: { stdout: "abc123" }, @@ -164,15 +174,8 @@ describe("runGatewayUpdate", () => { }); it("uses stable tag when beta tag is older than release", async () => { - await fs.mkdir(path.join(tempDir, ".git")); - await fs.writeFile( - path.join(tempDir, "package.json"), - JSON.stringify({ name: "openclaw", version: "1.0.0", packageManager: "pnpm@8.0.0" }), - "utf-8", - ); - const uiIndexPath = path.join(tempDir, "dist", "control-ui", "index.html"); - await fs.mkdir(path.dirname(uiIndexPath), { recursive: true }); - await fs.writeFile(uiIndexPath, "", "utf-8"); + await setupGitCheckout({ packageManager: "pnpm@8.0.0" }); + await setupUiIndex(); const stableTag = "v1.0.1-1"; const betaTag = "v1.0.0-beta.2"; const { runner, calls } = createRunner({ @@ -243,29 +246,18 @@ describe("runGatewayUpdate", () => { "utf-8", ); - const calls: string[] = []; - const runCommand = async (argv: string[]) => { - const key = argv.join(" "); - calls.push(key); - if (key === `git -C ${pkgRoot} rev-parse --show-toplevel`) { - return { stdout: "", stderr: "not a git repository", code: 128 }; - } - if (key === "npm root -g") { - return { stdout: nodeModules, stderr: "", code: 0 }; - } - if (key === params.expectedInstallCommand) { + const { calls, runCommand } = createGlobalInstallHarness({ + pkgRoot, + npmRootOutput: nodeModules, + installCommand: params.expectedInstallCommand, + onInstall: async () => { await fs.writeFile( path.join(pkgRoot, "package.json"), JSON.stringify({ name: "openclaw", version: "2.0.0" }), "utf-8", ); - return { stdout: "ok", stderr: "", code: 0 }; - } - if (key === "pnpm root -g") { - return { stdout: "", stderr: "", code: 1 }; - } - return { stdout: "", stderr: "", code: 0 }; - }; + }, + }); const result = await runGatewayUpdate({ cwd: pkgRoot, @@ -278,6 +270,37 @@ describe("runGatewayUpdate", () => { return { calls, result }; } + const createGlobalInstallHarness = (params: { + pkgRoot: string; + npmRootOutput?: string; + installCommand: string; + onInstall?: () => Promise; + }) => { + const calls: string[] = []; + const runCommand = async (argv: string[]) => { + const key = argv.join(" "); + calls.push(key); + if (key === `git -C ${params.pkgRoot} rev-parse --show-toplevel`) { + return { stdout: "", stderr: "not a git repository", code: 128 }; + } + if (key === "npm root -g") { + if (params.npmRootOutput) { + return { stdout: params.npmRootOutput, stderr: "", code: 0 }; + } + return { stdout: "", stderr: "", code: 1 }; + } + if (key === "pnpm root -g") { + return { stdout: "", stderr: "", code: 1 }; + } + if (key === params.installCommand) { + await params.onInstall?.(); + return { stdout: "ok", stderr: "", code: 0 }; + } + return { stdout: "", stderr: "", code: 0 }; + }; + return { calls, runCommand }; + }; + it.each([ { title: "updates global npm installs when detected", @@ -364,29 +387,17 @@ describe("runGatewayUpdate", () => { "utf-8", ); - const calls: string[] = []; - const runCommand = async (argv: string[]) => { - const key = argv.join(" "); - calls.push(key); - if (key === `git -C ${pkgRoot} rev-parse --show-toplevel`) { - return { stdout: "", stderr: "not a git repository", code: 128 }; - } - if (key === "npm root -g") { - return { stdout: "", stderr: "", code: 1 }; - } - if (key === "pnpm root -g") { - return { stdout: "", stderr: "", code: 1 }; - } - if (key === "bun add -g openclaw@latest") { + const { calls, runCommand } = createGlobalInstallHarness({ + pkgRoot, + installCommand: "bun add -g openclaw@latest", + onInstall: async () => { await fs.writeFile( path.join(pkgRoot, "package.json"), JSON.stringify({ name: "openclaw", version: "2.0.0" }), "utf-8", ); - return { stdout: "ok", stderr: "", code: 0 }; - } - return { stdout: "", stderr: "", code: 0 }; - }; + }, + }); const result = await runGatewayUpdate({ cwd: pkgRoot, @@ -429,12 +440,7 @@ describe("runGatewayUpdate", () => { }); it("fails with a clear reason when openclaw.mjs is missing", async () => { - await fs.mkdir(path.join(tempDir, ".git")); - await fs.writeFile( - path.join(tempDir, "package.json"), - JSON.stringify({ name: "openclaw", version: "1.0.0", packageManager: "pnpm@8.0.0" }), - "utf-8", - ); + await setupGitCheckout({ packageManager: "pnpm@8.0.0" }); await fs.rm(path.join(tempDir, "openclaw.mjs"), { force: true }); const stableTag = "v1.0.1-1"; @@ -463,15 +469,8 @@ describe("runGatewayUpdate", () => { }); it("repairs UI assets when doctor run removes control-ui files", async () => { - await fs.mkdir(path.join(tempDir, ".git")); - await fs.writeFile( - path.join(tempDir, "package.json"), - JSON.stringify({ name: "openclaw", version: "1.0.0", packageManager: "pnpm@8.0.0" }), - "utf-8", - ); - const uiIndexPath = path.join(tempDir, "dist", "control-ui", "index.html"); - await fs.mkdir(path.dirname(uiIndexPath), { recursive: true }); - await fs.writeFile(uiIndexPath, "", "utf-8"); + await setupGitCheckout({ packageManager: "pnpm@8.0.0" }); + const uiIndexPath = await setupUiIndex(); const stableTag = "v1.0.1-1"; const { runCommand, calls, doctorKey, getUiBuildCount } = createStableTagRunner({ @@ -481,9 +480,7 @@ describe("runGatewayUpdate", () => { await fs.mkdir(path.dirname(uiIndexPath), { recursive: true }); await fs.writeFile(uiIndexPath, `${count}`, "utf-8"); }, - onDoctor: async () => { - await fs.rm(path.join(tempDir, "dist", "control-ui"), { recursive: true, force: true }); - }, + onDoctor: removeControlUiAssets, }); const result = await runGatewayUpdate({ @@ -500,15 +497,8 @@ describe("runGatewayUpdate", () => { }); it("fails when UI assets are still missing after post-doctor repair", async () => { - await fs.mkdir(path.join(tempDir, ".git")); - await fs.writeFile( - path.join(tempDir, "package.json"), - JSON.stringify({ name: "openclaw", version: "1.0.0", packageManager: "pnpm@8.0.0" }), - "utf-8", - ); - const uiIndexPath = path.join(tempDir, "dist", "control-ui", "index.html"); - await fs.mkdir(path.dirname(uiIndexPath), { recursive: true }); - await fs.writeFile(uiIndexPath, "", "utf-8"); + await setupGitCheckout({ packageManager: "pnpm@8.0.0" }); + const uiIndexPath = await setupUiIndex(); const stableTag = "v1.0.1-1"; const { runCommand } = createStableTagRunner({ @@ -520,9 +510,7 @@ describe("runGatewayUpdate", () => { await fs.writeFile(uiIndexPath, "built", "utf-8"); } }, - onDoctor: async () => { - await fs.rm(path.join(tempDir, "dist", "control-ui"), { recursive: true, force: true }); - }, + onDoctor: removeControlUiAssets, }); const result = await runGatewayUpdate({ diff --git a/src/infra/update-startup.test.ts b/src/infra/update-startup.test.ts index 49732e12ce7..4893d063095 100644 --- a/src/infra/update-startup.test.ts +++ b/src/infra/update-startup.test.ts @@ -109,7 +109,7 @@ describe("update-startup", () => { suiteCase = 0; }); - it("logs update hint for npm installs when newer tag exists", async () => { + async function runUpdateCheckAndReadState(channel: "stable" | "beta") { vi.mocked(resolveOpenClawPackageRoot).mockResolvedValue("/opt/openclaw"); vi.mocked(checkUpdateStatus).mockResolvedValue({ root: "/opt/openclaw", @@ -123,49 +123,35 @@ describe("update-startup", () => { const log = { info: vi.fn() }; await runGatewayUpdateCheck({ - cfg: { update: { channel: "stable" } }, + cfg: { update: { channel } }, log, isNixMode: false, allowInTests: true, }); + const statePath = path.join(tempDir, "update-check.json"); + const parsed = JSON.parse(await fs.readFile(statePath, "utf-8")) as { + lastNotifiedVersion?: string; + lastNotifiedTag?: string; + }; + return { log, parsed }; + } + + it("logs update hint for npm installs when newer tag exists", async () => { + const { log, parsed } = await runUpdateCheckAndReadState("stable"); + expect(log.info).toHaveBeenCalledWith( expect.stringContaining("update available (latest): v2.0.0"), ); - - const statePath = path.join(tempDir, "update-check.json"); - const raw = await fs.readFile(statePath, "utf-8"); - const parsed = JSON.parse(raw) as { lastNotifiedVersion?: string }; expect(parsed.lastNotifiedVersion).toBe("2.0.0"); }); it("uses latest when beta tag is older than release", async () => { - vi.mocked(resolveOpenClawPackageRoot).mockResolvedValue("/opt/openclaw"); - vi.mocked(checkUpdateStatus).mockResolvedValue({ - root: "/opt/openclaw", - installKind: "package", - packageManager: "npm", - } satisfies UpdateCheckResult); - vi.mocked(resolveNpmChannelTag).mockResolvedValue({ - tag: "latest", - version: "2.0.0", - }); - - const log = { info: vi.fn() }; - await runGatewayUpdateCheck({ - cfg: { update: { channel: "beta" } }, - log, - isNixMode: false, - allowInTests: true, - }); + const { log, parsed } = await runUpdateCheckAndReadState("beta"); expect(log.info).toHaveBeenCalledWith( expect.stringContaining("update available (latest): v2.0.0"), ); - - const statePath = path.join(tempDir, "update-check.json"); - const raw = await fs.readFile(statePath, "utf-8"); - const parsed = JSON.parse(raw) as { lastNotifiedTag?: string }; expect(parsed.lastNotifiedTag).toBe("latest"); }); diff --git a/src/memory/batch-error-utils.ts b/src/memory/batch-error-utils.ts new file mode 100644 index 00000000000..95a812c3669 --- /dev/null +++ b/src/memory/batch-error-utils.ts @@ -0,0 +1,23 @@ +type BatchOutputErrorLike = { + error?: { message?: string }; + response?: { + body?: { + error?: { message?: string }; + }; + }; +}; + +export function extractBatchErrorMessage(lines: BatchOutputErrorLike[]): string | undefined { + const first = lines.find((line) => line.error?.message || line.response?.body?.error); + return ( + first?.error?.message ?? + (typeof first?.response?.body?.error?.message === "string" + ? first?.response?.body?.error?.message + : undefined) + ); +} + +export function formatUnavailableBatchError(err: unknown): string | undefined { + const message = err instanceof Error ? err.message : String(err); + return message ? `error file unavailable: ${message}` : undefined; +} diff --git a/src/memory/batch-openai.ts b/src/memory/batch-openai.ts index cdce55b0a98..d0c84f7fb08 100644 --- a/src/memory/batch-openai.ts +++ b/src/memory/batch-openai.ts @@ -1,4 +1,5 @@ import type { OpenAiEmbeddingClient } from "./embeddings-openai.js"; +import { extractBatchErrorMessage, formatUnavailableBatchError } from "./batch-error-utils.js"; import { postJsonWithRetry } from "./batch-http.js"; import { applyEmbeddingBatchOutputLine } from "./batch-output.js"; import { buildBatchHeaders, normalizeBatchBaseUrl, splitBatchRequests } from "./batch-utils.js"; @@ -133,16 +134,9 @@ async function readOpenAiBatchError(params: { fileId: params.errorFileId, }); const lines = parseOpenAiBatchOutput(content); - const first = lines.find((line) => line.error?.message || line.response?.body?.error); - const message = - first?.error?.message ?? - (typeof first?.response?.body?.error?.message === "string" - ? first?.response?.body?.error?.message - : undefined); - return message; + return extractBatchErrorMessage(lines); } catch (err) { - const message = err instanceof Error ? err.message : String(err); - return message ? `error file unavailable: ${message}` : undefined; + return formatUnavailableBatchError(err); } } diff --git a/src/memory/batch-voyage.ts b/src/memory/batch-voyage.ts index b2c7f9f4d52..1a9b21a6ad3 100644 --- a/src/memory/batch-voyage.ts +++ b/src/memory/batch-voyage.ts @@ -1,6 +1,7 @@ import { createInterface } from "node:readline"; import { Readable } from "node:stream"; import type { VoyageEmbeddingClient } from "./embeddings-voyage.js"; +import { extractBatchErrorMessage, formatUnavailableBatchError } from "./batch-error-utils.js"; import { postJsonWithRetry } from "./batch-http.js"; import { applyEmbeddingBatchOutputLine } from "./batch-output.js"; import { buildBatchHeaders, normalizeBatchBaseUrl, splitBatchRequests } from "./batch-utils.js"; @@ -128,16 +129,9 @@ async function readVoyageBatchError(params: { .map((line) => line.trim()) .filter(Boolean) .map((line) => JSON.parse(line) as VoyageBatchOutputLine); - const first = lines.find((line) => line.error?.message || line.response?.body?.error); - const message = - first?.error?.message ?? - (typeof first?.response?.body?.error?.message === "string" - ? first?.response?.body?.error?.message - : undefined); - return message; + return extractBatchErrorMessage(lines); } catch (err) { - const message = err instanceof Error ? err.message : String(err); - return message ? `error file unavailable: ${message}` : undefined; + return formatUnavailableBatchError(err); } } diff --git a/src/memory/embedding.test-mocks.ts b/src/memory/embedding.test-mocks.ts index 200ceb9adc3..d288a54b1d1 100644 --- a/src/memory/embedding.test-mocks.ts +++ b/src/memory/embedding.test-mocks.ts @@ -1,4 +1,5 @@ import { vi } from "vitest"; +import "./test-runtime-mocks.js"; // Avoid exporting vitest mock types (TS2742 under pnpm + d.ts emit). // oxlint-disable-next-line typescript/no-explicit-any @@ -24,18 +25,6 @@ export function resetEmbeddingMocks(): void { hoisted.embedQuery.mockImplementation(async () => [0, 1, 0]); } -// Unit tests: avoid importing the real chokidar implementation (native fsevents, etc.). -vi.mock("chokidar", () => ({ - default: { - watch: () => ({ on: () => {}, close: async () => {} }), - }, - watch: () => ({ on: () => {}, close: async () => {} }), -})); - -vi.mock("./sqlite-vec.js", () => ({ - loadSqliteVecExtension: async () => ({ ok: false, error: "sqlite-vec disabled in tests" }), -})); - vi.mock("./embeddings.js", () => ({ createEmbeddingProvider: async () => ({ requestedProvider: "openai", diff --git a/src/memory/embeddings.test.ts b/src/memory/embeddings.test.ts index 73136f8cf48..6da3c5b5bc4 100644 --- a/src/memory/embeddings.test.ts +++ b/src/memory/embeddings.test.ts @@ -328,30 +328,42 @@ describe("local embedding normalization", () => { vi.unstubAllGlobals(); }); - it("normalizes local embeddings to magnitude ~1.0", async () => { - const unnormalizedVector = [2.35, 3.45, 0.63, 4.3, 1.2, 5.1, 2.8, 3.9]; - const resolveModelFileMock = vi.fn(async () => "/fake/model.gguf"); - - importNodeLlamaCppMock.mockResolvedValue({ - getLlama: async () => ({ - loadModel: vi.fn().mockResolvedValue({ - createEmbeddingContext: vi.fn().mockResolvedValue({ - getEmbeddingFor: vi.fn().mockResolvedValue({ - vector: new Float32Array(unnormalizedVector), - }), - }), - }), - }), - resolveModelFile: resolveModelFileMock, - LlamaLogLevel: { error: 0 }, - }); - - const result = await createEmbeddingProvider({ + async function createLocalProviderForTest() { + return createEmbeddingProvider({ config: {} as never, provider: "local", model: "", fallback: "none", }); + } + + function mockSingleLocalEmbeddingVector( + vector: number[], + resolveModelFile: (modelPath: string, modelDirectory?: string) => Promise = async () => + "/fake/model.gguf", + ): void { + importNodeLlamaCppMock.mockResolvedValue({ + getLlama: async () => ({ + loadModel: vi.fn().mockResolvedValue({ + createEmbeddingContext: vi.fn().mockResolvedValue({ + getEmbeddingFor: vi.fn().mockResolvedValue({ + vector: new Float32Array(vector), + }), + }), + }), + }), + resolveModelFile, + LlamaLogLevel: { error: 0 }, + }); + } + + it("normalizes local embeddings to magnitude ~1.0", async () => { + const unnormalizedVector = [2.35, 3.45, 0.63, 4.3, 1.2, 5.1, 2.8, 3.9]; + const resolveModelFileMock = vi.fn(async () => "/fake/model.gguf"); + + mockSingleLocalEmbeddingVector(unnormalizedVector, resolveModelFileMock); + + const result = await createLocalProviderForTest(); const embedding = await result.provider.embedQuery("test query"); @@ -364,26 +376,9 @@ describe("local embedding normalization", () => { it("handles zero vector without division by zero", async () => { const zeroVector = [0, 0, 0, 0]; - importNodeLlamaCppMock.mockResolvedValue({ - getLlama: async () => ({ - loadModel: vi.fn().mockResolvedValue({ - createEmbeddingContext: vi.fn().mockResolvedValue({ - getEmbeddingFor: vi.fn().mockResolvedValue({ - vector: new Float32Array(zeroVector), - }), - }), - }), - }), - resolveModelFile: async () => "/fake/model.gguf", - LlamaLogLevel: { error: 0 }, - }); + mockSingleLocalEmbeddingVector(zeroVector); - const result = await createEmbeddingProvider({ - config: {} as never, - provider: "local", - model: "", - fallback: "none", - }); + const result = await createLocalProviderForTest(); const embedding = await result.provider.embedQuery("test"); @@ -394,26 +389,9 @@ describe("local embedding normalization", () => { it("sanitizes non-finite values before normalization", async () => { const nonFiniteVector = [1, Number.NaN, Number.POSITIVE_INFINITY, Number.NEGATIVE_INFINITY]; - importNodeLlamaCppMock.mockResolvedValue({ - getLlama: async () => ({ - loadModel: vi.fn().mockResolvedValue({ - createEmbeddingContext: vi.fn().mockResolvedValue({ - getEmbeddingFor: vi.fn().mockResolvedValue({ - vector: new Float32Array(nonFiniteVector), - }), - }), - }), - }), - resolveModelFile: async () => "/fake/model.gguf", - LlamaLogLevel: { error: 0 }, - }); + mockSingleLocalEmbeddingVector(nonFiniteVector); - const result = await createEmbeddingProvider({ - config: {} as never, - provider: "local", - model: "", - fallback: "none", - }); + const result = await createLocalProviderForTest(); const embedding = await result.provider.embedQuery("test"); @@ -444,12 +422,7 @@ describe("local embedding normalization", () => { LlamaLogLevel: { error: 0 }, }); - const result = await createEmbeddingProvider({ - config: {} as never, - provider: "local", - model: "", - fallback: "none", - }); + const result = await createLocalProviderForTest(); const embeddings = await result.provider.embedBatch(["text1", "text2", "text3"]); diff --git a/src/memory/index.test.ts b/src/memory/index.test.ts index 4c94ebfc31f..93408ae6f2b 100644 --- a/src/memory/index.test.ts +++ b/src/memory/index.test.ts @@ -3,21 +3,10 @@ import os from "node:os"; import path from "node:path"; import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { getMemorySearchManager, type MemoryIndexManager } from "./index.js"; +import "./test-runtime-mocks.js"; let embedBatchCalls = 0; -// Unit tests: avoid importing the real chokidar implementation (native fsevents, etc.). -vi.mock("chokidar", () => ({ - default: { - watch: () => ({ on: () => {}, close: async () => {} }), - }, - watch: () => ({ on: () => {}, close: async () => {} }), -})); - -vi.mock("./sqlite-vec.js", () => ({ - loadSqliteVecExtension: async () => ({ ok: false, error: "sqlite-vec disabled in tests" }), -})); - vi.mock("./embeddings.js", () => { const embedText = (text: string) => { const lower = text.toLowerCase(); diff --git a/src/memory/manager.async-search.test.ts b/src/memory/manager.async-search.test.ts index 114957bbc4c..660ba15ac90 100644 --- a/src/memory/manager.async-search.test.ts +++ b/src/memory/manager.async-search.test.ts @@ -3,25 +3,17 @@ import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { getMemorySearchManager, type MemoryIndexManager } from "./index.js"; +import { createOpenAIEmbeddingProviderMock } from "./test-embeddings-mock.js"; const embedBatch = vi.fn(async () => []); const embedQuery = vi.fn(async () => [0.2, 0.2, 0.2]); vi.mock("./embeddings.js", () => ({ - createEmbeddingProvider: async () => ({ - requestedProvider: "openai", - provider: { - id: "openai", - model: "text-embedding-3-small", + createEmbeddingProvider: async () => + createOpenAIEmbeddingProviderMock({ embedQuery, embedBatch, - }, - openAi: { - baseUrl: "https://api.openai.com/v1", - headers: { Authorization: "Bearer test", "Content-Type": "application/json" }, - model: "text-embedding-3-small", - }, - }), + }), })); describe("memory search async sync", () => { diff --git a/src/memory/manager.atomic-reindex.test.ts b/src/memory/manager.atomic-reindex.test.ts index 8979f388b7a..e69e520da63 100644 --- a/src/memory/manager.atomic-reindex.test.ts +++ b/src/memory/manager.atomic-reindex.test.ts @@ -2,48 +2,19 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; -import { getMemorySearchManager, type MemoryIndexManager } from "./index.js"; +import type { MemoryIndexManager } from "./index.js"; +import { getEmbedBatchMock, resetEmbeddingMocks } from "./embedding.test-mocks.js"; +import { getRequiredMemoryIndexManager } from "./test-manager-helpers.js"; let shouldFail = false; -vi.mock("chokidar", () => ({ - default: { - watch: vi.fn(() => ({ - on: vi.fn(), - close: vi.fn(async () => undefined), - })), - }, -})); - -vi.mock("./embeddings.js", () => { - return { - createEmbeddingProvider: async () => ({ - requestedProvider: "openai", - provider: { - id: "mock", - model: "mock-embed", - embedQuery: async () => [1, 0, 0], - embedBatch: async (texts: string[]) => { - if (shouldFail) { - throw new Error("embedding failure"); - } - return texts.map((_, index) => [index + 1, 0, 0]); - }, - }, - }), - }; -}); - -vi.mock("./sqlite-vec.js", () => ({ - loadSqliteVecExtension: async () => ({ ok: false, error: "sqlite-vec disabled in tests" }), -})); - describe("memory manager atomic reindex", () => { let fixtureRoot = ""; let caseId = 0; let workspaceDir: string; let indexPath: string; let manager: MemoryIndexManager | null = null; + const embedBatch = getEmbedBatchMock(); beforeAll(async () => { fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-mem-atomic-")); @@ -51,7 +22,14 @@ describe("memory manager atomic reindex", () => { beforeEach(async () => { vi.stubEnv("OPENCLAW_TEST_MEMORY_UNSAFE_REINDEX", "0"); + resetEmbeddingMocks(); shouldFail = false; + embedBatch.mockImplementation(async (texts: string[]) => { + if (shouldFail) { + throw new Error("embedding failure"); + } + return texts.map((_, index) => [index + 1, 0, 0]); + }); workspaceDir = path.join(fixtureRoot, `case-${caseId++}`); await fs.mkdir(workspaceDir, { recursive: true }); indexPath = path.join(workspaceDir, "index.sqlite"); @@ -92,12 +70,7 @@ describe("memory manager atomic reindex", () => { }, }; - const result = await getMemorySearchManager({ cfg, agentId: "main" }); - expect(result.manager).not.toBeNull(); - if (!result.manager) { - throw new Error("manager missing"); - } - manager = result.manager; + manager = await getRequiredMemoryIndexManager({ cfg, agentId: "main" }); await manager.sync({ force: true }); const beforeStatus = manager.status(); diff --git a/src/memory/manager.batch.test.ts b/src/memory/manager.batch.test.ts index b990380db43..c5cdff00677 100644 --- a/src/memory/manager.batch.test.ts +++ b/src/memory/manager.batch.test.ts @@ -3,37 +3,18 @@ import os from "node:os"; import path from "node:path"; import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { getMemorySearchManager, type MemoryIndexManager } from "./index.js"; +import { createOpenAIEmbeddingProviderMock } from "./test-embeddings-mock.js"; +import "./test-runtime-mocks.js"; const embedBatch = vi.fn(async () => []); const embedQuery = vi.fn(async () => [0.5, 0.5, 0.5]); -// Unit tests: avoid importing the real chokidar implementation (native fsevents, etc.). -vi.mock("chokidar", () => ({ - default: { - watch: () => ({ on: () => {}, close: async () => {} }), - }, - watch: () => ({ on: () => {}, close: async () => {} }), -})); - -vi.mock("./sqlite-vec.js", () => ({ - loadSqliteVecExtension: async () => ({ ok: false, error: "sqlite-vec disabled in tests" }), -})); - vi.mock("./embeddings.js", () => ({ - createEmbeddingProvider: async () => ({ - requestedProvider: "openai", - provider: { - id: "openai", - model: "text-embedding-3-small", + createEmbeddingProvider: async () => + createOpenAIEmbeddingProviderMock({ embedQuery, embedBatch, - }, - openAi: { - baseUrl: "https://api.openai.com/v1", - headers: { Authorization: "Bearer test", "Content-Type": "application/json" }, - model: "text-embedding-3-small", - }, - }), + }), })); describe("memory indexing with OpenAI batches", () => { diff --git a/src/memory/manager.sync-errors-do-not-crash.test.ts b/src/memory/manager.sync-errors-do-not-crash.test.ts index faa56cc11f3..4103521ca93 100644 --- a/src/memory/manager.sync-errors-do-not-crash.test.ts +++ b/src/memory/manager.sync-errors-do-not-crash.test.ts @@ -2,40 +2,22 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { getMemorySearchManager, type MemoryIndexManager } from "./index.js"; - -vi.mock("chokidar", () => ({ - default: { - watch: vi.fn(() => ({ - on: vi.fn(), - close: vi.fn(async () => undefined), - })), - }, -})); - -vi.mock("./embeddings.js", () => { - return { - createEmbeddingProvider: async () => ({ - requestedProvider: "openai", - provider: { - id: "mock", - model: "mock-embed", - embedQuery: async () => [0, 0, 0], - embedBatch: async () => { - throw new Error("openai embeddings failed: 400 bad request"); - }, - }, - }), - }; -}); +import type { MemoryIndexManager } from "./index.js"; +import { getEmbedBatchMock, resetEmbeddingMocks } from "./embedding.test-mocks.js"; +import { getRequiredMemoryIndexManager } from "./test-manager-helpers.js"; describe("memory manager sync failures", () => { let workspaceDir: string; let indexPath: string; let manager: MemoryIndexManager | null = null; + const embedBatch = getEmbedBatchMock(); beforeEach(async () => { vi.useFakeTimers(); + resetEmbeddingMocks(); + embedBatch.mockImplementation(async () => { + throw new Error("openai embeddings failed: 400 bad request"); + }); workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-mem-")); indexPath = path.join(workspaceDir, "index.sqlite"); await fs.mkdir(path.join(workspaceDir, "memory")); @@ -73,12 +55,7 @@ describe("memory manager sync failures", () => { }, }; - const result = await getMemorySearchManager({ cfg, agentId: "main" }); - expect(result.manager).not.toBeNull(); - if (!result.manager) { - throw new Error("manager missing"); - } - manager = result.manager; + manager = await getRequiredMemoryIndexManager({ cfg, agentId: "main" }); const syncSpy = vi.spyOn(manager, "sync"); // Call the internal scheduler directly; it uses fire-and-forget sync. diff --git a/src/memory/search-manager.test.ts b/src/memory/search-manager.test.ts index fd51bb35a3e..e313cc98a66 100644 --- a/src/memory/search-manager.test.ts +++ b/src/memory/search-manager.test.ts @@ -70,6 +70,31 @@ vi.mock("./manager.js", () => ({ import { QmdMemoryManager } from "./qmd-manager.js"; import { getMemorySearchManager } from "./search-manager.js"; +type SearchManagerResult = Awaited>; +type SearchManager = NonNullable; + +function createQmdCfg(agentId: string) { + return { + memory: { backend: "qmd", qmd: {} }, + agents: { list: [{ id: agentId, default: true, workspace: "/tmp/workspace" }] }, + } as const; +} + +function requireManager(result: SearchManagerResult): SearchManager { + expect(result.manager).toBeTruthy(); + if (!result.manager) { + throw new Error("manager missing"); + } + return result.manager; +} + +async function createFailedQmdSearchHarness(params: { agentId: string; errorMessage: string }) { + const cfg = createQmdCfg(params.agentId); + mockPrimary.search.mockRejectedValueOnce(new Error(params.errorMessage)); + const first = await getMemorySearchManager({ cfg, agentId: params.agentId }); + return { cfg, manager: requireManager(first), firstResult: first }; +} + beforeEach(() => { mockPrimary.search.mockClear(); mockPrimary.readFile.mockClear(); @@ -92,10 +117,7 @@ beforeEach(() => { describe("getMemorySearchManager caching", () => { it("reuses the same QMD manager instance for repeated calls", async () => { - const cfg = { - memory: { backend: "qmd", qmd: {} }, - agents: { list: [{ id: "main", default: true, workspace: "/tmp/workspace" }] }, - } as const; + const cfg = createQmdCfg("main"); const first = await getMemorySearchManager({ cfg, agentId: "main" }); const second = await getMemorySearchManager({ cfg, agentId: "main" }); @@ -107,24 +129,21 @@ describe("getMemorySearchManager caching", () => { it("evicts failed qmd wrapper so next call retries qmd", async () => { const retryAgentId = "retry-agent"; - const cfg = { - memory: { backend: "qmd", qmd: {} }, - agents: { list: [{ id: retryAgentId, default: true, workspace: "/tmp/workspace" }] }, - } as const; + const { + cfg, + manager: firstManager, + firstResult: first, + } = await createFailedQmdSearchHarness({ + agentId: retryAgentId, + errorMessage: "qmd query failed", + }); - mockPrimary.search.mockRejectedValueOnce(new Error("qmd query failed")); - const first = await getMemorySearchManager({ cfg, agentId: retryAgentId }); - expect(first.manager).toBeTruthy(); - if (!first.manager) { - throw new Error("manager missing"); - } - - const fallbackResults = await first.manager.search("hello"); + const fallbackResults = await firstManager.search("hello"); expect(fallbackResults).toHaveLength(1); expect(fallbackResults[0]?.path).toBe("MEMORY.md"); const second = await getMemorySearchManager({ cfg, agentId: retryAgentId }); - expect(second.manager).toBeTruthy(); + requireManager(second); expect(second.manager).not.toBe(first.manager); // eslint-disable-next-line @typescript-eslint/unbound-method expect(QmdMemoryManager.create).toHaveBeenCalledTimes(2); @@ -132,16 +151,13 @@ describe("getMemorySearchManager caching", () => { it("does not cache status-only qmd managers", async () => { const agentId = "status-agent"; - const cfg = { - memory: { backend: "qmd", qmd: {} }, - agents: { list: [{ id: agentId, default: true, workspace: "/tmp/workspace" }] }, - } as const; + const cfg = createQmdCfg(agentId); const first = await getMemorySearchManager({ cfg, agentId, purpose: "status" }); const second = await getMemorySearchManager({ cfg, agentId, purpose: "status" }); - expect(first.manager).toBeTruthy(); - expect(second.manager).toBeTruthy(); + requireManager(first); + requireManager(second); // eslint-disable-next-line @typescript-eslint/unbound-method expect(QmdMemoryManager.create).toHaveBeenCalledTimes(2); // eslint-disable-next-line @typescript-eslint/unbound-method @@ -158,53 +174,36 @@ describe("getMemorySearchManager caching", () => { it("does not evict a newer cached wrapper when closing an older failed wrapper", async () => { const retryAgentId = "retry-agent-close"; - const cfg = { - memory: { backend: "qmd", qmd: {} }, - agents: { list: [{ id: retryAgentId, default: true, workspace: "/tmp/workspace" }] }, - } as const; - - mockPrimary.search.mockRejectedValueOnce(new Error("qmd query failed")); - - const first = await getMemorySearchManager({ cfg, agentId: retryAgentId }); - expect(first.manager).toBeTruthy(); - if (!first.manager) { - throw new Error("manager missing"); - } - await first.manager.search("hello"); + const { + cfg, + manager: firstManager, + firstResult: first, + } = await createFailedQmdSearchHarness({ + agentId: retryAgentId, + errorMessage: "qmd query failed", + }); + await firstManager.search("hello"); const second = await getMemorySearchManager({ cfg, agentId: retryAgentId }); - expect(second.manager).toBeTruthy(); - if (!second.manager) { - throw new Error("manager missing"); - } + const secondManager = requireManager(second); expect(second.manager).not.toBe(first.manager); - await first.manager.close?.(); + await firstManager.close?.(); const third = await getMemorySearchManager({ cfg, agentId: retryAgentId }); - expect(third.manager).toBe(second.manager); + expect(third.manager).toBe(secondManager); // eslint-disable-next-line @typescript-eslint/unbound-method expect(QmdMemoryManager.create).toHaveBeenCalledTimes(2); }); it("falls back to builtin search when qmd fails with sqlite busy", async () => { const retryAgentId = "retry-agent-busy"; - const cfg = { - memory: { backend: "qmd", qmd: {} }, - agents: { list: [{ id: retryAgentId, default: true, workspace: "/tmp/workspace" }] }, - } as const; + const { manager: firstManager } = await createFailedQmdSearchHarness({ + agentId: retryAgentId, + errorMessage: "qmd index busy while reading results: SQLITE_BUSY: database is locked", + }); - mockPrimary.search.mockRejectedValueOnce( - new Error("qmd index busy while reading results: SQLITE_BUSY: database is locked"), - ); - - const first = await getMemorySearchManager({ cfg, agentId: retryAgentId }); - expect(first.manager).toBeTruthy(); - if (!first.manager) { - throw new Error("manager missing"); - } - - const results = await first.manager.search("hello"); + const results = await firstManager.search("hello"); expect(results).toHaveLength(1); expect(results[0]?.path).toBe("MEMORY.md"); expect(fallbackSearch).toHaveBeenCalledTimes(1); @@ -212,19 +211,12 @@ describe("getMemorySearchManager caching", () => { it("keeps original qmd error when fallback manager initialization fails", async () => { const retryAgentId = "retry-agent-no-fallback-auth"; - const cfg = { - memory: { backend: "qmd", qmd: {} }, - agents: { list: [{ id: retryAgentId, default: true, workspace: "/tmp/workspace" }] }, - } as const; - - mockPrimary.search.mockRejectedValueOnce(new Error("qmd query failed")); + const { manager: firstManager } = await createFailedQmdSearchHarness({ + agentId: retryAgentId, + errorMessage: "qmd query failed", + }); mockMemoryIndexGet.mockRejectedValueOnce(new Error("No API key found for provider openai")); - const first = await getMemorySearchManager({ cfg, agentId: retryAgentId }); - if (!first.manager) { - throw new Error("manager missing"); - } - - await expect(first.manager.search("hello")).rejects.toThrow("qmd query failed"); + await expect(firstManager.search("hello")).rejects.toThrow("qmd query failed"); }); }); diff --git a/src/memory/test-embeddings-mock.ts b/src/memory/test-embeddings-mock.ts new file mode 100644 index 00000000000..5d2d4220cbb --- /dev/null +++ b/src/memory/test-embeddings-mock.ts @@ -0,0 +1,19 @@ +export function createOpenAIEmbeddingProviderMock(params: { + embedQuery: (input: string) => Promise; + embedBatch: (input: string[]) => Promise; +}) { + return { + requestedProvider: "openai", + provider: { + id: "openai", + model: "text-embedding-3-small", + embedQuery: params.embedQuery, + embedBatch: params.embedBatch, + }, + openAi: { + baseUrl: "https://api.openai.com/v1", + headers: { Authorization: "Bearer test", "Content-Type": "application/json" }, + model: "text-embedding-3-small", + }, + }; +} diff --git a/src/memory/test-manager-helpers.ts b/src/memory/test-manager-helpers.ts new file mode 100644 index 00000000000..4bbcf2d608e --- /dev/null +++ b/src/memory/test-manager-helpers.ts @@ -0,0 +1,19 @@ +import type { OpenClawConfig } from "../config/config.js"; +import { getMemorySearchManager, type MemoryIndexManager } from "./index.js"; + +export async function getRequiredMemoryIndexManager(params: { + cfg: OpenClawConfig; + agentId?: string; +}): Promise { + const result = await getMemorySearchManager({ + cfg: params.cfg, + agentId: params.agentId ?? "main", + }); + if (!result.manager) { + throw new Error("manager missing"); + } + if (!("sync" in result.manager) || typeof result.manager.sync !== "function") { + throw new Error("manager does not support sync"); + } + return result.manager as unknown as MemoryIndexManager; +} diff --git a/src/memory/test-runtime-mocks.ts b/src/memory/test-runtime-mocks.ts new file mode 100644 index 00000000000..044ad26998b --- /dev/null +++ b/src/memory/test-runtime-mocks.ts @@ -0,0 +1,13 @@ +import { vi } from "vitest"; + +// Unit tests: avoid importing the real chokidar implementation (native fsevents, etc.). +vi.mock("chokidar", () => ({ + default: { + watch: () => ({ on: () => {}, close: async () => {} }), + }, + watch: () => ({ on: () => {}, close: async () => {} }), +})); + +vi.mock("./sqlite-vec.js", () => ({ + loadSqliteVecExtension: async () => ({ ok: false, error: "sqlite-vec disabled in tests" }), +})); diff --git a/src/pairing/pairing-store.ts b/src/pairing/pairing-store.ts index 428acc289d4..4b72bc58476 100644 --- a/src/pairing/pairing-store.ts +++ b/src/pairing/pairing-store.ts @@ -7,7 +7,7 @@ import { getPairingAdapter } from "../channels/plugins/pairing.js"; import { resolveOAuthDir, resolveStateDir } from "../config/paths.js"; import { withFileLock as withPathLock } from "../infra/file-lock.js"; import { resolveRequiredHomeDir } from "../infra/home-dir.js"; -import { safeParseJson } from "../utils.js"; +import { readJsonFileWithFallback, writeJsonFileAtomically } from "../plugin-sdk/json-store.js"; const PAIRING_CODE_LENGTH = 8; const PAIRING_CODE_ALPHABET = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789"; @@ -98,31 +98,26 @@ async function readJsonFile( filePath: string, fallback: T, ): Promise<{ value: T; exists: boolean }> { - try { - const raw = await fs.promises.readFile(filePath, "utf-8"); - const parsed = safeParseJson(raw); - if (parsed == null) { - return { value: fallback, exists: true }; - } - return { value: parsed, exists: true }; - } catch (err) { - const code = (err as { code?: string }).code; - if (code === "ENOENT") { - return { value: fallback, exists: false }; - } - return { value: fallback, exists: false }; - } + return await readJsonFileWithFallback(filePath, fallback); } async function writeJsonFile(filePath: string, value: unknown): Promise { - const dir = path.dirname(filePath); - await fs.promises.mkdir(dir, { recursive: true, mode: 0o700 }); - const tmp = path.join(dir, `${path.basename(filePath)}.${crypto.randomUUID()}.tmp`); - await fs.promises.writeFile(tmp, `${JSON.stringify(value, null, 2)}\n`, { - encoding: "utf-8", + await writeJsonFileAtomically(filePath, value); +} + +async function readPairingRequests(filePath: string): Promise { + const { value } = await readJsonFile(filePath, { + version: 1, + requests: [], }); - await fs.promises.chmod(tmp, 0o600); - await fs.promises.rename(tmp, filePath); + return Array.isArray(value.requests) ? value.requests : []; +} + +async function readPrunedPairingRequests(filePath: string): Promise<{ + requests: PairingRequest[]; + removed: boolean; +}> { + return pruneExpiredRequests(await readPairingRequests(filePath), Date.now()); } async function ensureJsonFile(filePath: string, fallback: unknown) { @@ -208,6 +203,21 @@ function generateUniqueCode(existing: Set): string { throw new Error("failed to generate unique pairing code"); } +function normalizePairingAccountId(accountId?: string): string { + return accountId?.trim().toLowerCase() || ""; +} + +function requestMatchesAccountId(entry: PairingRequest, normalizedAccountId: string): boolean { + if (!normalizedAccountId) { + return true; + } + return ( + String(entry.meta?.accountId ?? "") + .trim() + .toLowerCase() === normalizedAccountId + ); +} + function normalizeId(value: string | number): string { return String(value).trim(); } @@ -331,17 +341,35 @@ export async function readChannelAllowFromStore( return dedupePreserveOrder([...scopedEntries, ...legacyEntries]); } +type AllowFromStoreEntryUpdateParams = { + channel: PairingChannel; + entry: string | number; + accountId?: string; + env?: NodeJS.ProcessEnv; +}; + +async function updateChannelAllowFromStore( + params: { + apply: (current: string[], normalized: string) => string[] | null; + } & AllowFromStoreEntryUpdateParams, +): Promise<{ changed: boolean; allowFrom: string[] }> { + return await updateAllowFromStoreEntry({ + channel: params.channel, + entry: params.entry, + accountId: params.accountId, + env: params.env, + apply: params.apply, + }); +} + export async function addChannelAllowFromStoreEntry(params: { channel: PairingChannel; entry: string | number; accountId?: string; env?: NodeJS.ProcessEnv; }): Promise<{ changed: boolean; allowFrom: string[] }> { - return await updateAllowFromStoreEntry({ - channel: params.channel, - entry: params.entry, - accountId: params.accountId, - env: params.env, + return await updateChannelAllowFromStore({ + ...params, apply: (current, normalized) => { if (current.includes(normalized)) { return null; @@ -357,11 +385,8 @@ export async function removeChannelAllowFromStoreEntry(params: { accountId?: string; env?: NodeJS.ProcessEnv; }): Promise<{ changed: boolean; allowFrom: string[] }> { - return await updateAllowFromStoreEntry({ - channel: params.channel, - entry: params.entry, - accountId: params.accountId, - env: params.env, + return await updateChannelAllowFromStore({ + ...params, apply: (current, normalized) => { const next = current.filter((entry) => entry !== normalized); if (next.length === current.length) { @@ -382,16 +407,8 @@ export async function listChannelPairingRequests( filePath, { version: 1, requests: [] } satisfies PairingStore, async () => { - const { value } = await readJsonFile(filePath, { - version: 1, - requests: [], - }); - const reqs = Array.isArray(value.requests) ? value.requests : []; - const nowMs = Date.now(); - const { requests: prunedExpired, removed: expiredRemoved } = pruneExpiredRequests( - reqs, - nowMs, - ); + const { requests: prunedExpired, removed: expiredRemoved } = + await readPrunedPairingRequests(filePath); const { requests: pruned, removed: cappedRemoved } = pruneExcessRequests( prunedExpired, PAIRING_PENDING_MAX, @@ -402,14 +419,9 @@ export async function listChannelPairingRequests( requests: pruned, } satisfies PairingStore); } - const normalizedAccountId = accountId?.trim().toLowerCase() || ""; + const normalizedAccountId = normalizePairingAccountId(accountId); const filtered = normalizedAccountId - ? pruned.filter( - (entry) => - String(entry.meta?.accountId ?? "") - .trim() - .toLowerCase() === normalizedAccountId, - ) + ? pruned.filter((entry) => requestMatchesAccountId(entry, normalizedAccountId)) : pruned; return filtered .filter( @@ -440,10 +452,6 @@ export async function upsertChannelPairingRequest(params: { filePath, { version: 1, requests: [] } satisfies PairingStore, async () => { - const { value } = await readJsonFile(filePath, { - version: 1, - requests: [], - }); const now = new Date().toISOString(); const nowMs = Date.now(); const id = normalizeId(params.id); @@ -458,7 +466,7 @@ export async function upsertChannelPairingRequest(params: { : undefined; const meta = normalizedAccountId ? { ...baseMeta, accountId: normalizedAccountId } : baseMeta; - let reqs = Array.isArray(value.requests) ? value.requests : []; + let reqs = await readPairingRequests(filePath); const { requests: prunedExpired, removed: expiredRemoved } = pruneExpiredRequests( reqs, nowMs, @@ -542,26 +550,13 @@ export async function approveChannelPairingCode(params: { filePath, { version: 1, requests: [] } satisfies PairingStore, async () => { - const { value } = await readJsonFile(filePath, { - version: 1, - requests: [], - }); - const reqs = Array.isArray(value.requests) ? value.requests : []; - const nowMs = Date.now(); - const { requests: pruned, removed } = pruneExpiredRequests(reqs, nowMs); - const normalizedAccountId = params.accountId?.trim().toLowerCase() || ""; + const { requests: pruned, removed } = await readPrunedPairingRequests(filePath); + const normalizedAccountId = normalizePairingAccountId(params.accountId); const idx = pruned.findIndex((r) => { if (String(r.code ?? "").toUpperCase() !== code) { return false; } - if (!normalizedAccountId) { - return true; - } - return ( - String(r.meta?.accountId ?? "") - .trim() - .toLowerCase() === normalizedAccountId - ); + return requestMatchesAccountId(r, normalizedAccountId); }); if (idx < 0) { if (removed) { diff --git a/src/plugin-sdk/allow-from.ts b/src/plugin-sdk/allow-from.ts index 17188bc7d11..c349caa017e 100644 --- a/src/plugin-sdk/allow-from.ts +++ b/src/plugin-sdk/allow-from.ts @@ -8,3 +8,57 @@ export function formatAllowFromLowercase(params: { .map((entry) => (params.stripPrefixRe ? entry.replace(params.stripPrefixRe, "") : entry)) .map((entry) => entry.toLowerCase()); } + +type ParsedChatAllowTarget = + | { kind: "chat_id"; chatId: number } + | { kind: "chat_guid"; chatGuid: string } + | { kind: "chat_identifier"; chatIdentifier: string } + | { kind: "handle"; handle: string }; + +export function isAllowedParsedChatSender(params: { + allowFrom: Array; + sender: string; + chatId?: number | null; + chatGuid?: string | null; + chatIdentifier?: string | null; + normalizeSender: (sender: string) => string; + parseAllowTarget: (entry: string) => TParsed; +}): boolean { + const allowFrom = params.allowFrom.map((entry) => String(entry).trim()); + if (allowFrom.length === 0) { + return true; + } + if (allowFrom.includes("*")) { + return true; + } + + const senderNormalized = params.normalizeSender(params.sender); + const chatId = params.chatId ?? undefined; + const chatGuid = params.chatGuid?.trim(); + const chatIdentifier = params.chatIdentifier?.trim(); + + for (const entry of allowFrom) { + if (!entry) { + continue; + } + const parsed = params.parseAllowTarget(entry); + if (parsed.kind === "chat_id" && chatId !== undefined) { + if (parsed.chatId === chatId) { + return true; + } + } else if (parsed.kind === "chat_guid" && chatGuid) { + if (parsed.chatGuid === chatGuid) { + return true; + } + } else if (parsed.kind === "chat_identifier" && chatIdentifier) { + if (parsed.chatIdentifier === chatIdentifier) { + return true; + } + } else if (parsed.kind === "handle" && senderNormalized) { + if (parsed.handle === senderNormalized) { + return true; + } + } + } + return false; +} diff --git a/src/plugin-sdk/command-auth.ts b/src/plugin-sdk/command-auth.ts new file mode 100644 index 00000000000..135846f6378 --- /dev/null +++ b/src/plugin-sdk/command-auth.ts @@ -0,0 +1,50 @@ +import type { OpenClawConfig } from "../config/config.js"; + +export type ResolveSenderCommandAuthorizationParams = { + cfg: OpenClawConfig; + rawBody: string; + isGroup: boolean; + dmPolicy: string; + configuredAllowFrom: string[]; + senderId: string; + isSenderAllowed: (senderId: string, allowFrom: string[]) => boolean; + readAllowFromStore: () => Promise; + shouldComputeCommandAuthorized: (rawBody: string, cfg: OpenClawConfig) => boolean; + resolveCommandAuthorizedFromAuthorizers: (params: { + useAccessGroups: boolean; + authorizers: Array<{ configured: boolean; allowed: boolean }>; + }) => boolean; +}; + +export async function resolveSenderCommandAuthorization( + params: ResolveSenderCommandAuthorizationParams, +): Promise<{ + shouldComputeAuth: boolean; + effectiveAllowFrom: string[]; + senderAllowedForCommands: boolean; + commandAuthorized: boolean | undefined; +}> { + const shouldComputeAuth = params.shouldComputeCommandAuthorized(params.rawBody, params.cfg); + const storeAllowFrom = + !params.isGroup && (params.dmPolicy !== "open" || shouldComputeAuth) + ? await params.readAllowFromStore().catch(() => []) + : []; + const effectiveAllowFrom = [...params.configuredAllowFrom, ...storeAllowFrom]; + const useAccessGroups = params.cfg.commands?.useAccessGroups !== false; + const senderAllowedForCommands = params.isSenderAllowed(params.senderId, effectiveAllowFrom); + const commandAuthorized = shouldComputeAuth + ? params.resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups, + authorizers: [ + { configured: effectiveAllowFrom.length > 0, allowed: senderAllowedForCommands }, + ], + }) + : undefined; + + return { + shouldComputeAuth, + effectiveAllowFrom, + senderAllowedForCommands, + commandAuthorized, + }; +} diff --git a/src/plugin-sdk/index.ts b/src/plugin-sdk/index.ts index 662a4fec95e..e415f441892 100644 --- a/src/plugin-sdk/index.ts +++ b/src/plugin-sdk/index.ts @@ -84,6 +84,11 @@ export type { OpenClawConfig as ClawdbotConfig } from "../config/config.js"; export type { FileLockHandle, FileLockOptions } from "./file-lock.js"; export { acquireFileLock, withFileLock } from "./file-lock.js"; export { normalizeWebhookPath, resolveWebhookPath } from "./webhook-path.js"; +export { + registerWebhookTarget, + rejectNonPostWebhookRequest, + resolveWebhookTargets, +} from "./webhook-targets.js"; export type { AgentMediaPayload } from "./agent-media-payload.js"; export { buildAgentMediaPayload } from "./agent-media-payload.js"; export { @@ -141,9 +146,13 @@ export { ToolPolicySchema } from "../config/zod-schema.agent-runtime.js"; export type { RuntimeEnv } from "../runtime.js"; export type { WizardPrompter } from "../wizard/prompts.js"; export { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "../routing/session-key.js"; -export { formatAllowFromLowercase } from "./allow-from.js"; +export { formatAllowFromLowercase, isAllowedParsedChatSender } from "./allow-from.js"; +export { resolveSenderCommandAuthorization } from "./command-auth.js"; +export { handleSlackMessageAction } from "./slack-message-actions.js"; +export { extractToolSend } from "./tool-send.js"; export { resolveChannelAccountConfigBasePath } from "./config-paths.js"; export { chunkTextForOutbound } from "./text-chunking.js"; +export { readJsonFileWithFallback, writeJsonFileAtomically } from "./json-store.js"; export type { ChatType } from "../channels/chat-type.js"; /** @deprecated Use ChatType instead */ export type { RoutePeerKind } from "../routing/resolve-route.js"; @@ -173,6 +182,7 @@ export { export { fetchWithSsrFGuard } from "../infra/net/fetch-guard.js"; export { SsrFBlockedError, isBlockedHostname, isPrivateIpAddress } from "../infra/net/ssrf.js"; export type { LookupFn, SsrFPolicy } from "../infra/net/ssrf.js"; +export { rawDataToString } from "../infra/ws.js"; export { isWSLSync, isWSL2Sync, isWSLEnv } from "../infra/wsl.js"; export { isTruthyEnvValue } from "../infra/env.js"; export { resolveToolsBySender } from "../config/group-policy.js"; diff --git a/src/plugin-sdk/json-store.ts b/src/plugin-sdk/json-store.ts new file mode 100644 index 00000000000..e768aea8ada --- /dev/null +++ b/src/plugin-sdk/json-store.ts @@ -0,0 +1,35 @@ +import crypto from "node:crypto"; +import fs from "node:fs"; +import path from "node:path"; +import { safeParseJson } from "../utils.js"; + +export async function readJsonFileWithFallback( + filePath: string, + fallback: T, +): Promise<{ value: T; exists: boolean }> { + try { + const raw = await fs.promises.readFile(filePath, "utf-8"); + const parsed = safeParseJson(raw); + if (parsed == null) { + return { value: fallback, exists: true }; + } + return { value: parsed, exists: true }; + } catch (err) { + const code = (err as { code?: string }).code; + if (code === "ENOENT") { + return { value: fallback, exists: false }; + } + return { value: fallback, exists: false }; + } +} + +export async function writeJsonFileAtomically(filePath: string, value: unknown): Promise { + const dir = path.dirname(filePath); + await fs.promises.mkdir(dir, { recursive: true, mode: 0o700 }); + const tmp = path.join(dir, `${path.basename(filePath)}.${crypto.randomUUID()}.tmp`); + await fs.promises.writeFile(tmp, `${JSON.stringify(value, null, 2)}\n`, { + encoding: "utf-8", + }); + await fs.promises.chmod(tmp, 0o600); + await fs.promises.rename(tmp, filePath); +} diff --git a/src/plugin-sdk/slack-message-actions.ts b/src/plugin-sdk/slack-message-actions.ts new file mode 100644 index 00000000000..a98557d2c47 --- /dev/null +++ b/src/plugin-sdk/slack-message-actions.ts @@ -0,0 +1,162 @@ +import type { AgentToolResult } from "@mariozechner/pi-agent-core"; +import type { ChannelMessageActionContext } from "../channels/plugins/types.js"; +import { readNumberParam, readStringParam } from "../agents/tools/common.js"; + +type SlackActionInvoke = ( + action: Record, + cfg: ChannelMessageActionContext["cfg"], + toolContext?: ChannelMessageActionContext["toolContext"], +) => Promise>; + +export async function handleSlackMessageAction(params: { + providerId: string; + ctx: ChannelMessageActionContext; + invoke: SlackActionInvoke; + normalizeChannelId?: (channelId: string) => string; + includeReadThreadId?: boolean; +}): Promise> { + const { providerId, ctx, invoke, normalizeChannelId, includeReadThreadId = false } = params; + const { action, cfg, params: actionParams } = ctx; + const accountId = ctx.accountId ?? undefined; + const resolveChannelId = () => { + const channelId = + readStringParam(actionParams, "channelId") ?? + readStringParam(actionParams, "to", { required: true }); + return normalizeChannelId ? normalizeChannelId(channelId) : channelId; + }; + + if (action === "send") { + const to = readStringParam(actionParams, "to", { required: true }); + const content = readStringParam(actionParams, "message", { + required: true, + allowEmpty: true, + }); + const mediaUrl = readStringParam(actionParams, "media", { trim: false }); + const threadId = readStringParam(actionParams, "threadId"); + const replyTo = readStringParam(actionParams, "replyTo"); + return await invoke( + { + action: "sendMessage", + to, + content, + mediaUrl: mediaUrl ?? undefined, + accountId, + threadTs: threadId ?? replyTo ?? undefined, + }, + cfg, + ctx.toolContext, + ); + } + + if (action === "react") { + const messageId = readStringParam(actionParams, "messageId", { + required: true, + }); + const emoji = readStringParam(actionParams, "emoji", { allowEmpty: true }); + const remove = typeof actionParams.remove === "boolean" ? actionParams.remove : undefined; + return await invoke( + { + action: "react", + channelId: resolveChannelId(), + messageId, + emoji, + remove, + accountId, + }, + cfg, + ); + } + + if (action === "reactions") { + const messageId = readStringParam(actionParams, "messageId", { + required: true, + }); + const limit = readNumberParam(actionParams, "limit", { integer: true }); + return await invoke( + { + action: "reactions", + channelId: resolveChannelId(), + messageId, + limit, + accountId, + }, + cfg, + ); + } + + if (action === "read") { + const limit = readNumberParam(actionParams, "limit", { integer: true }); + const readAction: Record = { + action: "readMessages", + channelId: resolveChannelId(), + limit, + before: readStringParam(actionParams, "before"), + after: readStringParam(actionParams, "after"), + accountId, + }; + if (includeReadThreadId) { + readAction.threadId = readStringParam(actionParams, "threadId"); + } + return await invoke(readAction, cfg); + } + + if (action === "edit") { + const messageId = readStringParam(actionParams, "messageId", { + required: true, + }); + const content = readStringParam(actionParams, "message", { required: true }); + return await invoke( + { + action: "editMessage", + channelId: resolveChannelId(), + messageId, + content, + accountId, + }, + cfg, + ); + } + + if (action === "delete") { + const messageId = readStringParam(actionParams, "messageId", { + required: true, + }); + return await invoke( + { + action: "deleteMessage", + channelId: resolveChannelId(), + messageId, + accountId, + }, + cfg, + ); + } + + if (action === "pin" || action === "unpin" || action === "list-pins") { + const messageId = + action === "list-pins" + ? undefined + : readStringParam(actionParams, "messageId", { required: true }); + return await invoke( + { + action: action === "pin" ? "pinMessage" : action === "unpin" ? "unpinMessage" : "listPins", + channelId: resolveChannelId(), + messageId, + accountId, + }, + cfg, + ); + } + + if (action === "member-info") { + const userId = readStringParam(actionParams, "userId", { required: true }); + return await invoke({ action: "memberInfo", userId, accountId }, cfg); + } + + if (action === "emoji-list") { + const limit = readNumberParam(actionParams, "limit", { integer: true }); + return await invoke({ action: "emojiList", limit, accountId }, cfg); + } + + throw new Error(`Action ${action} is not supported for provider ${providerId}.`); +} diff --git a/src/plugin-sdk/tool-send.ts b/src/plugin-sdk/tool-send.ts new file mode 100644 index 00000000000..b34b0509064 --- /dev/null +++ b/src/plugin-sdk/tool-send.ts @@ -0,0 +1,15 @@ +export function extractToolSend( + args: Record, + expectedAction = "sendMessage", +): { to: string; accountId?: string } | null { + const action = typeof args.action === "string" ? args.action.trim() : ""; + if (action !== expectedAction) { + return null; + } + const to = typeof args.to === "string" ? args.to : undefined; + if (!to) { + return null; + } + const accountId = typeof args.accountId === "string" ? args.accountId.trim() : undefined; + return { to, accountId }; +} diff --git a/src/plugin-sdk/webhook-targets.ts b/src/plugin-sdk/webhook-targets.ts new file mode 100644 index 00000000000..81747c89412 --- /dev/null +++ b/src/plugin-sdk/webhook-targets.ts @@ -0,0 +1,49 @@ +import type { IncomingMessage, ServerResponse } from "node:http"; +import { normalizeWebhookPath } from "./webhook-path.js"; + +export type RegisteredWebhookTarget = { + target: T; + unregister: () => void; +}; + +export function registerWebhookTarget( + targetsByPath: Map, + target: T, +): RegisteredWebhookTarget { + const key = normalizeWebhookPath(target.path); + const normalizedTarget = { ...target, path: key }; + const existing = targetsByPath.get(key) ?? []; + targetsByPath.set(key, [...existing, normalizedTarget]); + const unregister = () => { + const updated = (targetsByPath.get(key) ?? []).filter((entry) => entry !== normalizedTarget); + if (updated.length > 0) { + targetsByPath.set(key, updated); + return; + } + targetsByPath.delete(key); + }; + return { target: normalizedTarget, unregister }; +} + +export function resolveWebhookTargets( + req: IncomingMessage, + targetsByPath: Map, +): { path: string; targets: T[] } | null { + const url = new URL(req.url ?? "/", "http://localhost"); + const path = normalizeWebhookPath(url.pathname); + const targets = targetsByPath.get(path); + if (!targets || targets.length === 0) { + return null; + } + return { path, targets }; +} + +export function rejectNonPostWebhookRequest(req: IncomingMessage, res: ServerResponse): boolean { + if (req.method === "POST") { + return false; + } + res.statusCode = 405; + res.setHeader("Allow", "POST"); + res.end("Method Not Allowed"); + return true; +} diff --git a/src/routing/resolve-route.test.ts b/src/routing/resolve-route.test.ts index 5cf2f3447d5..9c36656deab 100644 --- a/src/routing/resolve-route.test.ts +++ b/src/routing/resolve-route.test.ts @@ -386,24 +386,49 @@ test("dmScope=per-account-channel-peer uses default accountId when not provided" }); describe("parentPeer binding inheritance (thread support)", () => { + const threadPeer = { kind: "channel" as const, id: "thread-456" }; + const defaultParentPeer = { kind: "channel" as const, id: "parent-channel-123" }; + + function makeDiscordPeerBinding(agentId: string, peerId: string) { + return { + agentId, + match: { + channel: "discord" as const, + peer: { kind: "channel" as const, id: peerId }, + }, + }; + } + + function makeDiscordGuildBinding(agentId: string, guildId: string) { + return { + agentId, + match: { + channel: "discord" as const, + guildId, + }, + }; + } + + function resolveDiscordThreadRoute(params: { + cfg: OpenClawConfig; + parentPeer?: { kind: "channel"; id: string } | null; + guildId?: string; + }) { + const parentPeer = "parentPeer" in params ? params.parentPeer : defaultParentPeer; + return resolveAgentRoute({ + cfg: params.cfg, + channel: "discord", + peer: threadPeer, + parentPeer, + guildId: params.guildId, + }); + } + test("thread inherits binding from parent channel when no direct match", () => { const cfg: OpenClawConfig = { - bindings: [ - { - agentId: "adecco", - match: { - channel: "discord", - peer: { kind: "channel", id: "parent-channel-123" }, - }, - }, - ], + bindings: [makeDiscordPeerBinding("adecco", defaultParentPeer.id)], }; - const route = resolveAgentRoute({ - cfg, - channel: "discord", - peer: { kind: "channel", id: "thread-456" }, - parentPeer: { kind: "channel", id: "parent-channel-123" }, - }); + const route = resolveDiscordThreadRoute({ cfg }); expect(route.agentId).toBe("adecco"); expect(route.matchedBy).toBe("binding.peer.parent"); }); @@ -411,28 +436,11 @@ describe("parentPeer binding inheritance (thread support)", () => { test("direct peer binding wins over parent peer binding", () => { const cfg: OpenClawConfig = { bindings: [ - { - agentId: "thread-agent", - match: { - channel: "discord", - peer: { kind: "channel", id: "thread-456" }, - }, - }, - { - agentId: "parent-agent", - match: { - channel: "discord", - peer: { kind: "channel", id: "parent-channel-123" }, - }, - }, + makeDiscordPeerBinding("thread-agent", threadPeer.id), + makeDiscordPeerBinding("parent-agent", defaultParentPeer.id), ], }; - const route = resolveAgentRoute({ - cfg, - channel: "discord", - peer: { kind: "channel", id: "thread-456" }, - parentPeer: { kind: "channel", id: "parent-channel-123" }, - }); + const route = resolveDiscordThreadRoute({ cfg }); expect(route.agentId).toBe("thread-agent"); expect(route.matchedBy).toBe("binding.peer"); }); @@ -440,29 +448,11 @@ describe("parentPeer binding inheritance (thread support)", () => { test("parent peer binding wins over guild binding", () => { const cfg: OpenClawConfig = { bindings: [ - { - agentId: "parent-agent", - match: { - channel: "discord", - peer: { kind: "channel", id: "parent-channel-123" }, - }, - }, - { - agentId: "guild-agent", - match: { - channel: "discord", - guildId: "guild-789", - }, - }, + makeDiscordPeerBinding("parent-agent", defaultParentPeer.id), + makeDiscordGuildBinding("guild-agent", "guild-789"), ], }; - const route = resolveAgentRoute({ - cfg, - channel: "discord", - peer: { kind: "channel", id: "thread-456" }, - parentPeer: { kind: "channel", id: "parent-channel-123" }, - guildId: "guild-789", - }); + const route = resolveDiscordThreadRoute({ cfg, guildId: "guild-789" }); expect(route.agentId).toBe("parent-agent"); expect(route.matchedBy).toBe("binding.peer.parent"); }); @@ -470,73 +460,29 @@ describe("parentPeer binding inheritance (thread support)", () => { test("falls back to guild binding when no parent peer match", () => { const cfg: OpenClawConfig = { bindings: [ - { - agentId: "other-parent-agent", - match: { - channel: "discord", - peer: { kind: "channel", id: "other-parent-999" }, - }, - }, - { - agentId: "guild-agent", - match: { - channel: "discord", - guildId: "guild-789", - }, - }, + makeDiscordPeerBinding("other-parent-agent", "other-parent-999"), + makeDiscordGuildBinding("guild-agent", "guild-789"), ], }; - const route = resolveAgentRoute({ - cfg, - channel: "discord", - peer: { kind: "channel", id: "thread-456" }, - parentPeer: { kind: "channel", id: "parent-channel-123" }, - guildId: "guild-789", - }); + const route = resolveDiscordThreadRoute({ cfg, guildId: "guild-789" }); expect(route.agentId).toBe("guild-agent"); expect(route.matchedBy).toBe("binding.guild"); }); test("parentPeer with empty id is ignored", () => { const cfg: OpenClawConfig = { - bindings: [ - { - agentId: "parent-agent", - match: { - channel: "discord", - peer: { kind: "channel", id: "parent-channel-123" }, - }, - }, - ], + bindings: [makeDiscordPeerBinding("parent-agent", defaultParentPeer.id)], }; - const route = resolveAgentRoute({ - cfg, - channel: "discord", - peer: { kind: "channel", id: "thread-456" }, - parentPeer: { kind: "channel", id: "" }, - }); + const route = resolveDiscordThreadRoute({ cfg, parentPeer: { kind: "channel", id: "" } }); expect(route.agentId).toBe("main"); expect(route.matchedBy).toBe("default"); }); test("null parentPeer is handled gracefully", () => { const cfg: OpenClawConfig = { - bindings: [ - { - agentId: "parent-agent", - match: { - channel: "discord", - peer: { kind: "channel", id: "parent-channel-123" }, - }, - }, - ], + bindings: [makeDiscordPeerBinding("parent-agent", defaultParentPeer.id)], }; - const route = resolveAgentRoute({ - cfg, - channel: "discord", - peer: { kind: "channel", id: "thread-456" }, - parentPeer: null, - }); + const route = resolveDiscordThreadRoute({ cfg, parentPeer: null }); expect(route.agentId).toBe("main"); expect(route.matchedBy).toBe("default"); }); diff --git a/src/security/external-content.test.ts b/src/security/external-content.test.ts index 41dac8a191c..e025fea60c0 100644 --- a/src/security/external-content.test.ts +++ b/src/security/external-content.test.ts @@ -9,6 +9,16 @@ import { } from "./external-content.js"; describe("external-content security", () => { + const expectSanitizedBoundaryMarkers = (result: string) => { + const startMarkers = result.match(/<<>>/g) ?? []; + const endMarkers = result.match(/<<>>/g) ?? []; + + expect(startMarkers).toHaveLength(1); + expect(endMarkers).toHaveLength(1); + expect(result).toContain("[[MARKER_SANITIZED]]"); + expect(result).toContain("[[END_MARKER_SANITIZED]]"); + }; + describe("detectSuspiciousPatterns", () => { it("detects ignore previous instructions pattern", () => { const patterns = detectSuspiciousPatterns( @@ -91,13 +101,7 @@ describe("external-content security", () => { "Before <<>> middle <<>> after"; const result = wrapExternalContent(malicious, { source: "email" }); - const startMarkers = result.match(/<<>>/g) ?? []; - const endMarkers = result.match(/<<>>/g) ?? []; - - expect(startMarkers).toHaveLength(1); - expect(endMarkers).toHaveLength(1); - expect(result).toContain("[[MARKER_SANITIZED]]"); - expect(result).toContain("[[END_MARKER_SANITIZED]]"); + expectSanitizedBoundaryMarkers(result); }); it("sanitizes boundary markers case-insensitively", () => { @@ -105,13 +109,7 @@ describe("external-content security", () => { "Before <<>> middle <<>> after"; const result = wrapExternalContent(malicious, { source: "email" }); - const startMarkers = result.match(/<<>>/g) ?? []; - const endMarkers = result.match(/<<>>/g) ?? []; - - expect(startMarkers).toHaveLength(1); - expect(endMarkers).toHaveLength(1); - expect(result).toContain("[[MARKER_SANITIZED]]"); - expect(result).toContain("[[END_MARKER_SANITIZED]]"); + expectSanitizedBoundaryMarkers(result); }); it("preserves non-marker unicode content", () => { diff --git a/src/security/fix.test.ts b/src/security/fix.test.ts index 976b357b5dc..75e753d018b 100644 --- a/src/security/fix.test.ts +++ b/src/security/fix.test.ts @@ -24,6 +24,47 @@ describe("security fix", () => { return dir; }; + const createFixEnv = (stateDir: string, configPath: string) => ({ + ...process.env, + OPENCLAW_STATE_DIR: stateDir, + OPENCLAW_CONFIG_PATH: configPath, + }); + + const writeJsonConfig = async (configPath: string, config: Record) => { + await fs.writeFile(configPath, `${JSON.stringify(config, null, 2)}\n`, "utf-8"); + }; + + const writeWhatsAppConfig = async (configPath: string, whatsapp: Record) => { + await writeJsonConfig(configPath, { + channels: { + whatsapp, + }, + }); + }; + + const readParsedConfig = async (configPath: string) => + JSON.parse(await fs.readFile(configPath, "utf-8")) as Record; + + const runFixAndReadChannels = async (stateDir: string, configPath: string) => { + const env = createFixEnv(stateDir, configPath); + const res = await fixSecurityFootguns({ env, stateDir, configPath }); + const parsed = await readParsedConfig(configPath); + return { + res, + channels: parsed.channels as Record>, + }; + }; + + const writeWhatsAppAllowFromStore = async (stateDir: string, allowFrom: string[]) => { + const credsDir = path.join(stateDir, "credentials"); + await fs.mkdir(credsDir, { recursive: true }); + await fs.writeFile( + path.join(credsDir, "whatsapp-allowFrom.json"), + `${JSON.stringify({ version: 1, allowFrom }, null, 2)}\n`, + "utf-8", + ); + }; + beforeAll(async () => { fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-security-fix-suite-")); }); @@ -39,39 +80,20 @@ describe("security fix", () => { await fs.chmod(stateDir, 0o755); const configPath = path.join(stateDir, "openclaw.json"); - await fs.writeFile( - configPath, - `${JSON.stringify( - { - channels: { - telegram: { groupPolicy: "open" }, - whatsapp: { groupPolicy: "open" }, - discord: { groupPolicy: "open" }, - signal: { groupPolicy: "open" }, - imessage: { groupPolicy: "open" }, - }, - logging: { redactSensitive: "off" }, - }, - null, - 2, - )}\n`, - "utf-8", - ); + await writeJsonConfig(configPath, { + channels: { + telegram: { groupPolicy: "open" }, + whatsapp: { groupPolicy: "open" }, + discord: { groupPolicy: "open" }, + signal: { groupPolicy: "open" }, + imessage: { groupPolicy: "open" }, + }, + logging: { redactSensitive: "off" }, + }); await fs.chmod(configPath, 0o644); - const credsDir = path.join(stateDir, "credentials"); - await fs.mkdir(credsDir, { recursive: true }); - await fs.writeFile( - path.join(credsDir, "whatsapp-allowFrom.json"), - `${JSON.stringify({ version: 1, allowFrom: [" +15551234567 "] }, null, 2)}\n`, - "utf-8", - ); - - const env = { - ...process.env, - OPENCLAW_STATE_DIR: stateDir, - OPENCLAW_CONFIG_PATH: configPath, - }; + await writeWhatsAppAllowFromStore(stateDir, [" +15551234567 "]); + const env = createFixEnv(stateDir, configPath); const res = await fixSecurityFootguns({ env, stateDir, configPath }); expect(res.ok).toBe(true); @@ -93,7 +115,7 @@ describe("security fix", () => { const configMode = (await fs.stat(configPath)).mode & 0o777; expectPerms(configMode, 0o600); - const parsed = JSON.parse(await fs.readFile(configPath, "utf-8")) as Record; + const parsed = await readParsedConfig(configPath); const channels = parsed.channels as Record>; expect(channels.telegram.groupPolicy).toBe("allowlist"); expect(channels.whatsapp.groupPolicy).toBe("allowlist"); @@ -108,43 +130,16 @@ describe("security fix", () => { const stateDir = await createStateDir("per-account"); const configPath = path.join(stateDir, "openclaw.json"); - await fs.writeFile( - configPath, - `${JSON.stringify( - { - channels: { - whatsapp: { - accounts: { - a1: { groupPolicy: "open" }, - }, - }, - }, - }, - null, - 2, - )}\n`, - "utf-8", - ); + await writeWhatsAppConfig(configPath, { + accounts: { + a1: { groupPolicy: "open" }, + }, + }); - const credsDir = path.join(stateDir, "credentials"); - await fs.mkdir(credsDir, { recursive: true }); - await fs.writeFile( - path.join(credsDir, "whatsapp-allowFrom.json"), - `${JSON.stringify({ version: 1, allowFrom: ["+15550001111"] }, null, 2)}\n`, - "utf-8", - ); - - const env = { - ...process.env, - OPENCLAW_STATE_DIR: stateDir, - OPENCLAW_CONFIG_PATH: configPath, - }; - - const res = await fixSecurityFootguns({ env, stateDir, configPath }); + await writeWhatsAppAllowFromStore(stateDir, ["+15550001111"]); + const { res, channels } = await runFixAndReadChannels(stateDir, configPath); expect(res.ok).toBe(true); - const parsed = JSON.parse(await fs.readFile(configPath, "utf-8")) as Record; - const channels = parsed.channels as Record>; const whatsapp = channels.whatsapp; const accounts = whatsapp.accounts as Record>; @@ -156,39 +151,15 @@ describe("security fix", () => { const stateDir = await createStateDir("no-seed"); const configPath = path.join(stateDir, "openclaw.json"); - await fs.writeFile( - configPath, - `${JSON.stringify( - { - channels: { - whatsapp: { groupPolicy: "open", allowFrom: ["+15552223333"] }, - }, - }, - null, - 2, - )}\n`, - "utf-8", - ); + await writeWhatsAppConfig(configPath, { + groupPolicy: "open", + allowFrom: ["+15552223333"], + }); - const credsDir = path.join(stateDir, "credentials"); - await fs.mkdir(credsDir, { recursive: true }); - await fs.writeFile( - path.join(credsDir, "whatsapp-allowFrom.json"), - `${JSON.stringify({ version: 1, allowFrom: ["+15550001111"] }, null, 2)}\n`, - "utf-8", - ); - - const env = { - ...process.env, - OPENCLAW_STATE_DIR: stateDir, - OPENCLAW_CONFIG_PATH: configPath, - }; - - const res = await fixSecurityFootguns({ env, stateDir, configPath }); + await writeWhatsAppAllowFromStore(stateDir, ["+15550001111"]); + const { res, channels } = await runFixAndReadChannels(stateDir, configPath); expect(res.ok).toBe(true); - const parsed = JSON.parse(await fs.readFile(configPath, "utf-8")) as Record; - const channels = parsed.channels as Record>; expect(channels.whatsapp.groupPolicy).toBe("allowlist"); expect(channels.whatsapp.groupAllowFrom).toBeUndefined(); }); @@ -201,11 +172,7 @@ describe("security fix", () => { await fs.writeFile(configPath, "{ this is not json }\n", "utf-8"); await fs.chmod(configPath, 0o644); - const env = { - ...process.env, - OPENCLAW_STATE_DIR: stateDir, - OPENCLAW_CONFIG_PATH: configPath, - }; + const env = createFixEnv(stateDir, configPath); const res = await fixSecurityFootguns({ env, stateDir, configPath }); expect(res.ok).toBe(false); diff --git a/src/shared/frontmatter.ts b/src/shared/frontmatter.ts index 2db4fb95117..91e49017be6 100644 --- a/src/shared/frontmatter.ts +++ b/src/shared/frontmatter.ts @@ -97,3 +97,43 @@ export function resolveOpenClawManifestInstall( export function resolveOpenClawManifestOs(metadataObj: Record): string[] { return normalizeStringList(metadataObj.os); } + +export type ParsedOpenClawManifestInstallBase = { + raw: Record; + kind: string; + id?: string; + label?: string; + bins?: string[]; +}; + +export function parseOpenClawManifestInstallBase( + input: unknown, + allowedKinds: readonly string[], +): ParsedOpenClawManifestInstallBase | undefined { + if (!input || typeof input !== "object") { + return undefined; + } + const raw = input as Record; + const kindRaw = + typeof raw.kind === "string" ? raw.kind : typeof raw.type === "string" ? raw.type : ""; + const kind = kindRaw.trim().toLowerCase(); + if (!allowedKinds.includes(kind)) { + return undefined; + } + + const spec: ParsedOpenClawManifestInstallBase = { + raw, + kind, + }; + if (typeof raw.id === "string") { + spec.id = raw.id; + } + if (typeof raw.label === "string") { + spec.label = raw.label; + } + const bins = normalizeStringList(raw.bins); + if (bins.length > 0) { + spec.bins = bins; + } + return spec; +} diff --git a/src/test-utils/chunk-test-helpers.ts b/src/test-utils/chunk-test-helpers.ts new file mode 100644 index 00000000000..09b4d5c8a54 --- /dev/null +++ b/src/test-utils/chunk-test-helpers.ts @@ -0,0 +1,22 @@ +export function countLines(text: string): number { + return text.split("\n").length; +} + +export function hasBalancedFences(chunk: string): boolean { + let open: { markerChar: string; markerLen: number } | null = null; + for (const line of chunk.split("\n")) { + const match = line.match(/^( {0,3})(`{3,}|~{3,})(.*)$/); + if (!match) { + continue; + } + const marker = match[2]; + if (!open) { + open = { markerChar: marker[0], markerLen: marker.length }; + continue; + } + if (open.markerChar === marker[0] && marker.length >= open.markerLen) { + open = null; + } + } + return open === null; +} diff --git a/src/test-utils/exec-assertions.ts b/src/test-utils/exec-assertions.ts new file mode 100644 index 00000000000..7bcf17a66cd --- /dev/null +++ b/src/test-utils/exec-assertions.ts @@ -0,0 +1,16 @@ +import { expect } from "vitest"; + +export function expectSingleNpmInstallIgnoreScriptsCall(params: { + calls: Array<[unknown, { cwd?: string } | undefined]>; + expectedCwd: string; +}) { + const npmCalls = params.calls.filter((call) => Array.isArray(call[0]) && call[0][0] === "npm"); + expect(npmCalls.length).toBe(1); + const first = npmCalls[0]; + if (!first) { + throw new Error("expected npm install call"); + } + const [argv, opts] = first; + expect(argv).toEqual(["npm", "install", "--omit=dev", "--silent", "--ignore-scripts"]); + expect(opts?.cwd).toBe(params.expectedCwd); +} diff --git a/src/test-utils/mock-http-response.ts b/src/test-utils/mock-http-response.ts new file mode 100644 index 00000000000..6334ad30f37 --- /dev/null +++ b/src/test-utils/mock-http-response.ts @@ -0,0 +1,25 @@ +import type { ServerResponse } from "node:http"; + +export function createMockServerResponse(): ServerResponse & { body?: string } { + const headers: Record = {}; + const res: { + headersSent: boolean; + statusCode: number; + body?: string; + setHeader: (key: string, value: string) => unknown; + end: (body?: string) => unknown; + } = { + headersSent: false, + statusCode: 200, + setHeader: (key: string, value: string) => { + headers[key.toLowerCase()] = value; + return res; + }, + end: (body?: string) => { + res.headersSent = true; + res.body = body; + return res; + }, + }; + return res as unknown as ServerResponse & { body?: string }; +} diff --git a/src/test-utils/ports.ts b/src/test-utils/ports.ts index 00fa86aa00a..b2b3958e9ec 100644 --- a/src/test-utils/ports.ts +++ b/src/test-utils/ports.ts @@ -92,3 +92,18 @@ export async function getDeterministicFreePortBlock(params?: { throw new Error("failed to acquire a free port block"); } + +export async function getFreePortBlockWithPermissionFallback(params: { + offsets: number[]; + fallbackBase: number; +}): Promise { + try { + return await getDeterministicFreePortBlock({ offsets: params.offsets }); + } catch (err) { + const code = (err as NodeJS.ErrnoException | undefined)?.code; + if (code === "EPERM" || code === "EACCES") { + return params.fallbackBase + (process.pid % 10_000); + } + throw err; + } +}