diff --git a/src/agents/bash-tools.exec-approval-request.test.ts b/src/agents/bash-tools.exec-approval-request.test.ts index 349663aba..20e08cf1b 100644 --- a/src/agents/bash-tools.exec-approval-request.test.ts +++ b/src/agents/bash-tools.exec-approval-request.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { DEFAULT_APPROVAL_REQUEST_TIMEOUT_MS, DEFAULT_APPROVAL_TIMEOUT_MS, @@ -8,15 +8,20 @@ vi.mock("./tools/gateway.js", () => ({ callGatewayTool: vi.fn(), })); +let callGatewayTool: typeof import("./tools/gateway.js").callGatewayTool; +let requestExecApprovalDecision: typeof import("./bash-tools.exec-approval-request.js").requestExecApprovalDecision; + describe("requestExecApprovalDecision", () => { - beforeEach(async () => { - const { callGatewayTool } = await import("./tools/gateway.js"); + beforeAll(async () => { + ({ callGatewayTool } = await import("./tools/gateway.js")); + ({ requestExecApprovalDecision } = await import("./bash-tools.exec-approval-request.js")); + }); + + beforeEach(() => { vi.mocked(callGatewayTool).mockReset(); }); it("returns string decisions", async () => { - const { requestExecApprovalDecision } = await import("./bash-tools.exec-approval-request.js"); - const { callGatewayTool } = await import("./tools/gateway.js"); vi.mocked(callGatewayTool).mockResolvedValue({ decision: "allow-once" }); const result = await requestExecApprovalDecision({ @@ -51,9 +56,6 @@ describe("requestExecApprovalDecision", () => { }); it("returns null for missing or non-string decisions", async () => { - const { requestExecApprovalDecision } = await import("./bash-tools.exec-approval-request.js"); - const { callGatewayTool } = await import("./tools/gateway.js"); - vi.mocked(callGatewayTool).mockResolvedValueOnce({}); await expect( requestExecApprovalDecision({ diff --git a/src/agents/bash-tools.exec.approval-id.e2e.test.ts b/src/agents/bash-tools.exec.approval-id.e2e.test.ts index 3d90797b2..8a07a7a82 100644 --- a/src/agents/bash-tools.exec.approval-id.e2e.test.ts +++ b/src/agents/bash-tools.exec.approval-id.e2e.test.ts @@ -1,7 +1,7 @@ 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 { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; vi.mock("./tools/gateway.js", () => ({ callGatewayTool: vi.fn(), @@ -15,10 +15,18 @@ vi.mock("./tools/nodes-utils.js", () => ({ resolveNodeIdFromList: vi.fn((nodes: Array<{ nodeId: string }>) => nodes[0]?.nodeId), })); +let callGatewayTool: typeof import("./tools/gateway.js").callGatewayTool; +let createExecTool: typeof import("./bash-tools.exec.js").createExecTool; + describe("exec approvals", () => { let previousHome: string | undefined; let previousUserProfile: string | undefined; + beforeAll(async () => { + ({ callGatewayTool } = await import("./tools/gateway.js")); + ({ createExecTool } = await import("./bash-tools.exec.js")); + }); + beforeEach(async () => { previousHome = process.env.HOME; previousUserProfile = process.env.USERPROFILE; @@ -43,7 +51,6 @@ describe("exec approvals", () => { }); it("reuses approval id as the node runId", async () => { - const { callGatewayTool } = await import("./tools/gateway.js"); let invokeParams: unknown; vi.mocked(callGatewayTool).mockImplementation(async (method, _opts, params) => { @@ -58,7 +65,6 @@ describe("exec approvals", () => { return { ok: true }; }); - const { createExecTool } = await import("./bash-tools.exec.js"); const tool = createExecTool({ host: "node", ask: "always", @@ -78,7 +84,6 @@ describe("exec approvals", () => { }); it("skips approval when node allowlist is satisfied", async () => { - const { callGatewayTool } = await import("./tools/gateway.js"); const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-test-bin-")); const binDir = path.join(tempDir, "bin"); await fs.mkdir(binDir, { recursive: true }); @@ -111,7 +116,6 @@ describe("exec approvals", () => { return { ok: true }; }); - const { createExecTool } = await import("./bash-tools.exec.js"); const tool = createExecTool({ host: "node", ask: "on-miss", @@ -128,14 +132,12 @@ describe("exec approvals", () => { }); it("honors ask=off for elevated gateway exec without prompting", async () => { - const { callGatewayTool } = await import("./tools/gateway.js"); const calls: string[] = []; vi.mocked(callGatewayTool).mockImplementation(async (method) => { calls.push(method); return { ok: true }; }); - const { createExecTool } = await import("./bash-tools.exec.js"); const tool = createExecTool({ ask: "off", security: "full", @@ -149,7 +151,6 @@ describe("exec approvals", () => { }); it("requires approval for elevated ask when allowlist misses", async () => { - const { callGatewayTool } = await import("./tools/gateway.js"); const calls: string[] = []; let resolveApproval: (() => void) | undefined; const approvalSeen = new Promise((resolve) => { @@ -169,7 +170,6 @@ describe("exec approvals", () => { return { ok: true }; }); - const { createExecTool } = await import("./bash-tools.exec.js"); const tool = createExecTool({ ask: "on-miss", security: "allowlist", diff --git a/src/agents/openclaw-tools.sessions.e2e.test.ts b/src/agents/openclaw-tools.sessions.e2e.test.ts index d2e93702c..7d4d813a3 100644 --- a/src/agents/openclaw-tools.sessions.e2e.test.ts +++ b/src/agents/openclaw-tools.sessions.e2e.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from "vitest"; +import { beforeAll, describe, expect, it, vi } from "vitest"; import { addSubagentRunForTests, listSubagentRunsForRequester, @@ -41,7 +41,13 @@ const waitForCalls = async (getCount: () => number, count: number, timeoutMs = 2 ); }; +let sessionsModule: typeof import("../config/sessions.js"); + describe("sessions tools", () => { + beforeAll(async () => { + sessionsModule = await import("../config/sessions.js"); + }); + it("uses number (not integer) in tool schemas for Gemini compatibility", () => { const tools = createOpenClawTools(); const byName = (name: string) => { @@ -767,7 +773,6 @@ describe("sessions tools", () => { startedAt: now - 2 * 60_000, }); - const sessionsModule = await import("../config/sessions.js"); const loadSessionStoreSpy = vi .spyOn(sessionsModule, "loadSessionStore") .mockImplementation(() => ({ @@ -827,7 +832,6 @@ describe("sessions tools", () => { startedAt: Date.now() - 60_000, }); - const sessionsModule = await import("../config/sessions.js"); const loadSessionStoreSpy = vi .spyOn(sessionsModule, "loadSessionStore") .mockImplementation(() => ({ diff --git a/src/agents/pi-embedded-helpers.buildbootstrapcontextfiles.e2e.test.ts b/src/agents/pi-embedded-helpers.buildbootstrapcontextfiles.e2e.test.ts index 46a56e6ae..b9a290871 100644 --- a/src/agents/pi-embedded-helpers.buildbootstrapcontextfiles.e2e.test.ts +++ b/src/agents/pi-embedded-helpers.buildbootstrapcontextfiles.e2e.test.ts @@ -118,38 +118,47 @@ describe("buildBootstrapContextFiles", () => { }); }); -describe("resolveBootstrapMaxChars", () => { - it("returns default when unset", () => { - expect(resolveBootstrapMaxChars()).toBe(DEFAULT_BOOTSTRAP_MAX_CHARS); - }); - it("uses configured value when valid", () => { - const cfg = { - agents: { defaults: { bootstrapMaxChars: 12345 } }, - } as OpenClawConfig; - expect(resolveBootstrapMaxChars(cfg)).toBe(12345); - }); - it("falls back when invalid", () => { - const cfg = { - agents: { defaults: { bootstrapMaxChars: -1 } }, - } as OpenClawConfig; - expect(resolveBootstrapMaxChars(cfg)).toBe(DEFAULT_BOOTSTRAP_MAX_CHARS); - }); -}); +type BootstrapLimitResolverCase = { + name: "bootstrapMaxChars" | "bootstrapTotalMaxChars"; + resolve: (cfg?: OpenClawConfig) => number; + defaultValue: number; +}; -describe("resolveBootstrapTotalMaxChars", () => { - it("returns default when unset", () => { - expect(resolveBootstrapTotalMaxChars()).toBe(DEFAULT_BOOTSTRAP_TOTAL_MAX_CHARS); +const BOOTSTRAP_LIMIT_RESOLVERS: BootstrapLimitResolverCase[] = [ + { + name: "bootstrapMaxChars", + resolve: resolveBootstrapMaxChars, + defaultValue: DEFAULT_BOOTSTRAP_MAX_CHARS, + }, + { + name: "bootstrapTotalMaxChars", + resolve: resolveBootstrapTotalMaxChars, + defaultValue: DEFAULT_BOOTSTRAP_TOTAL_MAX_CHARS, + }, +]; + +describe("bootstrap limit resolvers", () => { + it("return defaults when unset", () => { + for (const resolver of BOOTSTRAP_LIMIT_RESOLVERS) { + expect(resolver.resolve()).toBe(resolver.defaultValue); + } }); - it("uses configured value when valid", () => { - const cfg = { - agents: { defaults: { bootstrapTotalMaxChars: 12345 } }, - } as OpenClawConfig; - expect(resolveBootstrapTotalMaxChars(cfg)).toBe(12345); + + it("use configured values when valid", () => { + for (const resolver of BOOTSTRAP_LIMIT_RESOLVERS) { + const cfg = { + agents: { defaults: { [resolver.name]: 12345 } }, + } as OpenClawConfig; + expect(resolver.resolve(cfg)).toBe(12345); + } }); - it("falls back when invalid", () => { - const cfg = { - agents: { defaults: { bootstrapTotalMaxChars: -1 } }, - } as OpenClawConfig; - expect(resolveBootstrapTotalMaxChars(cfg)).toBe(DEFAULT_BOOTSTRAP_TOTAL_MAX_CHARS); + + it("fall back when values are invalid", () => { + for (const resolver of BOOTSTRAP_LIMIT_RESOLVERS) { + const cfg = { + agents: { defaults: { [resolver.name]: -1 } }, + } as OpenClawConfig; + expect(resolver.resolve(cfg)).toBe(resolver.defaultValue); + } }); }); diff --git a/src/agents/pi-embedded-helpers.isbillingerrormessage.e2e.test.ts b/src/agents/pi-embedded-helpers.isbillingerrormessage.e2e.test.ts index c62aac873..62dd44531 100644 --- a/src/agents/pi-embedded-helpers.isbillingerrormessage.e2e.test.ts +++ b/src/agents/pi-embedded-helpers.isbillingerrormessage.e2e.test.ts @@ -35,10 +35,6 @@ describe("isAuthErrorMessage", () => { expect(isAuthErrorMessage(sample)).toBe(true); } }); - it("ignores unrelated errors", () => { - expect(isAuthErrorMessage("rate limit exceeded")).toBe(false); - expect(isAuthErrorMessage("billing issue detected")).toBe(false); - }); }); describe("isBillingErrorMessage", () => { @@ -54,11 +50,6 @@ describe("isBillingErrorMessage", () => { expect(isBillingErrorMessage(sample)).toBe(true); } }); - it("ignores unrelated errors", () => { - expect(isBillingErrorMessage("rate limit exceeded")).toBe(false); - expect(isBillingErrorMessage("invalid api key")).toBe(false); - expect(isBillingErrorMessage("context length exceeded")).toBe(false); - }); it("does not false-positive on issue IDs or text containing 402", () => { const falsePositives = [ "Fixed issue CHE-402 in the latest release", @@ -110,14 +101,6 @@ describe("isCloudCodeAssistFormatError", () => { expect(isCloudCodeAssistFormatError(sample)).toBe(true); } }); - it("ignores unrelated errors", () => { - expect(isCloudCodeAssistFormatError("rate limit exceeded")).toBe(false); - expect( - isCloudCodeAssistFormatError( - '400 {"type":"error","error":{"type":"invalid_request_error","message":"messages.84.content.1.image.source.base64.data: At least one of the image dimensions exceed max allowed size for many-image requests: 2000 pixels"}}', - ), - ).toBe(false); - }); }); describe("isCloudflareOrHtmlErrorPage", () => { @@ -195,13 +178,6 @@ describe("isContextOverflowError", () => { } }); - it("ignores unrelated errors", () => { - expect(isContextOverflowError("rate limit exceeded")).toBe(false); - expect(isContextOverflowError("request size exceeds upload limit")).toBe(false); - expect(isContextOverflowError("model not found")).toBe(false); - expect(isContextOverflowError("authentication failed")).toBe(false); - }); - it("ignores normal conversation text mentioning context overflow", () => { // These are legitimate conversation snippets, not error messages expect(isContextOverflowError("Let's investigate the context overflow bug")).toBe(false); @@ -211,6 +187,46 @@ describe("isContextOverflowError", () => { }); }); +describe("error classifiers", () => { + it("ignore unrelated errors", () => { + const checks: Array<{ + matcher: (message: string) => boolean; + samples: string[]; + }> = [ + { + matcher: isAuthErrorMessage, + samples: ["rate limit exceeded", "billing issue detected"], + }, + { + matcher: isBillingErrorMessage, + samples: ["rate limit exceeded", "invalid api key", "context length exceeded"], + }, + { + matcher: isCloudCodeAssistFormatError, + samples: [ + "rate limit exceeded", + '400 {"type":"error","error":{"type":"invalid_request_error","message":"messages.84.content.1.image.source.base64.data: At least one of the image dimensions exceed max allowed size for many-image requests: 2000 pixels"}}', + ], + }, + { + matcher: isContextOverflowError, + samples: [ + "rate limit exceeded", + "request size exceeds upload limit", + "model not found", + "authentication failed", + ], + }, + ]; + + for (const check of checks) { + for (const sample of check.samples) { + expect(check.matcher(sample)).toBe(false); + } + } + }); +}); + describe("isLikelyContextOverflowError", () => { it("matches context overflow hints", () => { const samples = [ diff --git a/src/agents/pi-embedded-helpers.sanitizeuserfacingtext.e2e.test.ts b/src/agents/pi-embedded-helpers.sanitizeuserfacingtext.e2e.test.ts index 8c0af5cc2..f29e2ebd6 100644 --- a/src/agents/pi-embedded-helpers.sanitizeuserfacingtext.e2e.test.ts +++ b/src/agents/pi-embedded-helpers.sanitizeuserfacingtext.e2e.test.ts @@ -14,10 +14,12 @@ describe("sanitizeUserFacingText", () => { expect(sanitizeUserFacingText("Hi there!")).toBe("Hi there!"); }); - it("does not clobber normal numeric prefixes", () => { - expect(sanitizeUserFacingText("202 results found")).toBe("202 results found"); - expect(sanitizeUserFacingText("400 days left")).toBe("400 days left"); - }); + it.each(["202 results found", "400 days left"])( + "does not clobber normal numeric prefix: %s", + (text) => { + expect(sanitizeUserFacingText(text)).toBe(text); + }, + ); it("sanitizes role ordering errors", () => { const result = sanitizeUserFacingText("400 Incorrect role information", { errorContext: true }); @@ -30,45 +32,27 @@ describe("sanitizeUserFacingText", () => { ); }); - it("sanitizes direct context-overflow errors", () => { - expect( - sanitizeUserFacingText( - "Context overflow: prompt too large for the model. Try /reset (or /new) to start a fresh session, or use a larger-context model.", - { errorContext: true }, - ), - ).toContain("Context overflow: prompt too large for the model."); - expect( - sanitizeUserFacingText("Request size exceeds model context window", { errorContext: true }), - ).toContain("Context overflow: prompt too large for the model."); + it.each([ + "Context overflow: prompt too large for the model. Try /reset (or /new) to start a fresh session, or use a larger-context model.", + "Request size exceeds model context window", + ])("sanitizes direct context-overflow error: %s", (text) => { + expect(sanitizeUserFacingText(text, { errorContext: true })).toContain( + "Context overflow: prompt too large for the model.", + ); }); - it("does not swallow assistant text that quotes the canonical context-overflow string", () => { - const text = - "Changelog note: we fixed false positives for `Context overflow: prompt too large for the model. Try /reset (or /new) to start a fresh session, or use a larger-context model.` in 2026.2.9"; + it.each([ + "Changelog note: we fixed false positives for `Context overflow: prompt too large for the model. Try /reset (or /new) to start a fresh session, or use a larger-context model.` in 2026.2.9", + "nah it failed, hit a context overflow. the prompt was too large for the model. want me to retry it with a different approach?", + "Problem: When a subagent reads a very large file, it can exceed the model context window. Auto-compaction cannot help in that case.", + ])("does not rewrite regular context-overflow mentions: %s", (text) => { expect(sanitizeUserFacingText(text)).toBe(text); }); - it("does not rewrite conversational mentions of context overflow", () => { - const text = - "nah it failed, hit a context overflow. the prompt was too large for the model. want me to retry it with a different approach?"; - expect(sanitizeUserFacingText(text)).toBe(text); - }); - - it("does not rewrite technical summaries that mention context overflow", () => { - const text = - "Problem: When a subagent reads a very large file, it can exceed the model context window. Auto-compaction cannot help in that case."; - expect(sanitizeUserFacingText(text)).toBe(text); - }); - - it("does not rewrite conversational billing/help text without errorContext", () => { - const text = - "If your API billing is low, top up credits in your provider dashboard and retry payment verification."; - expect(sanitizeUserFacingText(text)).toBe(text); - }); - - it("does not rewrite normal text that mentions billing and plan", () => { - const text = - "Firebase downgraded us to the free Spark plan; check whether we need to re-enable billing."; + it.each([ + "If your API billing is low, top up credits in your provider dashboard and retry payment verification.", + "Firebase downgraded us to the free Spark plan; check whether we need to re-enable billing.", + ])("does not rewrite regular billing mentions: %s", (text) => { expect(sanitizeUserFacingText(text)).toBe(text); }); @@ -95,25 +79,27 @@ describe("sanitizeUserFacingText", () => { ); }); - it("collapses consecutive duplicate paragraphs", () => { - const text = "Hello there!\n\nHello there!"; - expect(sanitizeUserFacingText(text)).toBe("Hello there!"); + it.each([ + { + input: "Hello there!\n\nHello there!", + expected: "Hello there!", + }, + { + input: "Hello there!\n\nDifferent line.", + expected: "Hello there!\n\nDifferent line.", + }, + ])("normalizes paragraph blocks", ({ input, expected }) => { + expect(sanitizeUserFacingText(input)).toBe(expected); }); - it("does not collapse distinct paragraphs", () => { - const text = "Hello there!\n\nDifferent line."; - expect(sanitizeUserFacingText(text)).toBe(text); - }); - - it("strips leading newlines from LLM output", () => { - expect(sanitizeUserFacingText("\n\nHello there!")).toBe("Hello there!"); - expect(sanitizeUserFacingText("\nHello there!")).toBe("Hello there!"); - expect(sanitizeUserFacingText("\n\n\nMultiple newlines")).toBe("Multiple newlines"); - }); - - it("strips leading whitespace and newlines combined", () => { - expect(sanitizeUserFacingText("\n \nHello")).toBe("Hello"); - expect(sanitizeUserFacingText(" \n\nHello")).toBe("Hello"); + it.each([ + { input: "\n\nHello there!", expected: "Hello there!" }, + { input: "\nHello there!", expected: "Hello there!" }, + { input: "\n\n\nMultiple newlines", expected: "Multiple newlines" }, + { input: "\n \nHello", expected: "Hello" }, + { input: " \n\nHello", expected: "Hello" }, + ])("strips leading empty lines: %j", ({ input, expected }) => { + expect(sanitizeUserFacingText(input)).toBe(expected); }); it("preserves trailing whitespace and internal newlines", () => { @@ -121,9 +107,8 @@ describe("sanitizeUserFacingText", () => { expect(sanitizeUserFacingText("Line 1\nLine 2")).toBe("Line 1\nLine 2"); }); - it("returns empty for whitespace-only input", () => { - expect(sanitizeUserFacingText("\n\n")).toBe(""); - expect(sanitizeUserFacingText(" \n ")).toBe(""); + it.each(["\n\n", " \n "])("returns empty for whitespace-only input: %j", (input) => { + expect(sanitizeUserFacingText(input)).toBe(""); }); }); @@ -334,81 +319,60 @@ describe("downgradeOpenAIReasoningBlocks", () => { }); describe("normalizeTextForComparison", () => { - it("lowercases text", () => { - expect(normalizeTextForComparison("Hello World")).toBe("hello world"); - }); - - it("trims whitespace", () => { - expect(normalizeTextForComparison(" hello ")).toBe("hello"); - }); - - it("collapses multiple spaces", () => { - expect(normalizeTextForComparison("hello world")).toBe("hello world"); - }); - - it("strips emoji", () => { - expect(normalizeTextForComparison("Hello 👋 World 🌍")).toBe("hello world"); - }); - - it("handles mixed normalization", () => { - expect(normalizeTextForComparison(" Hello 👋 WORLD 🌍 ")).toBe("hello world"); + it.each([ + { input: "Hello World", expected: "hello world" }, + { input: " hello ", expected: "hello" }, + { input: "hello world", expected: "hello world" }, + { input: "Hello 👋 World 🌍", expected: "hello world" }, + { input: " Hello 👋 WORLD 🌍 ", expected: "hello world" }, + ])("normalizes comparison text", ({ input, expected }) => { + expect(normalizeTextForComparison(input)).toBe(expected); }); }); describe("isMessagingToolDuplicate", () => { - it("returns false for empty sentTexts", () => { - expect(isMessagingToolDuplicate("hello world", [])).toBe(false); - }); - - it("returns false for short texts", () => { - expect(isMessagingToolDuplicate("short", ["short"])).toBe(false); - }); - - it("detects exact duplicates", () => { - expect( - isMessagingToolDuplicate("Hello, this is a test message!", [ - "Hello, this is a test message!", - ]), - ).toBe(true); - }); - - it("detects duplicates with different casing", () => { - expect( - isMessagingToolDuplicate("HELLO, THIS IS A TEST MESSAGE!", [ - "hello, this is a test message!", - ]), - ).toBe(true); - }); - - it("detects duplicates with emoji variations", () => { - expect( - isMessagingToolDuplicate("Hello! 👋 This is a test message!", [ - "Hello! This is a test message!", - ]), - ).toBe(true); - }); - - it("detects substring duplicates (LLM elaboration)", () => { - expect( - isMessagingToolDuplicate('I sent the message: "Hello, this is a test message!"', [ - "Hello, this is a test message!", - ]), - ).toBe(true); - }); - - it("detects when sent text contains block reply (reverse substring)", () => { - expect( - isMessagingToolDuplicate("Hello, this is a test message!", [ - 'I sent the message: "Hello, this is a test message!"', - ]), - ).toBe(true); - }); - - it("returns false for non-matching texts", () => { - expect( - isMessagingToolDuplicate("This is completely different content.", [ - "Hello, this is a test message!", - ]), - ).toBe(false); + it.each([ + { + input: "hello world", + sentTexts: [], + expected: false, + }, + { + input: "short", + sentTexts: ["short"], + expected: false, + }, + { + input: "Hello, this is a test message!", + sentTexts: ["Hello, this is a test message!"], + expected: true, + }, + { + input: "HELLO, THIS IS A TEST MESSAGE!", + sentTexts: ["hello, this is a test message!"], + expected: true, + }, + { + input: "Hello! 👋 This is a test message!", + sentTexts: ["Hello! This is a test message!"], + expected: true, + }, + { + input: 'I sent the message: "Hello, this is a test message!"', + sentTexts: ["Hello, this is a test message!"], + expected: true, + }, + { + input: "Hello, this is a test message!", + sentTexts: ['I sent the message: "Hello, this is a test message!"'], + expected: true, + }, + { + input: "This is completely different content.", + sentTexts: ["Hello, this is a test message!"], + expected: false, + }, + ])("returns $expected for duplicate check", ({ input, sentTexts, expected }) => { + expect(isMessagingToolDuplicate(input, sentTexts)).toBe(expected); }); }); diff --git a/src/agents/pi-embedded-runner-extraparams.e2e.test.ts b/src/agents/pi-embedded-runner-extraparams.e2e.test.ts index 966b00fca..69f2077b0 100644 --- a/src/agents/pi-embedded-runner-extraparams.e2e.test.ts +++ b/src/agents/pi-embedded-runner-extraparams.e2e.test.ts @@ -278,40 +278,49 @@ describe("applyExtraParamsToAgent", () => { expect(payload.store).toBe(false); }); - it("does not force store=true for Codex responses (Codex requires store=false)", () => { - const payload = runStoreMutationCase({ - applyProvider: "openai-codex", - applyModelId: "codex-mini-latest", - model: { - api: "openai-codex-responses", - provider: "openai-codex", - id: "codex-mini-latest", - baseUrl: "https://chatgpt.com/backend-api/codex/responses", - } as Model<"openai-codex-responses">, - }); - expect(payload.store).toBe(false); - }); + it.each([ + { + name: "with openai-codex provider config", + run: () => + runStoreMutationCase({ + applyProvider: "openai-codex", + applyModelId: "codex-mini-latest", + model: { + api: "openai-codex-responses", + provider: "openai-codex", + id: "codex-mini-latest", + baseUrl: "https://chatgpt.com/backend-api/codex/responses", + } as Model<"openai-codex-responses">, + }), + }, + { + name: "without config via provider/model hints", + run: () => { + const payload = { store: false }; + const baseStreamFn: StreamFn = (_model, _context, options) => { + options?.onPayload?.(payload); + return {} as ReturnType; + }; + const agent = { streamFn: baseStreamFn }; - it("does not force store=true for Codex responses (Codex requires store=false)", () => { - const payload = { store: false }; - const baseStreamFn: StreamFn = (_model, _context, options) => { - options?.onPayload?.(payload); - return {} as ReturnType; - }; - const agent = { streamFn: baseStreamFn }; + applyExtraParamsToAgent(agent, undefined, "openai-codex", "codex-mini-latest"); - applyExtraParamsToAgent(agent, undefined, "openai-codex", "codex-mini-latest"); + const model = { + api: "openai-codex-responses", + provider: "openai-codex", + id: "codex-mini-latest", + baseUrl: "https://chatgpt.com/backend-api/codex/responses", + } as Model<"openai-codex-responses">; + const context: Context = { messages: [] }; - const model = { - api: "openai-codex-responses", - provider: "openai-codex", - id: "codex-mini-latest", - baseUrl: "https://chatgpt.com/backend-api/codex/responses", - } as Model<"openai-codex-responses">; - const context: Context = { messages: [] }; - - void agent.streamFn?.(model, context, {}); - - expect(payload.store).toBe(false); - }); + void agent.streamFn?.(model, context, {}); + return payload; + }, + }, + ])( + "does not force store=true for Codex responses (Codex requires store=false) ($name)", + ({ run }) => { + expect(run().store).toBe(false); + }, + ); }); diff --git a/src/agents/pi-embedded-utils.e2e.test.ts b/src/agents/pi-embedded-utils.e2e.test.ts index ecb8dace5..5e8a9f39b 100644 --- a/src/agents/pi-embedded-utils.e2e.test.ts +++ b/src/agents/pi-embedded-utils.e2e.test.ts @@ -28,23 +28,25 @@ function makeAssistantMessage( } describe("extractAssistantText", () => { - it("strips Minimax tool invocation XML from text", () => { - const msg = makeAssistantMessage({ - role: "assistant", - content: [ - { - type: "text", - text: ` + it("strips tool-only Minimax invocation XML from text", () => { + const cases = [ + ` netstat -tlnp | grep 18789 `, - }, - ], - timestamp: Date.now(), - }); - - const result = extractAssistantText(msg); - expect(result).toBe(""); + ` +test + +`, + ]; + for (const text of cases) { + const msg = makeAssistantMessage({ + role: "assistant", + content: [{ type: "text", text }], + timestamp: Date.now(), + }); + expect(extractAssistantText(msg)).toBe(""); + } }); it("strips multiple tool invocations", () => { @@ -268,25 +270,6 @@ describe("extractAssistantText", () => { expect(result).toBe("Some text here.More text."); }); - it("returns empty string when message is only tool invocations", () => { - const msg = makeAssistantMessage({ - role: "assistant", - content: [ - { - type: "text", - text: ` -test - -`, - }, - ], - timestamp: Date.now(), - }); - - const result = extractAssistantText(msg); - expect(result).toBe(""); - }); - it("handles multiple text blocks", () => { const msg = makeAssistantMessage({ role: "assistant", @@ -436,140 +419,62 @@ File contents here`, expect(result).toBe("Here's what I found:\nDone checking."); }); - it("strips thinking tags from text content", () => { - const msg = makeAssistantMessage({ - role: "assistant", - content: [ - { - type: "text", - text: "El usuario quiere retomar una tarea...Aquí está tu respuesta.", - }, - ], - timestamp: Date.now(), - }); + it("strips reasoning/thinking tag variants", () => { + const cases = [ + { + name: "think tag", + text: "El usuario quiere retomar una tarea...Aquí está tu respuesta.", + expected: "Aquí está tu respuesta.", + }, + { + name: "think tag with attributes", + text: `HiddenVisible`, + expected: "Visible", + }, + { + name: "unclosed think tag", + text: "Pensando sobre el problema...", + expected: "", + }, + { + name: "thinking tag", + text: "Beforeinternal reasoningAfter", + expected: "BeforeAfter", + }, + { + name: "antthinking tag", + text: "Some reasoningThe actual answer.", + expected: "The actual answer.", + }, + { + name: "final wrapper", + text: "\nAnswer\n", + expected: "Answer", + }, + { + name: "thought tag", + text: "Internal deliberationFinal response.", + expected: "Final response.", + }, + { + name: "multiple think blocks", + text: "Startfirst thoughtMiddlesecond thoughtEnd", + expected: "StartMiddleEnd", + }, + ] as const; - const result = extractAssistantText(msg); - expect(result).toBe("Aquí está tu respuesta."); - }); - - it("strips thinking tags with attributes", () => { - const msg = makeAssistantMessage({ - role: "assistant", - content: [ - { - type: "text", - text: `HiddenVisible`, - }, - ], - timestamp: Date.now(), - }); - - const result = extractAssistantText(msg); - expect(result).toBe("Visible"); - }); - - it("strips thinking tags without closing tag", () => { - const msg = makeAssistantMessage({ - role: "assistant", - content: [ - { - type: "text", - text: "Pensando sobre el problema...", - }, - ], - timestamp: Date.now(), - }); - - const result = extractAssistantText(msg); - expect(result).toBe(""); - }); - - it("strips thinking tags with various formats", () => { - const msg = makeAssistantMessage({ - role: "assistant", - content: [ - { - type: "text", - text: "Beforeinternal reasoningAfter", - }, - ], - timestamp: Date.now(), - }); - - const result = extractAssistantText(msg); - expect(result).toBe("BeforeAfter"); - }); - - it("strips antthinking tags", () => { - const msg = makeAssistantMessage({ - role: "assistant", - content: [ - { - type: "text", - text: "Some reasoningThe actual answer.", - }, - ], - timestamp: Date.now(), - }); - - const result = extractAssistantText(msg); - expect(result).toBe("The actual answer."); - }); - - it("strips final tags while keeping content", () => { - const msg = makeAssistantMessage({ - role: "assistant", - content: [ - { - type: "text", - text: "\nAnswer\n", - }, - ], - timestamp: Date.now(), - }); - - const result = extractAssistantText(msg); - expect(result).toBe("Answer"); - }); - - it("strips thought tags", () => { - const msg = makeAssistantMessage({ - role: "assistant", - content: [ - { - type: "text", - text: "Internal deliberationFinal response.", - }, - ], - timestamp: Date.now(), - }); - - const result = extractAssistantText(msg); - expect(result).toBe("Final response."); - }); - - it("handles nested or multiple thinking blocks", () => { - const msg = makeAssistantMessage({ - role: "assistant", - content: [ - { - type: "text", - text: "Startfirst thoughtMiddlesecond thoughtEnd", - }, - ], - timestamp: Date.now(), - }); - - const result = extractAssistantText(msg); - expect(result).toBe("StartMiddleEnd"); + for (const testCase of cases) { + const msg = makeAssistantMessage({ + role: "assistant", + content: [{ type: "text", text: testCase.text }], + timestamp: Date.now(), + }); + expect(extractAssistantText(msg), testCase.name).toBe(testCase.expected); + } }); }); describe("formatReasoningMessage", () => { - it("returns empty string for empty input", () => { - expect(formatReasoningMessage("")).toBe(""); - }); - it("returns empty string for whitespace-only input", () => { expect(formatReasoningMessage(" \n \t ")).toBe(""); }); @@ -604,37 +509,51 @@ describe("formatReasoningMessage", () => { }); describe("stripDowngradedToolCallText", () => { - it("strips [Historical context: ...] blocks", () => { - const text = `[Historical context: a different model called tool "exec" with arguments {"command":"git status"}]`; - expect(stripDowngradedToolCallText(text)).toBe(""); - }); + it("strips downgraded marker blocks while preserving surrounding user-facing text", () => { + const cases = [ + { + name: "historical context only", + text: `[Historical context: a different model called tool "exec" with arguments {"command":"git status"}]`, + expected: "", + }, + { + name: "text before historical context", + text: `Here is the answer.\n[Historical context: a different model called tool "read"]`, + expected: "Here is the answer.", + }, + { + name: "text around historical context", + text: `Before.\n[Historical context: tool call info]\nAfter.`, + expected: "Before.\nAfter.", + }, + { + name: "multiple historical context blocks", + text: `[Historical context: first tool call]\n[Historical context: second tool call]`, + expected: "", + }, + { + name: "mixed tool call and historical context", + text: `Intro.\n[Tool Call: exec (ID: toolu_1)]\nArguments: { "command": "ls" }\n[Historical context: a different model called tool "read"]`, + expected: "Intro.", + }, + { + name: "no markers", + text: "Just a normal response with no markers.", + expected: "Just a normal response with no markers.", + }, + ] as const; - it("preserves text before [Historical context: ...] blocks", () => { - const text = `Here is the answer.\n[Historical context: a different model called tool "read"]`; - expect(stripDowngradedToolCallText(text)).toBe("Here is the answer."); - }); - - it("preserves text around [Historical context: ...] blocks", () => { - const text = `Before.\n[Historical context: tool call info]\nAfter.`; - expect(stripDowngradedToolCallText(text)).toBe("Before.\nAfter."); - }); - - it("strips multiple [Historical context: ...] blocks", () => { - const text = `[Historical context: first tool call]\n[Historical context: second tool call]`; - expect(stripDowngradedToolCallText(text)).toBe(""); - }); - - it("strips mixed [Tool Call: ...] and [Historical context: ...] blocks", () => { - const text = `Intro.\n[Tool Call: exec (ID: toolu_1)]\nArguments: { "command": "ls" }\n[Historical context: a different model called tool "read"]`; - expect(stripDowngradedToolCallText(text)).toBe("Intro."); - }); - - it("returns text unchanged when no markers are present", () => { - const text = "Just a normal response with no markers."; - expect(stripDowngradedToolCallText(text)).toBe("Just a normal response with no markers."); - }); - - it("returns empty string for empty input", () => { - expect(stripDowngradedToolCallText("")).toBe(""); + for (const testCase of cases) { + expect(stripDowngradedToolCallText(testCase.text), testCase.name).toBe(testCase.expected); + } + }); +}); + +describe("empty input handling", () => { + it("returns empty string", () => { + const helpers = [formatReasoningMessage, stripDowngradedToolCallText]; + for (const helper of helpers) { + expect(helper("")).toBe(""); + } }); }); diff --git a/src/agents/sandbox-merge.e2e.test.ts b/src/agents/sandbox-merge.e2e.test.ts index 8f3c7807e..592439a90 100644 --- a/src/agents/sandbox-merge.e2e.test.ts +++ b/src/agents/sandbox-merge.e2e.test.ts @@ -1,9 +1,21 @@ -import { describe, expect, it } from "vitest"; +import { beforeAll, describe, expect, it } from "vitest"; + +let resolveSandboxScope: typeof import("./sandbox.js").resolveSandboxScope; +let resolveSandboxDockerConfig: typeof import("./sandbox.js").resolveSandboxDockerConfig; +let resolveSandboxBrowserConfig: typeof import("./sandbox.js").resolveSandboxBrowserConfig; +let resolveSandboxPruneConfig: typeof import("./sandbox.js").resolveSandboxPruneConfig; describe("sandbox config merges", () => { - it("resolves sandbox scope deterministically", { timeout: 60_000 }, async () => { - const { resolveSandboxScope } = await import("./sandbox.js"); + beforeAll(async () => { + ({ + resolveSandboxScope, + resolveSandboxDockerConfig, + resolveSandboxBrowserConfig, + resolveSandboxPruneConfig, + } = await import("./sandbox.js")); + }); + it("resolves sandbox scope deterministically", { timeout: 60_000 }, async () => { expect(resolveSandboxScope({})).toBe("agent"); expect(resolveSandboxScope({ perSession: true })).toBe("session"); expect(resolveSandboxScope({ perSession: false })).toBe("shared"); @@ -11,8 +23,6 @@ describe("sandbox config merges", () => { }); it("merges sandbox docker env and ulimits (agent wins)", async () => { - const { resolveSandboxDockerConfig } = await import("./sandbox.js"); - const resolved = resolveSandboxDockerConfig({ scope: "agent", globalDocker: { @@ -33,8 +43,6 @@ describe("sandbox config merges", () => { }); it("merges sandbox docker binds (global + agent combined)", async () => { - const { resolveSandboxDockerConfig } = await import("./sandbox.js"); - const resolved = resolveSandboxDockerConfig({ scope: "agent", globalDocker: { @@ -52,8 +60,6 @@ describe("sandbox config merges", () => { }); it("returns undefined binds when neither global nor agent has binds", async () => { - const { resolveSandboxDockerConfig } = await import("./sandbox.js"); - const resolved = resolveSandboxDockerConfig({ scope: "agent", globalDocker: {}, @@ -64,8 +70,6 @@ describe("sandbox config merges", () => { }); it("ignores agent binds under shared scope", async () => { - const { resolveSandboxDockerConfig } = await import("./sandbox.js"); - const resolved = resolveSandboxDockerConfig({ scope: "shared", globalDocker: { @@ -80,8 +84,6 @@ describe("sandbox config merges", () => { }); it("ignores agent docker overrides under shared scope", async () => { - const { resolveSandboxDockerConfig } = await import("./sandbox.js"); - const resolved = resolveSandboxDockerConfig({ scope: "shared", globalDocker: { image: "global" }, @@ -92,8 +94,6 @@ describe("sandbox config merges", () => { }); it("applies per-agent browser and prune overrides (ignored under shared scope)", async () => { - const { resolveSandboxBrowserConfig, resolveSandboxPruneConfig } = await import("./sandbox.js"); - const browser = resolveSandboxBrowserConfig({ scope: "agent", globalBrowser: { enabled: false, headless: false, enableNoVnc: true }, diff --git a/src/agents/sandbox/validate-sandbox-security.test.ts b/src/agents/sandbox/validate-sandbox-security.test.ts index 4b3ff9d69..247b48b15 100644 --- a/src/agents/sandbox/validate-sandbox-security.test.ts +++ b/src/agents/sandbox/validate-sandbox-security.test.ts @@ -115,15 +115,6 @@ describe("validateSeccompProfile", () => { expect(() => validateSeccompProfile("/tmp/seccomp.json")).not.toThrow(); expect(() => validateSeccompProfile(undefined)).not.toThrow(); }); - - it("blocks unconfined (case-insensitive)", () => { - expect(() => validateSeccompProfile("unconfined")).toThrow( - /seccomp profile "unconfined" is blocked/, - ); - expect(() => validateSeccompProfile("Unconfined")).toThrow( - /seccomp profile "Unconfined" is blocked/, - ); - }); }); describe("validateApparmorProfile", () => { @@ -131,11 +122,23 @@ describe("validateApparmorProfile", () => { expect(() => validateApparmorProfile("openclaw-sandbox")).not.toThrow(); expect(() => validateApparmorProfile(undefined)).not.toThrow(); }); +}); - it("blocks unconfined (case-insensitive)", () => { - expect(() => validateApparmorProfile("unconfined")).toThrow( - /apparmor profile "unconfined" is blocked/, - ); +describe("profile hardening", () => { + it.each([ + { + name: "seccomp", + run: (value: string) => validateSeccompProfile(value), + expected: /seccomp profile ".+" is blocked/, + }, + { + name: "apparmor", + run: (value: string) => validateApparmorProfile(value), + expected: /apparmor profile ".+" is blocked/, + }, + ])("blocks unconfined profiles (case-insensitive): $name", ({ run, expected }) => { + expect(() => run("unconfined")).toThrow(expected); + expect(() => run("Unconfined")).toThrow(expected); }); }); diff --git a/src/agents/shell-utils.e2e.test.ts b/src/agents/shell-utils.e2e.test.ts index 9f4cb869b..25be7c757 100644 --- a/src/agents/shell-utils.e2e.test.ts +++ b/src/agents/shell-utils.e2e.test.ts @@ -90,19 +90,15 @@ describe("resolveShellFromPath", () => { } }); - if (isWin) { - it("returns undefined on Windows for missing PATH entries in this test harness", () => { - process.env.PATH = ""; - expect(resolveShellFromPath("bash")).toBeUndefined(); - }); - return; - } - it("returns undefined when PATH is empty", () => { process.env.PATH = ""; expect(resolveShellFromPath("bash")).toBeUndefined(); }); + if (isWin) { + return; + } + it("returns the first executable match from PATH", () => { const notExecutable = createTempCommandDir(tempDirs, [{ name: "bash", executable: false }]); const executable = createTempCommandDir(tempDirs, [{ name: "bash", executable: true }]); diff --git a/src/agents/subagent-announce.format.e2e.test.ts b/src/agents/subagent-announce.format.e2e.test.ts index 2b775be85..9aff7c564 100644 --- a/src/agents/subagent-announce.format.e2e.test.ts +++ b/src/agents/subagent-announce.format.e2e.test.ts @@ -1261,7 +1261,7 @@ describe("subagent announce formatting", () => { threadId: 99, }, }, - ] as const)("$testName", async (testCase) => { + ] as const)("thread routing: $testName", async (testCase) => { const { runSubagentAnnounceFlow } = await import("./subagent-announce.js"); embeddedRunMock.isEmbeddedPiRunActive.mockReturnValue(true); embeddedRunMock.isEmbeddedPiRunStreaming.mockReturnValue(false); @@ -1348,7 +1348,7 @@ describe("subagent announce formatting", () => { expectedChannel: "whatsapp", expectedAccountId: "acct-987", }, - ] as const)("$testName", async (testCase) => { + ] as const)("direct announce: $testName", async (testCase) => { const { runSubagentAnnounceFlow } = await import("./subagent-announce.js"); embeddedRunMock.isEmbeddedPiRunActive.mockReturnValue(false); embeddedRunMock.isEmbeddedPiRunStreaming.mockReturnValue(false); diff --git a/src/agents/system-prompt.e2e.test.ts b/src/agents/system-prompt.e2e.test.ts index cb9958fcb..4a8fd3403 100644 --- a/src/agents/system-prompt.e2e.test.ts +++ b/src/agents/system-prompt.e2e.test.ts @@ -535,7 +535,7 @@ describe("buildAgentSystemPrompt", () => { }); describe("buildSubagentSystemPrompt", () => { - it("includes sub-agent spawning guidance for depth-1 orchestrator when maxSpawnDepth >= 2", () => { + it("renders depth-1 orchestrator guidance, labels, and recovery notes", () => { const prompt = buildSubagentSystemPrompt({ childSessionKey: "agent:main:subagent:abc", task: "research task", @@ -549,21 +549,15 @@ describe("buildSubagentSystemPrompt", () => { expect(prompt).toContain("`subagents` tool"); expect(prompt).toContain("announce their results back to you automatically"); expect(prompt).toContain("Do NOT repeatedly poll `subagents list`"); + expect(prompt).toContain("spawned by the main agent"); + expect(prompt).toContain("reported to the main agent"); + expect(prompt).toContain("[compacted: tool output removed to free context]"); + expect(prompt).toContain("[truncated: output exceeded context limit]"); + expect(prompt).toContain("offset/limit"); + expect(prompt).toContain("instead of full-file `cat`"); }); - it("does not include spawning guidance for depth-1 leaf when maxSpawnDepth == 1", () => { - const prompt = buildSubagentSystemPrompt({ - childSessionKey: "agent:main:subagent:abc", - task: "research task", - childDepth: 1, - maxSpawnDepth: 1, - }); - - expect(prompt).not.toContain("## Sub-Agent Spawning"); - expect(prompt).not.toContain("You CAN spawn"); - }); - - it("includes leaf worker note for depth-2 sub-sub-agents", () => { + it("renders depth-2 leaf guidance with parent orchestrator labels", () => { const prompt = buildSubagentSystemPrompt({ childSessionKey: "agent:main:subagent:abc:subagent:def", task: "leaf task", @@ -574,54 +568,39 @@ describe("buildSubagentSystemPrompt", () => { expect(prompt).toContain("## Sub-Agent Spawning"); expect(prompt).toContain("leaf worker"); expect(prompt).toContain("CANNOT spawn further sub-agents"); - }); - - it("uses 'parent orchestrator' label for depth-2 agents", () => { - const prompt = buildSubagentSystemPrompt({ - childSessionKey: "agent:main:subagent:abc:subagent:def", - task: "leaf task", - childDepth: 2, - maxSpawnDepth: 2, - }); - expect(prompt).toContain("spawned by the parent orchestrator"); expect(prompt).toContain("reported to the parent orchestrator"); }); - it("uses 'main agent' label for depth-1 agents", () => { - const prompt = buildSubagentSystemPrompt({ - childSessionKey: "agent:main:subagent:abc", - task: "orchestrator task", - childDepth: 1, - maxSpawnDepth: 2, - }); + it("omits spawning guidance for depth-1 leaf agents", () => { + const leafCases = [ + { + name: "explicit maxSpawnDepth 1", + input: { + childSessionKey: "agent:main:subagent:abc", + task: "research task", + childDepth: 1, + maxSpawnDepth: 1, + }, + expectMainAgentLabel: false, + }, + { + name: "implicit default depth/maxSpawnDepth", + input: { + childSessionKey: "agent:main:subagent:abc", + task: "basic task", + }, + expectMainAgentLabel: true, + }, + ] as const; - expect(prompt).toContain("spawned by the main agent"); - expect(prompt).toContain("reported to the main agent"); - }); - - it("includes recovery guidance for compacted/truncated tool output", () => { - const prompt = buildSubagentSystemPrompt({ - childSessionKey: "agent:main:subagent:abc", - task: "investigate logs", - childDepth: 1, - maxSpawnDepth: 2, - }); - - expect(prompt).toContain("[compacted: tool output removed to free context]"); - expect(prompt).toContain("[truncated: output exceeded context limit]"); - expect(prompt).toContain("offset/limit"); - expect(prompt).toContain("instead of full-file `cat`"); - }); - - it("defaults to depth 1 and maxSpawnDepth 1 when not provided", () => { - const prompt = buildSubagentSystemPrompt({ - childSessionKey: "agent:main:subagent:abc", - task: "basic task", - }); - - // Should not include spawning guidance (default maxSpawnDepth is 1, depth 1 is leaf) - expect(prompt).not.toContain("## Sub-Agent Spawning"); - expect(prompt).toContain("spawned by the main agent"); + for (const testCase of leafCases) { + const prompt = buildSubagentSystemPrompt(testCase.input); + expect(prompt, testCase.name).not.toContain("## Sub-Agent Spawning"); + expect(prompt, testCase.name).not.toContain("You CAN spawn"); + if (testCase.expectMainAgentLabel) { + expect(prompt, testCase.name).toContain("spawned by the main agent"); + } + } }); }); diff --git a/src/agents/tools/common.e2e.test.ts b/src/agents/tools/common.e2e.test.ts index 67c6b23c0..ba6044ea7 100644 --- a/src/agents/tools/common.e2e.test.ts +++ b/src/agents/tools/common.e2e.test.ts @@ -35,12 +35,6 @@ describe("readStringOrNumberParam", () => { const params = { chatId: " abc " }; expect(readStringOrNumberParam(params, "chatId")).toBe("abc"); }); - - it("throws when required and missing", () => { - expect(() => readStringOrNumberParam({}, "chatId", { required: true })).toThrow( - /chatId required/, - ); - }); }); describe("readNumberParam", () => { @@ -53,8 +47,13 @@ describe("readNumberParam", () => { const params = { messageId: "42.9" }; expect(readNumberParam(params, "messageId", { integer: true })).toBe(42); }); +}); - it("throws when required and missing", () => { +describe("required parameter validation", () => { + it("throws when required values are missing", () => { + expect(() => readStringOrNumberParam({}, "chatId", { required: true })).toThrow( + /chatId required/, + ); expect(() => readNumberParam({}, "messageId", { required: true })).toThrow( /messageId required/, ); diff --git a/src/agents/tools/sessions.e2e.test.ts b/src/agents/tools/sessions.e2e.test.ts index 4e3d6a556..ea857a0f4 100644 --- a/src/agents/tools/sessions.e2e.test.ts +++ b/src/agents/tools/sessions.e2e.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { createTestRegistry } from "../../test-utils/channel-plugins.js"; import { extractAssistantText, sanitizeTextContent } from "./sessions-helpers.js"; @@ -22,10 +22,10 @@ vi.mock("../../config/config.js", async (importOriginal) => { import { createSessionsListTool } from "./sessions-list-tool.js"; import { createSessionsSendTool } from "./sessions-send-tool.js"; -const loadResolveAnnounceTarget = async () => await import("./sessions-announce-target.js"); +let resolveAnnounceTarget: (typeof import("./sessions-announce-target.js"))["resolveAnnounceTarget"]; +let setActivePluginRegistry: (typeof import("../../plugins/runtime.js"))["setActivePluginRegistry"]; const installRegistry = async () => { - const { setActivePluginRegistry } = await import("../../plugins/runtime.js"); setActivePluginRegistry( createTestRegistry([ { @@ -89,6 +89,11 @@ describe("sanitizeTextContent", () => { }); }); +beforeAll(async () => { + ({ resolveAnnounceTarget } = await import("./sessions-announce-target.js")); + ({ setActivePluginRegistry } = await import("../../plugins/runtime.js")); +}); + describe("extractAssistantText", () => { it("sanitizes blocks without injecting newlines", () => { const message = { @@ -134,7 +139,6 @@ describe("resolveAnnounceTarget", () => { }); it("derives non-WhatsApp announce targets from the session key", async () => { - const { resolveAnnounceTarget } = await loadResolveAnnounceTarget(); const target = await resolveAnnounceTarget({ sessionKey: "agent:main:discord:group:dev", displayKey: "agent:main:discord:group:dev", @@ -144,7 +148,6 @@ describe("resolveAnnounceTarget", () => { }); it("hydrates WhatsApp accountId from sessions.list when available", async () => { - const { resolveAnnounceTarget } = await loadResolveAnnounceTarget(); callGatewayMock.mockResolvedValueOnce({ sessions: [ { diff --git a/src/cli/cli-utils.test.ts b/src/cli/cli-utils.test.ts index 5e8bfee99..5f4db66fd 100644 --- a/src/cli/cli-utils.test.ts +++ b/src/cli/cli-utils.test.ts @@ -61,15 +61,17 @@ describe("dns cli", () => { }); describe("parseByteSize", () => { - it("parses bytes with units", () => { - expect(parseByteSize("10kb")).toBe(10 * 1024); - expect(parseByteSize("1mb")).toBe(1024 * 1024); - expect(parseByteSize("2gb")).toBe(2 * 1024 * 1024 * 1024); - }); - - it("parses shorthand units", () => { - expect(parseByteSize("5k")).toBe(5 * 1024); - expect(parseByteSize("1m")).toBe(1024 * 1024); + it("parses byte-size units and shorthand values", () => { + const cases = [ + ["parses 10kb", "10kb", 10 * 1024], + ["parses 1mb", "1mb", 1024 * 1024], + ["parses 2gb", "2gb", 2 * 1024 * 1024 * 1024], + ["parses shorthand 5k", "5k", 5 * 1024], + ["parses shorthand 1m", "1m", 1024 * 1024], + ] as const; + for (const [name, input, expected] of cases) { + expect(parseByteSize(input), name).toBe(expected); + } }); it("uses default unit when omitted", () => { @@ -84,27 +86,17 @@ describe("parseByteSize", () => { }); describe("parseDurationMs", () => { - it("parses bare ms", () => { - expect(parseDurationMs("10000")).toBe(10_000); - }); - - it("parses seconds suffix", () => { - expect(parseDurationMs("10s")).toBe(10_000); - }); - - it("parses minutes suffix", () => { - expect(parseDurationMs("1m")).toBe(60_000); - }); - - it("parses hours suffix", () => { - expect(parseDurationMs("2h")).toBe(7_200_000); - }); - - it("parses days suffix", () => { - expect(parseDurationMs("2d")).toBe(172_800_000); - }); - - it("supports decimals", () => { - expect(parseDurationMs("0.5s")).toBe(500); + it("parses duration strings", () => { + const cases = [ + ["parses bare ms", "10000", 10_000], + ["parses seconds suffix", "10s", 10_000], + ["parses minutes suffix", "1m", 60_000], + ["parses hours suffix", "2h", 7_200_000], + ["parses days suffix", "2d", 172_800_000], + ["supports decimals", "0.5s", 500], + ] as const; + for (const [name, input, expected] of cases) { + expect(parseDurationMs(input), name).toBe(expected); + } }); }); diff --git a/src/cli/config-cli.test.ts b/src/cli/config-cli.test.ts index ec1b6523b..f35cbd196 100644 --- a/src/cli/config-cli.test.ts +++ b/src/cli/config-cli.test.ts @@ -1,5 +1,5 @@ import { Command } from "commander"; -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import type { ConfigFileSnapshot, OpenClawConfig } from "../config/types.js"; /** @@ -53,8 +53,9 @@ function setSnapshot(resolved: OpenClawConfig, config: OpenClawConfig) { mockReadConfigFileSnapshot.mockResolvedValueOnce(buildSnapshot({ resolved, config })); } +let registerConfigCli: typeof import("./config-cli.js").registerConfigCli; + async function runConfigCommand(args: string[]) { - const { registerConfigCli } = await import("./config-cli.js"); const program = new Command(); program.exitOverride(); registerConfigCli(program); @@ -62,6 +63,10 @@ async function runConfigCommand(args: string[]) { } describe("config cli", () => { + beforeAll(async () => { + ({ registerConfigCli } = await import("./config-cli.js")); + }); + beforeEach(() => { vi.clearAllMocks(); }); @@ -166,7 +171,6 @@ describe("config cli", () => { }); it("shows --strict-json and keeps --json as a legacy alias in help", async () => { - const { registerConfigCli } = await import("./config-cli.js"); const program = new Command(); registerConfigCli(program); diff --git a/src/cli/program.smoke.e2e.test.ts b/src/cli/program.smoke.e2e.test.ts index cca4e06a9..ea65a1c7a 100644 --- a/src/cli/program.smoke.e2e.test.ts +++ b/src/cli/program.smoke.e2e.test.ts @@ -57,7 +57,7 @@ describe("cli program (smoke)", () => { "123e4567-e89b-12d3-a456-426614174000", ], }, - ])("$label", async ({ argv }) => { + ])("message command: $label", async ({ argv }) => { await expect(runProgram(argv)).rejects.toThrow("exit"); expect(messageCommand).toHaveBeenCalled(); }); @@ -92,7 +92,7 @@ describe("cli program (smoke)", () => { expectedTimeoutMs: undefined, expectedWarning: 'warning: invalid --timeout-ms "nope"; ignoring', }, - ])("$label", async ({ argv, expectedTimeoutMs, expectedWarning }) => { + ])("tui command: $label", async ({ argv, expectedTimeoutMs, expectedWarning }) => { await runProgram(argv); if (expectedWarning) { expect(runtime.error).toHaveBeenCalledWith(expectedWarning); @@ -118,7 +118,7 @@ describe("cli program (smoke)", () => { expectSetupCalled: false, expectOnboardCalled: true, }, - ])("$label", async ({ argv, expectSetupCalled, expectOnboardCalled }) => { + ])("setup command: $label", async ({ argv, expectSetupCalled, expectOnboardCalled }) => { await runProgram(argv); expect(setupCommand).toHaveBeenCalledTimes(expectSetupCalled ? 1 : 0); expect(onboardCommand).toHaveBeenCalledTimes(expectOnboardCalled ? 1 : 0); @@ -248,7 +248,7 @@ describe("cli program (smoke)", () => { runtime, ), }, - ])("$label", async ({ argv, expectCall }) => { + ])("channels command: $label", async ({ argv, expectCall }) => { await runProgram(argv); expectCall(); }); diff --git a/src/commands/channels.add.test.ts b/src/commands/channels.add.test.ts index e6d0c101d..aad2a5bb0 100644 --- a/src/commands/channels.add.test.ts +++ b/src/commands/channels.add.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it } from "vitest"; +import { beforeAll, beforeEach, describe, expect, it } from "vitest"; import { setDefaultChannelPluginRegistryForTests } from "./channel-test-helpers.js"; import { configMocks, offsetMocks } from "./channels.mock-harness.js"; import { baseConfigSnapshot, createTestRuntime } from "./test-runtime-config-helpers.js"; @@ -7,6 +7,10 @@ const runtime = createTestRuntime(); let channelsAddCommand: typeof import("./channels.js").channelsAddCommand; describe("channelsAddCommand", () => { + beforeAll(async () => { + ({ channelsAddCommand } = await import("./channels.js")); + }); + beforeEach(async () => { configMocks.readConfigFileSnapshot.mockReset(); configMocks.writeConfigFile.mockClear(); @@ -15,7 +19,6 @@ describe("channelsAddCommand", () => { runtime.error.mockClear(); runtime.exit.mockClear(); setDefaultChannelPluginRegistryForTests(); - ({ channelsAddCommand } = await import("./channels.js")); }); it("clears telegram update offsets when the token changes", async () => { diff --git a/src/commands/channels.adds-non-default-telegram-account.e2e.test.ts b/src/commands/channels.adds-non-default-telegram-account.e2e.test.ts index 84f2ff60d..936a113ba 100644 --- a/src/commands/channels.adds-non-default-telegram-account.e2e.test.ts +++ b/src/commands/channels.adds-non-default-telegram-account.e2e.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { setDefaultChannelPluginRegistryForTests } from "./channel-test-helpers.js"; import { configMocks, offsetMocks } from "./channels.mock-harness.js"; import { baseConfigSnapshot, createTestRuntime } from "./test-runtime-config-helpers.js"; @@ -23,8 +23,13 @@ import { } from "./channels.js"; const runtime = createTestRuntime(); +let clackPrompterModule: typeof import("../wizard/clack-prompter.js"); describe("channels command", () => { + beforeAll(async () => { + clackPrompterModule = await import("../wizard/clack-prompter.js"); + }); + beforeEach(() => { configMocks.readConfigFileSnapshot.mockReset(); configMocks.writeConfigFile.mockClear(); @@ -176,9 +181,8 @@ describe("channels command", () => { }); const prompt = { confirm: vi.fn().mockResolvedValue(true) }; - const prompterModule = await import("../wizard/clack-prompter.js"); const promptSpy = vi - .spyOn(prompterModule, "createClackPrompter") + .spyOn(clackPrompterModule, "createClackPrompter") .mockReturnValue(prompt as never); await channelsRemoveCommand({ channel: "discord", account: "default" }, runtime, { @@ -498,9 +502,8 @@ describe("channels command", () => { }); const prompt = { confirm: vi.fn().mockResolvedValue(true) }; - const prompterModule = await import("../wizard/clack-prompter.js"); const promptSpy = vi - .spyOn(prompterModule, "createClackPrompter") + .spyOn(clackPrompterModule, "createClackPrompter") .mockReturnValue(prompt as never); await channelsRemoveCommand({ channel: "telegram", account: "default" }, runtime, { diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index 876c698cc..0199f8bc5 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -921,6 +921,10 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { } } + if (shouldRepair && pendingChanges) { + shouldWriteConfig = true; + } + noteOpencodeProviderOverrides(cfg); return { diff --git a/src/commands/doctor.runs-legacy-state-migrations-yes-mode-without.e2e.test.ts b/src/commands/doctor.runs-legacy-state-migrations-yes-mode-without.e2e.test.ts index a6a0f988b..ca8c156f1 100644 --- a/src/commands/doctor.runs-legacy-state-migrations-yes-mode-without.e2e.test.ts +++ b/src/commands/doctor.runs-legacy-state-migrations-yes-mode-without.e2e.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from "vitest"; +import { beforeAll, describe, expect, it, vi } from "vitest"; import { arrangeLegacyStateMigrationTest, confirm, @@ -10,7 +10,15 @@ import { writeConfigFile, } from "./doctor.e2e-harness.js"; +let doctorCommand: typeof import("./doctor.js").doctorCommand; +let healthCommand: typeof import("./health.js").healthCommand; + describe("doctor command", () => { + beforeAll(async () => { + ({ doctorCommand } = await import("./doctor.js")); + ({ healthCommand } = await import("./health.js")); + }); + it("runs legacy state migrations in yes mode without prompting", async () => { const { doctorCommand, runtime, runLegacyStateMigrations } = await arrangeLegacyStateMigrationTest(); @@ -40,14 +48,12 @@ describe("doctor command", () => { it("skips gateway restarts in non-interactive mode", async () => { mockDoctorConfigSnapshot(); - const { healthCommand } = await import("./health.js"); vi.mocked(healthCommand).mockRejectedValueOnce(new Error("gateway closed")); serviceIsLoaded.mockResolvedValueOnce(true); serviceRestart.mockClear(); confirm.mockClear(); - const { doctorCommand } = await import("./doctor.js"); await doctorCommand(createDoctorRuntime(), { nonInteractive: true }); expect(serviceRestart).not.toHaveBeenCalled(); @@ -79,7 +85,6 @@ describe("doctor command", () => { }, }); - const { doctorCommand } = await import("./doctor.js"); await doctorCommand(createDoctorRuntime(), { yes: true }); const written = writeConfigFile.mock.calls.at(-1)?.[0] as Record; diff --git a/src/commands/doctor.warns-per-agent-sandbox-docker-browser-prune.e2e.test.ts b/src/commands/doctor.warns-per-agent-sandbox-docker-browser-prune.e2e.test.ts index 73c728229..2fbe02bdf 100644 --- a/src/commands/doctor.warns-per-agent-sandbox-docker-browser-prune.e2e.test.ts +++ b/src/commands/doctor.warns-per-agent-sandbox-docker-browser-prune.e2e.test.ts @@ -1,10 +1,16 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { describe, expect, it, vi } from "vitest"; +import { beforeAll, describe, expect, it, vi } from "vitest"; import { createDoctorRuntime, mockDoctorConfigSnapshot, note } from "./doctor.e2e-harness.js"; +let doctorCommand: typeof import("./doctor.js").doctorCommand; + describe("doctor command", () => { + beforeAll(async () => { + ({ doctorCommand } = await import("./doctor.js")); + }); + it("warns when per-agent sandbox docker/browser/prune overrides are ignored under shared scope", async () => { mockDoctorConfigSnapshot({ config: { @@ -34,7 +40,6 @@ describe("doctor command", () => { note.mockClear(); - const { doctorCommand } = await import("./doctor.js"); await doctorCommand(createDoctorRuntime(), { nonInteractive: true }); expect( @@ -74,7 +79,6 @@ describe("doctor command", () => { return realExists(value as never); }); - const { doctorCommand } = await import("./doctor.js"); await doctorCommand(createDoctorRuntime(), { nonInteractive: true }); expect(note.mock.calls.some(([_, title]) => title === "Extra workspace")).toBe(false); diff --git a/src/commands/doctor.warns-state-directory-is-missing.e2e.test.ts b/src/commands/doctor.warns-state-directory-is-missing.e2e.test.ts index ceb318b42..89de182f7 100644 --- a/src/commands/doctor.warns-state-directory-is-missing.e2e.test.ts +++ b/src/commands/doctor.warns-state-directory-is-missing.e2e.test.ts @@ -1,10 +1,16 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { describe, expect, it } from "vitest"; +import { beforeAll, describe, expect, it } from "vitest"; import { createDoctorRuntime, mockDoctorConfigSnapshot, note } from "./doctor.e2e-harness.js"; +let doctorCommand: typeof import("./doctor.js").doctorCommand; + describe("doctor command", () => { + beforeAll(async () => { + ({ doctorCommand } = await import("./doctor.js")); + }); + it("warns when the state directory is missing", async () => { mockDoctorConfigSnapshot(); @@ -13,7 +19,6 @@ describe("doctor command", () => { process.env.OPENCLAW_STATE_DIR = missingDir; note.mockClear(); - const { doctorCommand } = await import("./doctor.js"); await doctorCommand(createDoctorRuntime(), { nonInteractive: true, workspaceSuggestions: false, @@ -38,7 +43,6 @@ describe("doctor command", () => { }, }); - const { doctorCommand } = await import("./doctor.js"); await doctorCommand(createDoctorRuntime(), { nonInteractive: true, workspaceSuggestions: false, @@ -63,7 +67,6 @@ describe("doctor command", () => { note.mockClear(); try { - const { doctorCommand } = await import("./doctor.js"); await doctorCommand(createDoctorRuntime(), { nonInteractive: true, workspaceSuggestions: false, diff --git a/src/commands/model-picker.e2e.test.ts b/src/commands/model-picker.e2e.test.ts index 375ae994b..76ced67ba 100644 --- a/src/commands/model-picker.e2e.test.ts +++ b/src/commands/model-picker.e2e.test.ts @@ -61,28 +61,6 @@ function createSelectAllMultiselect() { } describe("promptDefaultModel", () => { - it("filters internal router models from the selection list", async () => { - loadModelCatalog.mockResolvedValue(OPENROUTER_CATALOG); - - const select = vi.fn(async (params) => { - const first = params.options[0]; - return first?.value ?? ""; - }); - const prompter = makePrompter({ select }); - const config = { agents: { defaults: {} } } as OpenClawConfig; - - await promptDefaultModel({ - config, - prompter, - allowKeep: false, - includeManual: false, - ignoreAllowlist: true, - }); - - const options = select.mock.calls[0]?.[0]?.options ?? []; - expectRouterModelFiltering(options); - }); - it("supports configuring vLLM during onboarding", async () => { loadModelCatalog.mockResolvedValue([ { @@ -133,21 +111,6 @@ describe("promptDefaultModel", () => { }); describe("promptModelAllowlist", () => { - it("filters internal router models from the selection list", async () => { - loadModelCatalog.mockResolvedValue(OPENROUTER_CATALOG); - - const multiselect = createSelectAllMultiselect(); - const prompter = makePrompter({ multiselect }); - const config = { agents: { defaults: {} } } as OpenClawConfig; - - await promptModelAllowlist({ config, prompter }); - - const call = multiselect.mock.calls[0]?.[0]; - const options = call?.options ?? []; - expectRouterModelFiltering(options as Array<{ value: string }>); - expect(call?.searchable).toBe(true); - }); - it("filters to allowed keys when provided", async () => { loadModelCatalog.mockResolvedValue([ { @@ -184,6 +147,37 @@ describe("promptModelAllowlist", () => { }); }); +describe("router model filtering", () => { + it("filters internal router models in both default and allowlist prompts", async () => { + loadModelCatalog.mockResolvedValue(OPENROUTER_CATALOG); + + const select = vi.fn(async (params) => { + const first = params.options[0]; + return first?.value ?? ""; + }); + const multiselect = createSelectAllMultiselect(); + const defaultPrompter = makePrompter({ select }); + const allowlistPrompter = makePrompter({ multiselect }); + const config = { agents: { defaults: {} } } as OpenClawConfig; + + await promptDefaultModel({ + config, + prompter: defaultPrompter, + allowKeep: false, + includeManual: false, + ignoreAllowlist: true, + }); + await promptModelAllowlist({ config, prompter: allowlistPrompter }); + + const defaultOptions = select.mock.calls[0]?.[0]?.options ?? []; + expectRouterModelFiltering(defaultOptions); + + const allowlistCall = multiselect.mock.calls[0]?.[0]; + expectRouterModelFiltering(allowlistCall?.options as Array<{ value: string }>); + expect(allowlistCall?.searchable).toBe(true); + }); +}); + describe("applyModelAllowlist", () => { it("preserves existing entries for selected models", () => { const config = { diff --git a/src/commands/models.set.e2e.test.ts b/src/commands/models.set.e2e.test.ts index 0a40b1e8a..625b92d1d 100644 --- a/src/commands/models.set.e2e.test.ts +++ b/src/commands/models.set.e2e.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; const readConfigFileSnapshot = vi.fn(); const writeConfigFile = vi.fn().mockResolvedValue(undefined); @@ -43,7 +43,15 @@ function expectWrittenPrimaryModel(model: string) { }); } +let modelsSetCommand: typeof import("./models/set.js").modelsSetCommand; +let modelsFallbacksAddCommand: typeof import("./models/fallbacks.js").modelsFallbacksAddCommand; + describe("models set + fallbacks", () => { + beforeAll(async () => { + ({ modelsSetCommand } = await import("./models/set.js")); + ({ modelsFallbacksAddCommand } = await import("./models/fallbacks.js")); + }); + beforeEach(() => { readConfigFileSnapshot.mockReset(); writeConfigFile.mockClear(); @@ -52,7 +60,6 @@ describe("models set + fallbacks", () => { it("normalizes z.ai provider in models set", async () => { mockConfigSnapshot({}); const runtime = makeRuntime(); - const { modelsSetCommand } = await import("./models/set.js"); await modelsSetCommand("z.ai/glm-4.7", runtime); @@ -62,7 +69,6 @@ describe("models set + fallbacks", () => { it("normalizes z-ai provider in models fallbacks add", async () => { mockConfigSnapshot({ agents: { defaults: { model: { fallbacks: [] } } } }); const runtime = makeRuntime(); - const { modelsFallbacksAddCommand } = await import("./models/fallbacks.js"); await modelsFallbacksAddCommand("z-ai/glm-4.7", runtime); @@ -79,7 +85,6 @@ describe("models set + fallbacks", () => { it("normalizes provider casing in models set", async () => { mockConfigSnapshot({}); const runtime = makeRuntime(); - const { modelsSetCommand } = await import("./models/set.js"); await modelsSetCommand("Z.AI/glm-4.7", runtime); diff --git a/src/commands/onboard-auth.e2e.test.ts b/src/commands/onboard-auth.e2e.test.ts index 49401616d..f103f805f 100644 --- a/src/commands/onboard-auth.e2e.test.ts +++ b/src/commands/onboard-auth.e2e.test.ts @@ -324,16 +324,6 @@ describe("applyMinimaxApiConfig", () => { expect(cfg.models?.providers?.minimax?.models[0]?.reasoning).toBe(false); }); - it("preserves existing model fallbacks", () => { - const cfg = applyMinimaxApiConfig(createConfigWithFallbacks()); - expectFallbacksPreserved(cfg); - }); - - it("adds model alias", () => { - const cfg = applyMinimaxApiConfig({}, "MiniMax-M2.1"); - expect(cfg.agents?.defaults?.models?.["minimax/MiniMax-M2.1"]?.alias).toBe("Minimax"); - }); - it("preserves existing model params when adding alias", () => { const cfg = applyMinimaxApiConfig( { @@ -530,19 +520,9 @@ describe("applyXaiConfig", () => { }); expect(cfg.agents?.defaults?.model?.primary).toBe(XAI_DEFAULT_MODEL_REF); }); - - it("preserves existing model fallbacks", () => { - const cfg = applyXaiConfig(createConfigWithFallbacks()); - expectFallbacksPreserved(cfg); - }); }); describe("applyXaiProviderConfig", () => { - it("adds model alias", () => { - const cfg = applyXaiProviderConfig({}); - expect(cfg.agents?.defaults?.models?.[XAI_DEFAULT_MODEL_REF]?.alias).toBe("Grok"); - }); - it("merges xAI models and keeps existing provider overrides", () => { const cfg = applyXaiProviderConfig( createLegacyProviderConfig({ @@ -560,6 +540,37 @@ describe("applyXaiProviderConfig", () => { }); }); +describe("fallback preservation helpers", () => { + it("preserves existing model fallbacks", () => { + const fallbackCases = [applyMinimaxApiConfig, applyXaiConfig] as const; + for (const applyConfig of fallbackCases) { + const cfg = applyConfig(createConfigWithFallbacks()); + expectFallbacksPreserved(cfg); + } + }); +}); + +describe("provider alias defaults", () => { + it("adds expected alias for provider defaults", () => { + const aliasCases = [ + { + applyConfig: () => applyMinimaxApiConfig({}, "MiniMax-M2.1"), + modelRef: "minimax/MiniMax-M2.1", + alias: "Minimax", + }, + { + applyConfig: () => applyXaiProviderConfig({}), + modelRef: XAI_DEFAULT_MODEL_REF, + alias: "Grok", + }, + ] as const; + for (const testCase of aliasCases) { + const cfg = testCase.applyConfig(); + expect(cfg.agents?.defaults?.models?.[testCase.modelRef]?.alias).toBe(testCase.alias); + } + }); +}); + describe("allowlist provider helpers", () => { it("adds allowlist entry and preserves alias", () => { const providerCases = [ diff --git a/src/commands/onboard-non-interactive.provider-auth.e2e.test.ts b/src/commands/onboard-non-interactive.provider-auth.e2e.test.ts index b2da8c10a..bfd5e2e40 100644 --- a/src/commands/onboard-non-interactive.provider-auth.e2e.test.ts +++ b/src/commands/onboard-non-interactive.provider-auth.e2e.test.ts @@ -1,7 +1,7 @@ import fs from "node:fs/promises"; import path from "node:path"; import { setTimeout as delay } from "node:timers/promises"; -import { describe, expect, it } from "vitest"; +import { beforeAll, describe, expect, it } from "vitest"; import { makeTempWorkspace } from "../test-helpers/workspace.js"; import { withEnvAsync } from "../test-utils/env.js"; import { MINIMAX_API_BASE_URL, MINIMAX_CN_API_BASE_URL } from "./onboard-auth.js"; @@ -17,6 +17,8 @@ type OnboardEnv = { configPath: string; runtime: NonInteractiveRuntime; }; +let ensureAuthProfileStore: typeof import("../agents/auth-profiles.js").ensureAuthProfileStore; +let upsertAuthProfile: typeof import("../agents/auth-profiles.js").upsertAuthProfile; type ProviderAuthConfigSnapshot = { auth?: { profiles?: Record }; @@ -121,7 +123,6 @@ async function expectApiKeyProfile(params: { key: string; metadata?: Record; }): Promise { - const { ensureAuthProfileStore } = await import("../agents/auth-profiles.js"); const store = ensureAuthProfileStore(); const profile = store.profiles[params.profileId]; expect(profile?.type).toBe("api_key"); @@ -135,6 +136,10 @@ async function expectApiKeyProfile(params: { } describe("onboard (non-interactive): provider auth", () => { + beforeAll(async () => { + ({ ensureAuthProfileStore, upsertAuthProfile } = await import("../agents/auth-profiles.js")); + }); + it("stores MiniMax API key and uses global baseUrl by default", async () => { await withOnboardEnv("openclaw-onboard-minimax-", async (env) => { const cfg = await runOnboardingAndReadConfig(env, { @@ -274,7 +279,6 @@ describe("onboard (non-interactive): provider auth", () => { expect(cfg.auth?.profiles?.["anthropic:default"]?.provider).toBe("anthropic"); expect(cfg.auth?.profiles?.["anthropic:default"]?.mode).toBe("token"); - const { ensureAuthProfileStore } = await import("../agents/auth-profiles.js"); const store = ensureAuthProfileStore(); const profile = store.profiles["anthropic:default"]; expect(profile?.type).toBe("token"); @@ -465,7 +469,6 @@ describe("onboard (non-interactive): provider auth", () => { await withOnboardEnv( "openclaw-onboard-custom-provider-profile-fallback-", async ({ configPath, runtime }) => { - const { upsertAuthProfile } = await import("../agents/auth-profiles.js"); upsertAuthProfile({ profileId: `${CUSTOM_LOCAL_PROVIDER_ID}:default`, credential: { diff --git a/src/commands/openai-model-default.e2e.test.ts b/src/commands/openai-model-default.e2e.test.ts index faf0f1ee0..5c099ddd9 100644 --- a/src/commands/openai-model-default.e2e.test.ts +++ b/src/commands/openai-model-default.e2e.test.ts @@ -49,6 +49,36 @@ function expectConfigUnchanged( expect(applied.next).toEqual(cfg); } +type SharedDefaultModelCase = { + apply: (cfg: OpenClawConfig) => { changed: boolean; next: OpenClawConfig }; + defaultModel: string; + overrideConfig: OpenClawConfig; + alreadyDefaultConfig: OpenClawConfig; +}; + +const SHARED_DEFAULT_MODEL_CASES: SharedDefaultModelCase[] = [ + { + apply: applyGoogleGeminiModelDefault, + defaultModel: GOOGLE_GEMINI_DEFAULT_MODEL, + overrideConfig: { + agents: { defaults: { model: { primary: "anthropic/claude-opus-4-5" } } }, + } as OpenClawConfig, + alreadyDefaultConfig: { + agents: { defaults: { model: { primary: GOOGLE_GEMINI_DEFAULT_MODEL } } }, + } as OpenClawConfig, + }, + { + apply: applyOpencodeZenModelDefault, + defaultModel: OPENCODE_ZEN_DEFAULT_MODEL, + overrideConfig: { + agents: { defaults: { model: "anthropic/claude-opus-4-5" } }, + } as OpenClawConfig, + alreadyDefaultConfig: { + agents: { defaults: { model: OPENCODE_ZEN_DEFAULT_MODEL } }, + } as OpenClawConfig, + }, +]; + describe("applyDefaultModelChoice", () => { it("ensures allowlist entry exists when returning an agent override", async () => { const defaultModel = "vercel-ai-gateway/anthropic/claude-opus-4.6"; @@ -109,27 +139,27 @@ describe("applyDefaultModelChoice", () => { }); }); -describe("applyGoogleGeminiModelDefault", () => { - it("sets gemini default when model is unset", () => { - const cfg: OpenClawConfig = { agents: { defaults: {} } }; - const applied = applyGoogleGeminiModelDefault(cfg); - expectPrimaryModelChanged(applied, GOOGLE_GEMINI_DEFAULT_MODEL); +describe("shared default model behavior", () => { + it("sets defaults when model is unset", () => { + for (const testCase of SHARED_DEFAULT_MODEL_CASES) { + const cfg: OpenClawConfig = { agents: { defaults: {} } }; + const applied = testCase.apply(cfg); + expectPrimaryModelChanged(applied, testCase.defaultModel); + } }); - it("overrides existing model", () => { - const cfg: OpenClawConfig = { - agents: { defaults: { model: { primary: "anthropic/claude-opus-4-5" } } }, - }; - const applied = applyGoogleGeminiModelDefault(cfg); - expectPrimaryModelChanged(applied, GOOGLE_GEMINI_DEFAULT_MODEL); + it("overrides existing models", () => { + for (const testCase of SHARED_DEFAULT_MODEL_CASES) { + const applied = testCase.apply(testCase.overrideConfig); + expectPrimaryModelChanged(applied, testCase.defaultModel); + } }); - it("no-ops when already gemini default", () => { - const cfg: OpenClawConfig = { - agents: { defaults: { model: { primary: GOOGLE_GEMINI_DEFAULT_MODEL } } }, - }; - const applied = applyGoogleGeminiModelDefault(cfg); - expectConfigUnchanged(applied, cfg); + it("no-ops when already on the target default", () => { + for (const testCase of SHARED_DEFAULT_MODEL_CASES) { + const applied = testCase.apply(testCase.alreadyDefaultConfig); + expectConfigUnchanged(applied, testCase.alreadyDefaultConfig); + } }); }); @@ -200,28 +230,6 @@ describe("applyOpenAICodexModelDefault", () => { }); describe("applyOpencodeZenModelDefault", () => { - it("sets opencode default when model is unset", () => { - const cfg: OpenClawConfig = { agents: { defaults: {} } }; - const applied = applyOpencodeZenModelDefault(cfg); - expectPrimaryModelChanged(applied, OPENCODE_ZEN_DEFAULT_MODEL); - }); - - it("overrides existing model", () => { - const cfg = { - agents: { defaults: { model: "anthropic/claude-opus-4-5" } }, - } as OpenClawConfig; - const applied = applyOpencodeZenModelDefault(cfg); - expectPrimaryModelChanged(applied, OPENCODE_ZEN_DEFAULT_MODEL); - }); - - it("no-ops when already opencode-zen default", () => { - const cfg = { - agents: { defaults: { model: OPENCODE_ZEN_DEFAULT_MODEL } }, - } as OpenClawConfig; - const applied = applyOpencodeZenModelDefault(cfg); - expectConfigUnchanged(applied, cfg); - }); - it("no-ops when already legacy opencode-zen default", () => { const cfg = { agents: { defaults: { model: "opencode-zen/claude-opus-4-5" } }, 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 e685f326f..7d2a54ddb 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 @@ -98,7 +98,7 @@ describe("legacy config detection", () => { ?.groupPolicy, "allowlist", ], - ])("%s", (_name, config, readValue, expectedValue) => { + ])("defaults: %s", (_name, config, readValue, expectedValue) => { expectValidConfigValue({ config, readValue, expectedValue }); }); it("rejects unsafe executable config values", async () => { @@ -149,7 +149,7 @@ describe("legacy config detection", () => { { channels: { slack: { dmPolicy: "open", allowFrom: ["U123"] } } }, "channels.slack.allowFrom", ], - ])("%s", (_name, config, expectedPath) => { + ])("rejects: %s", (_name, config, expectedPath) => { expectInvalidIssuePath(config, expectedPath); }); diff --git a/src/config/includes.test.ts b/src/config/includes.test.ts index 25ae27e65..a219ebb9e 100644 --- a/src/config/includes.test.ts +++ b/src/config/includes.test.ts @@ -119,21 +119,18 @@ describe("resolveConfigIncludes", () => { }); it("throws when sibling keys are used with non-object includes", () => { - const files = { [configPath("list.json")]: ["a", "b"] }; - const obj = { $include: "./list.json", extra: true }; - expect(() => resolve(obj, files)).toThrow(ConfigIncludeError); - expect(() => resolve(obj, files)).toThrow( - /Sibling keys require included content to be an object/, - ); - }); - - it("throws when sibling keys are used with primitive includes", () => { - const files = { [configPath("value.json")]: "hello" }; - const obj = { $include: "./value.json", extra: true }; - expect(() => resolve(obj, files)).toThrow(ConfigIncludeError); - expect(() => resolve(obj, files)).toThrow( - /Sibling keys require included content to be an object/, - ); + const cases = [ + { includeFile: "list.json", included: ["a", "b"] }, + { includeFile: "value.json", included: "hello" }, + ] as const; + for (const testCase of cases) { + const files = { [configPath(testCase.includeFile)]: testCase.included }; + const obj = { $include: `./${testCase.includeFile}`, extra: true }; + expect(() => resolve(obj, files), testCase.includeFile).toThrow(ConfigIncludeError); + expect(() => resolve(obj, files), testCase.includeFile).toThrow( + /Sibling keys require included content to be an object/, + ); + } }); it("resolves nested includes", () => { @@ -196,31 +193,30 @@ describe("resolveConfigIncludes", () => { } }); - it("throws ConfigIncludeError for invalid $include value type", () => { - const obj = { $include: 123 }; - expect(() => resolve(obj)).toThrow(ConfigIncludeError); - expect(() => resolve(obj)).toThrow(/expected string or array/); - }); - - it("throws ConfigIncludeError for invalid array item type", () => { - const files = { [configPath("valid.json")]: { valid: true } }; - const obj = { $include: ["./valid.json", 123] }; - expect(() => resolve(obj, files)).toThrow(ConfigIncludeError); - expect(() => resolve(obj, files)).toThrow(/expected string, got number/); - }); - - it("throws ConfigIncludeError for null/boolean include items", () => { + it("throws on invalid include value/item types", () => { const files = { [configPath("valid.json")]: { valid: true } }; const cases = [ - { value: null, expected: "object" }, - { value: false, expected: "boolean" }, - ]; - for (const item of cases) { - const obj = { $include: ["./valid.json", item.value] }; - expect(() => resolve(obj, files)).toThrow(ConfigIncludeError); - expect(() => resolve(obj, files)).toThrow( - new RegExp(`expected string, got ${item.expected}`), - ); + { + obj: { $include: 123 }, + expectedPattern: /expected string or array/, + }, + { + obj: { $include: ["./valid.json", 123] }, + expectedPattern: /expected string, got number/, + }, + { + obj: { $include: ["./valid.json", null] }, + expectedPattern: /expected string, got object/, + }, + { + obj: { $include: ["./valid.json", false] }, + expectedPattern: /expected string, got boolean/, + }, + ] as const; + + for (const testCase of cases) { + expect(() => resolve(testCase.obj, files)).toThrow(ConfigIncludeError); + expect(() => resolve(testCase.obj, files)).toThrow(testCase.expectedPattern); } }); @@ -304,158 +300,154 @@ describe("resolveConfigIncludes", () => { }); describe("real-world config patterns", () => { - it("supports per-client agent includes", () => { - const files = { - [configPath("clients", "mueller.json")]: { - agents: [ - { - id: "mueller-screenshot", - workspace: "~/clients/mueller/screenshot", + it("supports common modular include layouts", () => { + const cases = [ + { + name: "per-client agent includes", + files: { + [configPath("clients", "mueller.json")]: { + agents: [ + { + id: "mueller-screenshot", + workspace: "~/clients/mueller/screenshot", + }, + { + id: "mueller-transcribe", + workspace: "~/clients/mueller/transcribe", + }, + ], + broadcast: { + "group-mueller": ["mueller-screenshot", "mueller-transcribe"], + }, }, - { - id: "mueller-transcribe", - workspace: "~/clients/mueller/transcribe", + [configPath("clients", "schmidt.json")]: { + agents: [ + { + id: "schmidt-screenshot", + workspace: "~/clients/schmidt/screenshot", + }, + ], + broadcast: { "group-schmidt": ["schmidt-screenshot"] }, + }, + }, + obj: { + gateway: { port: 18789 }, + $include: ["./clients/mueller.json", "./clients/schmidt.json"], + }, + expected: { + gateway: { port: 18789 }, + agents: [ + { id: "mueller-screenshot", workspace: "~/clients/mueller/screenshot" }, + { id: "mueller-transcribe", workspace: "~/clients/mueller/transcribe" }, + { id: "schmidt-screenshot", workspace: "~/clients/schmidt/screenshot" }, + ], + broadcast: { + "group-mueller": ["mueller-screenshot", "mueller-transcribe"], + "group-schmidt": ["schmidt-screenshot"], }, - ], - broadcast: { - "group-mueller": ["mueller-screenshot", "mueller-transcribe"], }, }, - [configPath("clients", "schmidt.json")]: { - agents: [ - { - id: "schmidt-screenshot", - workspace: "~/clients/schmidt/screenshot", + { + name: "modular config structure", + files: { + [configPath("gateway.json")]: { + gateway: { port: 18789, bind: "loopback" }, }, - ], - broadcast: { "group-schmidt": ["schmidt-screenshot"] }, + [configPath("channels", "whatsapp.json")]: { + channels: { whatsapp: { dmPolicy: "pairing", allowFrom: ["+49123"] } }, + }, + [configPath("agents", "defaults.json")]: { + agents: { defaults: { sandbox: { mode: "all" } } }, + }, + }, + obj: { + $include: ["./gateway.json", "./channels/whatsapp.json", "./agents/defaults.json"], + }, + expected: { + gateway: { port: 18789, bind: "loopback" }, + channels: { whatsapp: { dmPolicy: "pairing", allowFrom: ["+49123"] } }, + agents: { defaults: { sandbox: { mode: "all" } } }, + }, }, - }; + ] as const; - const obj = { - gateway: { port: 18789 }, - $include: ["./clients/mueller.json", "./clients/schmidt.json"], - }; - - expect(resolve(obj, files)).toEqual({ - gateway: { port: 18789 }, - agents: [ - { id: "mueller-screenshot", workspace: "~/clients/mueller/screenshot" }, - { id: "mueller-transcribe", workspace: "~/clients/mueller/transcribe" }, - { id: "schmidt-screenshot", workspace: "~/clients/schmidt/screenshot" }, - ], - broadcast: { - "group-mueller": ["mueller-screenshot", "mueller-transcribe"], - "group-schmidt": ["schmidt-screenshot"], - }, - }); - }); - - it("supports modular config structure", () => { - const files = { - [configPath("gateway.json")]: { - gateway: { port: 18789, bind: "loopback" }, - }, - [configPath("channels", "whatsapp.json")]: { - channels: { whatsapp: { dmPolicy: "pairing", allowFrom: ["+49123"] } }, - }, - [configPath("agents", "defaults.json")]: { - agents: { defaults: { sandbox: { mode: "all" } } }, - }, - }; - - const obj = { - $include: ["./gateway.json", "./channels/whatsapp.json", "./agents/defaults.json"], - }; - - expect(resolve(obj, files)).toEqual({ - gateway: { port: 18789, bind: "loopback" }, - channels: { whatsapp: { dmPolicy: "pairing", allowFrom: ["+49123"] } }, - agents: { defaults: { sandbox: { mode: "all" } } }, - }); + for (const testCase of cases) { + expect(resolve(testCase.obj, testCase.files), testCase.name).toEqual(testCase.expected); + } }); }); describe("security: path traversal protection (CWE-22)", () => { describe("absolute path attacks", () => { - it("rejects /etc/passwd", () => { - const obj = { $include: "/etc/passwd" }; - expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); - expect(() => resolve(obj, {})).toThrow(/escapes config directory/); - }); - - it("rejects /etc/shadow", () => { - const obj = { $include: "/etc/shadow" }; - expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); - expect(() => resolve(obj, {})).toThrow(/escapes config directory/); - }); - - it("rejects home directory SSH key", () => { - const obj = { $include: `${process.env.HOME}/.ssh/id_rsa` }; - expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); - }); - - it("rejects /tmp paths", () => { - const obj = { $include: "/tmp/malicious.json" }; - expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); - }); - - it("rejects root directory", () => { - const obj = { $include: "/" }; - expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); + it("rejects absolute path attack variants", () => { + const cases = [ + { includePath: "/etc/passwd", expectEscapesMessage: true }, + { includePath: "/etc/shadow", expectEscapesMessage: true }, + { includePath: `${process.env.HOME}/.ssh/id_rsa`, expectEscapesMessage: false }, + { includePath: "/tmp/malicious.json", expectEscapesMessage: false }, + { includePath: "/", expectEscapesMessage: false }, + ] as const; + for (const testCase of cases) { + const obj = { $include: testCase.includePath }; + expect(() => resolve(obj, {}), testCase.includePath).toThrow(ConfigIncludeError); + if (testCase.expectEscapesMessage) { + expect(() => resolve(obj, {}), testCase.includePath).toThrow(/escapes config directory/); + } + } }); }); describe("relative traversal attacks", () => { - it("rejects ../../etc/passwd", () => { - const obj = { $include: "../../etc/passwd" }; - expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); - expect(() => resolve(obj, {})).toThrow(/escapes config directory/); - }); - - it("rejects ../../../etc/shadow", () => { - const obj = { $include: "../../../etc/shadow" }; - expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); - }); - - it("rejects deeply nested traversal", () => { - const obj = { $include: "../../../../../../../../etc/passwd" }; - expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); - }); - - it("rejects traversal to parent of config directory", () => { - const obj = { $include: "../sibling-dir/secret.json" }; - expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); - }); - - it("rejects mixed absolute and traversal", () => { - const obj = { $include: "/config/../../../etc/passwd" }; - expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); + it("rejects relative traversal path variants", () => { + const cases = [ + { includePath: "../../etc/passwd", expectEscapesMessage: true }, + { includePath: "../../../etc/shadow", expectEscapesMessage: false }, + { includePath: "../../../../../../../../etc/passwd", expectEscapesMessage: false }, + { includePath: "../sibling-dir/secret.json", expectEscapesMessage: false }, + { includePath: "/config/../../../etc/passwd", expectEscapesMessage: false }, + ] as const; + for (const testCase of cases) { + const obj = { $include: testCase.includePath }; + expect(() => resolve(obj, {}), testCase.includePath).toThrow(ConfigIncludeError); + if (testCase.expectEscapesMessage) { + expect(() => resolve(obj, {}), testCase.includePath).toThrow(/escapes config directory/); + } + } }); }); describe("legitimate includes (should work)", () => { - it("allows relative include in same directory", () => { - const files = { [configPath("sub.json")]: { key: "value" } }; - const obj = { $include: "./sub.json" }; - expect(resolve(obj, files)).toEqual({ key: "value" }); - }); + it("allows legitimate include paths under config root", () => { + const cases = [ + { + name: "same-directory with ./ prefix", + includePath: "./sub.json", + files: { [configPath("sub.json")]: { key: "value" } }, + expected: { key: "value" }, + }, + { + name: "same-directory without ./ prefix", + includePath: "sub.json", + files: { [configPath("sub.json")]: { key: "value" } }, + expected: { key: "value" }, + }, + { + name: "subdirectory", + includePath: "./sub/nested.json", + files: { [configPath("sub", "nested.json")]: { nested: true } }, + expected: { nested: true }, + }, + { + name: "deep subdirectory", + includePath: "./a/b/c/deep.json", + files: { [configPath("a", "b", "c", "deep.json")]: { deep: true } }, + expected: { deep: true }, + }, + ] as const; - it("allows include without ./ prefix", () => { - const files = { [configPath("sub.json")]: { key: "value" } }; - const obj = { $include: "sub.json" }; - expect(resolve(obj, files)).toEqual({ key: "value" }); - }); - - it("allows include in subdirectory", () => { - const files = { [configPath("sub", "nested.json")]: { nested: true } }; - const obj = { $include: "./sub/nested.json" }; - expect(resolve(obj, files)).toEqual({ nested: true }); - }); - - it("allows deeply nested subdirectory", () => { - const files = { [configPath("a", "b", "c", "deep.json")]: { deep: true } }; - const obj = { $include: "./a/b/c/deep.json" }; - expect(resolve(obj, files)).toEqual({ deep: true }); + for (const testCase of cases) { + const obj = { $include: testCase.includePath }; + expect(resolve(obj, testCase.files), testCase.name).toEqual(testCase.expected); + } }); // Note: Upward traversal from nested configs is restricted for security. @@ -464,52 +456,62 @@ describe("security: path traversal protection (CWE-22)", () => { }); describe("error properties", () => { - it("throws ConfigIncludeError with correct type", () => { - const obj = { $include: "/etc/passwd" }; - try { - resolve(obj, {}); - expect.fail("Should have thrown"); - } catch (err) { - expect(err).toBeInstanceOf(ConfigIncludeError); - expect(err).toHaveProperty("name", "ConfigIncludeError"); - } - }); + it("preserves error type/path/message details", () => { + const cases = [ + { + includePath: "/etc/passwd", + expectedMessageIncludes: ["escapes config directory", "/etc/passwd"], + }, + { + includePath: "/etc/shadow", + expectedMessageIncludes: ["/etc/shadow"], + }, + { + includePath: "../../etc/passwd", + expectedMessageIncludes: ["escapes config directory", "../../etc/passwd"], + }, + ] as const; - it("includes offending path in error", () => { - const maliciousPath = "/etc/shadow"; - const obj = { $include: maliciousPath }; - try { - resolve(obj, {}); - expect.fail("Should have thrown"); - } catch (err) { - expect(err).toBeInstanceOf(ConfigIncludeError); - expect((err as ConfigIncludeError).includePath).toBe(maliciousPath); - } - }); - - it("includes descriptive message", () => { - const obj = { $include: "../../etc/passwd" }; - try { - resolve(obj, {}); - expect.fail("Should have thrown"); - } catch (err) { - expect(err).toBeInstanceOf(ConfigIncludeError); - expect((err as Error).message).toContain("escapes config directory"); - expect((err as Error).message).toContain("../../etc/passwd"); + for (const testCase of cases) { + const obj = { $include: testCase.includePath }; + try { + resolve(obj, {}); + expect.fail("Should have thrown"); + } catch (err) { + expect(err, testCase.includePath).toBeInstanceOf(ConfigIncludeError); + expect(err, testCase.includePath).toHaveProperty("name", "ConfigIncludeError"); + expect((err as ConfigIncludeError).includePath, testCase.includePath).toBe( + testCase.includePath, + ); + for (const messagePart of testCase.expectedMessageIncludes) { + expect((err as Error).message, `${testCase.includePath}: ${messagePart}`).toContain( + messagePart, + ); + } + } } }); }); describe("array includes with malicious paths", () => { - it("rejects array with one malicious path", () => { - const files = { [configPath("good.json")]: { good: true } }; - const obj = { $include: ["./good.json", "/etc/passwd"] }; - expect(() => resolve(obj, files)).toThrow(ConfigIncludeError); - }); + it("rejects arrays that contain malicious include paths", () => { + const cases = [ + { + name: "one malicious path", + files: { [configPath("good.json")]: { good: true } }, + includePaths: ["./good.json", "/etc/passwd"], + }, + { + name: "multiple malicious paths", + files: {}, + includePaths: ["/etc/passwd", "/etc/shadow"], + }, + ] as const; - it("rejects array with multiple malicious paths", () => { - const obj = { $include: ["/etc/passwd", "/etc/shadow"] }; - expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); + for (const testCase of cases) { + const obj = { $include: testCase.includePaths }; + expect(() => resolve(obj, testCase.files), testCase.name).toThrow(ConfigIncludeError); + } }); it("allows array with all legitimate paths", () => { @@ -548,15 +550,20 @@ describe("security: path traversal protection (CWE-22)", () => { }); describe("edge cases", () => { - it("rejects null bytes in path", () => { - const obj = { $include: "./file\x00.json" }; - // Path with null byte should be rejected or handled safely - expect(() => resolve(obj, {})).toThrow(); - }); - - it("rejects double slashes", () => { - const obj = { $include: "//etc/passwd" }; - expect(() => resolve(obj, {})).toThrow(ConfigIncludeError); + it("rejects malformed include paths", () => { + const cases = [ + { includePath: "./file\x00.json", expectedError: undefined }, + { includePath: "//etc/passwd", expectedError: ConfigIncludeError }, + ] as const; + for (const testCase of cases) { + const obj = { $include: testCase.includePath }; + if (testCase.expectedError) { + expect(() => resolve(obj, {}), testCase.includePath).toThrow(testCase.expectedError); + continue; + } + // Path with null byte should be rejected or handled safely. + expect(() => resolve(obj, {}), testCase.includePath).toThrow(); + } }); it("allows child include when config is at filesystem root", () => { diff --git a/src/config/sessions/store.pruning.e2e.test.ts b/src/config/sessions/store.pruning.e2e.test.ts index 0ea3587e5..f78ff4cd3 100644 --- a/src/config/sessions/store.pruning.e2e.test.ts +++ b/src/config/sessions/store.pruning.e2e.test.ts @@ -10,6 +10,8 @@ import type { SessionEntry } from "./types.js"; vi.mock("../config.js", () => ({ loadConfig: vi.fn().mockReturnValue({}), })); +const { loadConfig } = await import("../config.js"); +const mockLoadConfig = vi.mocked(loadConfig) as ReturnType; const DAY_MS = 24 * 60 * 60 * 1000; @@ -45,7 +47,6 @@ describe("Integration: saveSessionStore with pruning", () => { let testDir: string; let storePath: string; let savedCacheTtl: string | undefined; - let mockLoadConfig: ReturnType; beforeAll(async () => { fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-pruning-integ-")); @@ -61,9 +62,7 @@ describe("Integration: saveSessionStore with pruning", () => { savedCacheTtl = process.env.OPENCLAW_SESSION_CACHE_TTL_MS; process.env.OPENCLAW_SESSION_CACHE_TTL_MS = "0"; clearSessionStoreCacheForTest(); - - const configModule = await import("../config.js"); - mockLoadConfig = configModule.loadConfig as ReturnType; + mockLoadConfig.mockReset(); }); afterEach(() => {