diff --git a/src/acp/client.test.ts b/src/acp/client.test.ts index 96822423c94..72958ca57c2 100644 --- a/src/acp/client.test.ts +++ b/src/acp/client.test.ts @@ -126,6 +126,35 @@ describe("resolveAcpClientSpawnInvocation", () => { }); describe("resolvePermissionRequest", () => { + async function expectPromptReject(params: { + request: Partial; + expectedToolName: string | undefined; + expectedTitle: string; + }) { + const prompt = vi.fn(async () => false); + const res = await resolvePermissionRequest(makePermissionRequest(params.request), { + prompt, + log: () => {}, + }); + expect(prompt).toHaveBeenCalledTimes(1); + expect(prompt).toHaveBeenCalledWith(params.expectedToolName, params.expectedTitle); + expect(res).toEqual({ outcome: { outcome: "selected", optionId: "reject" } }); + } + + async function expectAutoAllowWithoutPrompt(params: { + request: Partial; + cwd?: string; + }) { + const prompt = vi.fn(async () => true); + const res = await resolvePermissionRequest(makePermissionRequest(params.request), { + prompt, + log: () => {}, + cwd: params.cwd, + }); + expect(prompt).not.toHaveBeenCalled(); + expect(res).toEqual({ outcome: { outcome: "selected", optionId: "allow" } }); + } + it("auto-approves safe tools without prompting", async () => { const prompt = vi.fn(async () => true); const res = await resolvePermissionRequest(makePermissionRequest(), { prompt, log: () => {} }); @@ -185,37 +214,31 @@ describe("resolvePermissionRequest", () => { }); it("auto-approves read when rawInput path resolves inside cwd", async () => { - const prompt = vi.fn(async () => true); - const res = await resolvePermissionRequest( - makePermissionRequest({ + await expectAutoAllowWithoutPrompt({ + request: { toolCall: { toolCallId: "tool-read-inside-cwd", title: "read: ignored-by-raw-input", status: "pending", rawInput: { path: "docs/security.md" }, }, - }), - { prompt, log: () => {}, cwd: "/tmp/openclaw-acp-cwd" }, - ); - expect(prompt).not.toHaveBeenCalled(); - expect(res).toEqual({ outcome: { outcome: "selected", optionId: "allow" } }); + }, + cwd: "/tmp/openclaw-acp-cwd", + }); }); it("auto-approves read when rawInput file URL resolves inside cwd", async () => { - const prompt = vi.fn(async () => true); - const res = await resolvePermissionRequest( - makePermissionRequest({ + await expectAutoAllowWithoutPrompt({ + request: { toolCall: { toolCallId: "tool-read-inside-cwd-file-url", title: "read: ignored-by-raw-input", status: "pending", rawInput: { path: "file:///tmp/openclaw-acp-cwd/docs/security.md" }, }, - }), - { prompt, log: () => {}, cwd: "/tmp/openclaw-acp-cwd" }, - ); - expect(prompt).not.toHaveBeenCalled(); - expect(res).toEqual({ outcome: { outcome: "selected", optionId: "allow" } }); + }, + cwd: "/tmp/openclaw-acp-cwd", + }); }); it("prompts for read when rawInput path escapes cwd via traversal", async () => { @@ -343,56 +366,47 @@ describe("resolvePermissionRequest", () => { }); it("prompts when metadata tool name contains invalid characters", async () => { - const prompt = vi.fn(async () => false); - const res = await resolvePermissionRequest( - makePermissionRequest({ + await expectPromptReject({ + request: { toolCall: { toolCallId: "tool-invalid-meta", title: "read: src/index.ts", status: "pending", _meta: { toolName: "read.*" }, }, - }), - { prompt, log: () => {} }, - ); - expect(prompt).toHaveBeenCalledTimes(1); - expect(prompt).toHaveBeenCalledWith(undefined, "read: src/index.ts"); - expect(res).toEqual({ outcome: { outcome: "selected", optionId: "reject" } }); + }, + expectedToolName: undefined, + expectedTitle: "read: src/index.ts", + }); }); it("prompts when raw input tool name exceeds max length", async () => { - const prompt = vi.fn(async () => false); - const res = await resolvePermissionRequest( - makePermissionRequest({ + await expectPromptReject({ + request: { toolCall: { toolCallId: "tool-long-raw", title: "read: src/index.ts", status: "pending", rawInput: { toolName: "r".repeat(129) }, }, - }), - { prompt, log: () => {} }, - ); - expect(prompt).toHaveBeenCalledTimes(1); - expect(prompt).toHaveBeenCalledWith(undefined, "read: src/index.ts"); - expect(res).toEqual({ outcome: { outcome: "selected", optionId: "reject" } }); + }, + expectedToolName: undefined, + expectedTitle: "read: src/index.ts", + }); }); it("prompts when title tool name contains non-allowed characters", async () => { - const prompt = vi.fn(async () => false); - const res = await resolvePermissionRequest( - makePermissionRequest({ + await expectPromptReject({ + request: { toolCall: { toolCallId: "tool-bad-title-name", title: "readπŸš€: src/index.ts", status: "pending", }, - }), - { prompt, log: () => {} }, - ); - expect(prompt).toHaveBeenCalledTimes(1); - expect(prompt).toHaveBeenCalledWith(undefined, "readπŸš€: src/index.ts"); - expect(res).toEqual({ outcome: { outcome: "selected", optionId: "reject" } }); + }, + expectedToolName: undefined, + expectedTitle: "readπŸš€: src/index.ts", + }); }); it("returns cancelled when no permission options are present", async () => { diff --git a/src/agents/bash-tools.exec.approval-id.test.ts b/src/agents/bash-tools.exec.approval-id.test.ts index d99e3d6fcbb..3e0b9d6292e 100644 --- a/src/agents/bash-tools.exec.approval-id.test.ts +++ b/src/agents/bash-tools.exec.approval-id.test.ts @@ -2,6 +2,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { buildSystemRunPreparePayload } from "../test-utils/system-run-prepare-payload.js"; vi.mock("./tools/gateway.js", () => ({ callGatewayTool: vi.fn(), @@ -38,20 +39,7 @@ function buildPreparedSystemRunPayload(rawInvokeParams: unknown) { }; }; const params = invoke.params ?? {}; - const argv = Array.isArray(params.command) ? params.command.map(String) : []; - const rawCommand = typeof params.rawCommand === "string" ? params.rawCommand : null; - return { - payload: { - cmdText: rawCommand ?? argv.join(" "), - plan: { - argv, - cwd: typeof params.cwd === "string" ? params.cwd : null, - rawCommand, - agentId: typeof params.agentId === "string" ? params.agentId : null, - sessionKey: typeof params.sessionKey === "string" ? params.sessionKey : null, - }, - }, - }; + return buildSystemRunPreparePayload(params); } describe("exec approvals", () => { diff --git a/src/agents/byteplus.live.test.ts b/src/agents/byteplus.live.test.ts index 1c1b730a387..7da320dc011 100644 --- a/src/agents/byteplus.live.test.ts +++ b/src/agents/byteplus.live.test.ts @@ -2,6 +2,10 @@ import { completeSimple, type Model } from "@mariozechner/pi-ai"; import { describe, expect, it } from "vitest"; import { isTruthyEnvValue } from "../infra/env.js"; import { BYTEPLUS_CODING_BASE_URL, BYTEPLUS_DEFAULT_COST } from "./byteplus-models.js"; +import { + createSingleUserPromptMessage, + extractNonEmptyAssistantText, +} from "./live-test-helpers.js"; const BYTEPLUS_KEY = process.env.BYTEPLUS_API_KEY ?? ""; const BYTEPLUS_CODING_MODEL = process.env.BYTEPLUS_CODING_MODEL?.trim() || "ark-code-latest"; @@ -27,21 +31,12 @@ describeLive("byteplus coding plan live", () => { const res = await completeSimple( model, { - messages: [ - { - role: "user", - content: "Reply with the word ok.", - timestamp: Date.now(), - }, - ], + messages: createSingleUserPromptMessage(), }, { apiKey: BYTEPLUS_KEY, maxTokens: 64 }, ); - const text = res.content - .filter((block) => block.type === "text") - .map((block) => block.text.trim()) - .join(" "); + const text = extractNonEmptyAssistantText(res.content); expect(text.length).toBeGreaterThan(0); }, 30000); }); diff --git a/src/agents/live-test-helpers.ts b/src/agents/live-test-helpers.ts new file mode 100644 index 00000000000..4686a55e797 --- /dev/null +++ b/src/agents/live-test-helpers.ts @@ -0,0 +1,24 @@ +export const LIVE_OK_PROMPT = "Reply with the word ok."; + +export function createSingleUserPromptMessage(content = LIVE_OK_PROMPT) { + return [ + { + role: "user" as const, + content, + timestamp: Date.now(), + }, + ]; +} + +export function extractNonEmptyAssistantText( + content: Array<{ + type?: string; + text?: string; + }>, +) { + return content + .filter((block) => block.type === "text") + .map((block) => block.text?.trim() ?? "") + .filter(Boolean) + .join(" "); +} diff --git a/src/agents/models-config.providers.ollama-autodiscovery.test.ts b/src/agents/models-config.providers.ollama-autodiscovery.test.ts index 910f0e056e6..b878607edea 100644 --- a/src/agents/models-config.providers.ollama-autodiscovery.test.ts +++ b/src/agents/models-config.providers.ollama-autodiscovery.test.ts @@ -32,6 +32,14 @@ describe("Ollama auto-discovery", () => { originalFetch = globalThis.fetch; } + function mockOllamaUnreachable() { + globalThis.fetch = vi + .fn() + .mockRejectedValue( + new Error("connect ECONNREFUSED 127.0.0.1:11434"), + ) as unknown as typeof fetch; + } + it("auto-registers ollama provider when models are discovered locally", async () => { setupDiscoveryEnv(); globalThis.fetch = vi.fn().mockImplementation(async (url: string | URL) => { @@ -62,11 +70,7 @@ describe("Ollama auto-discovery", () => { it("does not warn when Ollama is unreachable and not explicitly configured", async () => { setupDiscoveryEnv(); const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); - globalThis.fetch = vi - .fn() - .mockRejectedValue( - new Error("connect ECONNREFUSED 127.0.0.1:11434"), - ) as unknown as typeof fetch; + mockOllamaUnreachable(); const agentDir = mkdtempSync(join(tmpdir(), "openclaw-test-")); const providers = await resolveImplicitProviders({ agentDir }); @@ -82,11 +86,7 @@ describe("Ollama auto-discovery", () => { it("warns when Ollama is unreachable and explicitly configured", async () => { setupDiscoveryEnv(); const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); - globalThis.fetch = vi - .fn() - .mockRejectedValue( - new Error("connect ECONNREFUSED 127.0.0.1:11434"), - ) as unknown as typeof fetch; + mockOllamaUnreachable(); const agentDir = mkdtempSync(join(tmpdir(), "openclaw-test-")); await resolveImplicitProviders({ diff --git a/src/agents/moonshot.live.test.ts b/src/agents/moonshot.live.test.ts index 455129896bc..216d37c4e67 100644 --- a/src/agents/moonshot.live.test.ts +++ b/src/agents/moonshot.live.test.ts @@ -1,6 +1,10 @@ import { completeSimple, type Model } from "@mariozechner/pi-ai"; import { describe, expect, it } from "vitest"; import { isTruthyEnvValue } from "../infra/env.js"; +import { + createSingleUserPromptMessage, + extractNonEmptyAssistantText, +} from "./live-test-helpers.js"; const MOONSHOT_KEY = process.env.MOONSHOT_API_KEY ?? ""; const MOONSHOT_BASE_URL = process.env.MOONSHOT_BASE_URL?.trim() || "https://api.moonshot.ai/v1"; @@ -27,21 +31,12 @@ describeLive("moonshot live", () => { const res = await completeSimple( model, { - messages: [ - { - role: "user", - content: "Reply with the word ok.", - timestamp: Date.now(), - }, - ], + messages: createSingleUserPromptMessage(), }, { apiKey: MOONSHOT_KEY, maxTokens: 64 }, ); - const text = res.content - .filter((block) => block.type === "text") - .map((block) => block.text.trim()) - .join(" "); + const text = extractNonEmptyAssistantText(res.content); expect(text.length).toBeGreaterThan(0); }, 30000); }); diff --git a/src/agents/openai-ws-connection.test.ts b/src/agents/openai-ws-connection.test.ts index 3122e4f6e3b..13769bd65b7 100644 --- a/src/agents/openai-ws-connection.test.ts +++ b/src/agents/openai-ws-connection.test.ts @@ -171,6 +171,20 @@ function buildManager(opts?: ConstructorParameters errors.push(e)); + return errors; +} + +async function connectManagerAndGetSocket(manager: OpenAIWebSocketManager) { + const connectPromise = manager.connect("sk-test"); + const sock = lastSocket(); + sock.simulateOpen(); + await connectPromise; + return sock; +} + // ───────────────────────────────────────────────────────────────────────────── // Tests // ───────────────────────────────────────────────────────────────────────────── @@ -576,13 +590,8 @@ describe("OpenAIWebSocketManager", () => { describe("error handling", () => { it("emits error event on malformed JSON message", async () => { const manager = buildManager(); - const p = manager.connect("sk-test"); - const sock = lastSocket(); - sock.simulateOpen(); - await p; - - const errors: Error[] = []; - manager.on("error", (e) => errors.push(e)); + const sock = await connectManagerAndGetSocket(manager); + const errors = attachErrorCollector(manager); sock.emit("message", Buffer.from("not valid json{{{{")); @@ -592,13 +601,8 @@ describe("OpenAIWebSocketManager", () => { it("emits error event when message has no type field", async () => { const manager = buildManager(); - const p = manager.connect("sk-test"); - const sock = lastSocket(); - sock.simulateOpen(); - await p; - - const errors: Error[] = []; - manager.on("error", (e) => errors.push(e)); + const sock = await connectManagerAndGetSocket(manager); + const errors = attachErrorCollector(manager); sock.emit("message", Buffer.from(JSON.stringify({ foo: "bar" }))); @@ -611,9 +615,7 @@ describe("OpenAIWebSocketManager", () => { const p = manager.connect("sk-test").catch(() => { /* ignore rejection */ }); - - const errors: Error[] = []; - manager.on("error", (e) => errors.push(e)); + const errors = attachErrorCollector(manager); lastSocket().simulateError(new Error("SSL handshake failed")); await p; @@ -626,9 +628,7 @@ describe("OpenAIWebSocketManager", () => { const p = manager.connect("sk-test").catch(() => { /* ignore rejection */ }); - - const errors: Error[] = []; - manager.on("error", (e) => errors.push(e)); + const errors = attachErrorCollector(manager); // Fire two errors in quick succession β€” previously the second would // be unhandled because .once("error") removed the handler after #1. diff --git a/src/agents/openclaw-tools.subagents.sessions-spawn.allowlist.test.ts b/src/agents/openclaw-tools.subagents.sessions-spawn.allowlist.test.ts index 92047913559..d539921653d 100644 --- a/src/agents/openclaw-tools.subagents.sessions-spawn.allowlist.test.ts +++ b/src/agents/openclaw-tools.subagents.sessions-spawn.allowlist.test.ts @@ -107,6 +107,24 @@ describe("openclaw-tools: subagents (sessions_spawn allowlist)", () => { expect(getChildSessionKey()?.startsWith(`agent:${params.agentId}:subagent:`)).toBe(true); } + async function expectInvalidAgentId(callId: string, agentId: string) { + setSessionsSpawnConfigOverride({ + session: { mainKey: "main", scope: "per-sender" }, + agents: { + list: [{ id: "main", subagents: { allowAgents: ["*"] } }], + }, + }); + const tool = await getSessionsSpawnTool({ + agentSessionKey: "main", + agentChannel: "whatsapp", + }); + const result = await tool.execute(callId, { task: "do thing", agentId }); + const details = result.details as { status?: string; error?: string }; + expect(details.status).toBe("error"); + expect(details.error).toContain("Invalid agentId"); + expect(callGatewayMock).not.toHaveBeenCalled(); + } + beforeEach(() => { resetSessionsSpawnConfigOverride(); resetSubagentRegistryForTests(); @@ -237,45 +255,11 @@ describe("openclaw-tools: subagents (sessions_spawn allowlist)", () => { }); it("rejects agentId containing path separators (#31311)", async () => { - setSessionsSpawnConfigOverride({ - session: { mainKey: "main", scope: "per-sender" }, - agents: { - list: [{ id: "main", subagents: { allowAgents: ["*"] } }], - }, - }); - const tool = await getSessionsSpawnTool({ - agentSessionKey: "main", - agentChannel: "whatsapp", - }); - const result = await tool.execute("call-path", { - task: "do thing", - agentId: "../../../etc/passwd", - }); - const details = result.details as { status?: string; error?: string }; - expect(details.status).toBe("error"); - expect(details.error).toContain("Invalid agentId"); - expect(callGatewayMock).not.toHaveBeenCalled(); + await expectInvalidAgentId("call-path", "../../../etc/passwd"); }); it("rejects agentId exceeding 64 characters (#31311)", async () => { - setSessionsSpawnConfigOverride({ - session: { mainKey: "main", scope: "per-sender" }, - agents: { - list: [{ id: "main", subagents: { allowAgents: ["*"] } }], - }, - }); - const tool = await getSessionsSpawnTool({ - agentSessionKey: "main", - agentChannel: "whatsapp", - }); - const result = await tool.execute("call-long", { - task: "do thing", - agentId: "a".repeat(65), - }); - const details = result.details as { status?: string; error?: string }; - expect(details.status).toBe("error"); - expect(details.error).toContain("Invalid agentId"); - expect(callGatewayMock).not.toHaveBeenCalled(); + await expectInvalidAgentId("call-long", "a".repeat(65)); }); it("accepts well-formed agentId with hyphens and underscores (#31311)", async () => { diff --git a/src/agents/path-policy.ts b/src/agents/path-policy.ts index be0e1ffd7ed..e289ee406cb 100644 --- a/src/agents/path-policy.ts +++ b/src/agents/path-policy.ts @@ -9,6 +9,16 @@ type RelativePathOptions = { includeRootInError?: boolean; }; +function throwPathEscapesBoundary(params: { + options?: RelativePathOptions; + rootResolved: string; + candidate: string; +}): never { + const boundary = params.options?.boundaryLabel ?? "workspace root"; + const suffix = params.options?.includeRootInError ? ` (${params.rootResolved})` : ""; + throw new Error(`Path escapes ${boundary}${suffix}: ${params.candidate}`); +} + function toRelativePathUnderRoot(params: { root: string; candidate: string; @@ -29,14 +39,18 @@ function toRelativePathUnderRoot(params: { if (params.options?.allowRoot) { return ""; } - const boundary = params.options?.boundaryLabel ?? "workspace root"; - const suffix = params.options?.includeRootInError ? ` (${rootResolved})` : ""; - throw new Error(`Path escapes ${boundary}${suffix}: ${params.candidate}`); + throwPathEscapesBoundary({ + options: params.options, + rootResolved, + candidate: params.candidate, + }); } if (relative.startsWith("..") || path.win32.isAbsolute(relative)) { - const boundary = params.options?.boundaryLabel ?? "workspace root"; - const suffix = params.options?.includeRootInError ? ` (${rootResolved})` : ""; - throw new Error(`Path escapes ${boundary}${suffix}: ${params.candidate}`); + throwPathEscapesBoundary({ + options: params.options, + rootResolved, + candidate: params.candidate, + }); } return relative; } @@ -48,14 +62,18 @@ function toRelativePathUnderRoot(params: { if (params.options?.allowRoot) { return ""; } - const boundary = params.options?.boundaryLabel ?? "workspace root"; - const suffix = params.options?.includeRootInError ? ` (${rootResolved})` : ""; - throw new Error(`Path escapes ${boundary}${suffix}: ${params.candidate}`); + throwPathEscapesBoundary({ + options: params.options, + rootResolved, + candidate: params.candidate, + }); } if (relative.startsWith("..") || path.isAbsolute(relative)) { - const boundary = params.options?.boundaryLabel ?? "workspace root"; - const suffix = params.options?.includeRootInError ? ` (${rootResolved})` : ""; - throw new Error(`Path escapes ${boundary}${suffix}: ${params.candidate}`); + throwPathEscapesBoundary({ + options: params.options, + rootResolved, + candidate: params.candidate, + }); } return relative; } diff --git a/src/agents/pi-embedded-block-chunker.test.ts b/src/agents/pi-embedded-block-chunker.test.ts index fe6614d2104..0b6c858ef95 100644 --- a/src/agents/pi-embedded-block-chunker.test.ts +++ b/src/agents/pi-embedded-block-chunker.test.ts @@ -1,6 +1,29 @@ import { describe, expect, it } from "vitest"; import { EmbeddedBlockChunker } from "./pi-embedded-block-chunker.js"; +function createFlushOnParagraphChunker(params: { minChars: number; maxChars: number }) { + return new EmbeddedBlockChunker({ + minChars: params.minChars, + maxChars: params.maxChars, + breakPreference: "paragraph", + flushOnParagraph: true, + }); +} + +function drainChunks(chunker: EmbeddedBlockChunker) { + const chunks: string[] = []; + chunker.drain({ force: false, emit: (chunk) => chunks.push(chunk) }); + return chunks; +} + +function expectFlushAtFirstParagraphBreak(text: string) { + const chunker = createFlushOnParagraphChunker({ minChars: 100, maxChars: 200 }); + chunker.append(text); + const chunks = drainChunks(chunker); + expect(chunks).toEqual(["First paragraph."]); + expect(chunker.bufferedText).toBe("Second paragraph."); +} + describe("EmbeddedBlockChunker", () => { it("breaks at paragraph boundary right after fence close", () => { const chunker = new EmbeddedBlockChunker({ @@ -21,8 +44,7 @@ describe("EmbeddedBlockChunker", () => { chunker.append(text); - const chunks: string[] = []; - chunker.drain({ force: false, emit: (chunk) => chunks.push(chunk) }); + const chunks = drainChunks(chunker); expect(chunks.length).toBe(1); expect(chunks[0]).toContain("console.log"); @@ -32,37 +54,11 @@ describe("EmbeddedBlockChunker", () => { }); it("flushes paragraph boundaries before minChars when flushOnParagraph is set", () => { - const chunker = new EmbeddedBlockChunker({ - minChars: 100, - maxChars: 200, - breakPreference: "paragraph", - flushOnParagraph: true, - }); - - chunker.append("First paragraph.\n\nSecond paragraph."); - - const chunks: string[] = []; - chunker.drain({ force: false, emit: (chunk) => chunks.push(chunk) }); - - expect(chunks).toEqual(["First paragraph."]); - expect(chunker.bufferedText).toBe("Second paragraph."); + expectFlushAtFirstParagraphBreak("First paragraph.\n\nSecond paragraph."); }); it("treats blank lines with whitespace as paragraph boundaries when flushOnParagraph is set", () => { - const chunker = new EmbeddedBlockChunker({ - minChars: 100, - maxChars: 200, - breakPreference: "paragraph", - flushOnParagraph: true, - }); - - chunker.append("First paragraph.\n \nSecond paragraph."); - - const chunks: string[] = []; - chunker.drain({ force: false, emit: (chunk) => chunks.push(chunk) }); - - expect(chunks).toEqual(["First paragraph."]); - expect(chunker.bufferedText).toBe("Second paragraph."); + expectFlushAtFirstParagraphBreak("First paragraph.\n \nSecond paragraph."); }); it("falls back to maxChars when flushOnParagraph is set and no paragraph break exists", () => { @@ -75,8 +71,7 @@ describe("EmbeddedBlockChunker", () => { chunker.append("abcdefghijKLMNOP"); - const chunks: string[] = []; - chunker.drain({ force: false, emit: (chunk) => chunks.push(chunk) }); + const chunks = drainChunks(chunker); expect(chunks).toEqual(["abcdefghij"]); expect(chunker.bufferedText).toBe("KLMNOP"); @@ -92,8 +87,7 @@ describe("EmbeddedBlockChunker", () => { chunker.append("abcdefghijk\n\nRest"); - const chunks: string[] = []; - chunker.drain({ force: false, emit: (chunk) => chunks.push(chunk) }); + const chunks = drainChunks(chunker); expect(chunks.every((chunk) => chunk.length <= 10)).toBe(true); expect(chunks).toEqual(["abcdefghij", "k"]); @@ -121,8 +115,7 @@ describe("EmbeddedBlockChunker", () => { chunker.append(text); - const chunks: string[] = []; - chunker.drain({ force: false, emit: (chunk) => chunks.push(chunk) }); + const chunks = drainChunks(chunker); expect(chunks).toEqual(["Intro\n```js\nconst a = 1;\n\nconst b = 2;\n```"]); expect(chunker.bufferedText).toBe("After fence"); diff --git a/src/agents/pi-embedded-runner/google.ts b/src/agents/pi-embedded-runner/google.ts index 9657c26686d..094aa9142c3 100644 --- a/src/agents/pi-embedded-runner/google.ts +++ b/src/agents/pi-embedded-runner/google.ts @@ -200,7 +200,7 @@ function stripStaleAssistantUsageBeforeLatestCompaction(messages: AgentMessage[] return touched ? out : messages; } -function findUnsupportedSchemaKeywords(schema: unknown, path: string): string[] { +export function findUnsupportedSchemaKeywords(schema: unknown, path: string): string[] { if (!schema || typeof schema !== "object") { return []; } diff --git a/src/agents/pi-extensions/compaction-safeguard.test.ts b/src/agents/pi-extensions/compaction-safeguard.test.ts index 00140f7b724..621100a5d6e 100644 --- a/src/agents/pi-extensions/compaction-safeguard.test.ts +++ b/src/agents/pi-extensions/compaction-safeguard.test.ts @@ -482,40 +482,39 @@ describe("compaction-safeguard double-compaction guard", () => { }); }); +async function expectWorkspaceSummaryEmptyForAgentsAlias( + createAlias: (outsidePath: string, agentsPath: string) => void, +) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-compaction-summary-")); + const prevCwd = process.cwd(); + try { + const outside = path.join(root, "outside-secret.txt"); + fs.writeFileSync(outside, "secret"); + createAlias(outside, path.join(root, "AGENTS.md")); + process.chdir(root); + await expect(readWorkspaceContextForSummary()).resolves.toBe(""); + } finally { + process.chdir(prevCwd); + fs.rmSync(root, { recursive: true, force: true }); + } +} + describe("readWorkspaceContextForSummary", () => { it.runIf(process.platform !== "win32")( "returns empty when AGENTS.md is a symlink escape", async () => { - const root = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-compaction-summary-")); - const prevCwd = process.cwd(); - try { - const outside = path.join(root, "outside-secret.txt"); - fs.writeFileSync(outside, "secret"); - fs.symlinkSync(outside, path.join(root, "AGENTS.md")); - process.chdir(root); - await expect(readWorkspaceContextForSummary()).resolves.toBe(""); - } finally { - process.chdir(prevCwd); - fs.rmSync(root, { recursive: true, force: true }); - } + await expectWorkspaceSummaryEmptyForAgentsAlias((outside, agentsPath) => { + fs.symlinkSync(outside, agentsPath); + }); }, ); it.runIf(process.platform !== "win32")( "returns empty when AGENTS.md is a hardlink alias", async () => { - const root = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-compaction-summary-")); - const prevCwd = process.cwd(); - try { - const outside = path.join(root, "outside-secret.txt"); - fs.writeFileSync(outside, "secret"); - fs.linkSync(outside, path.join(root, "AGENTS.md")); - process.chdir(root); - await expect(readWorkspaceContextForSummary()).resolves.toBe(""); - } finally { - process.chdir(prevCwd); - fs.rmSync(root, { recursive: true, force: true }); - } + await expectWorkspaceSummaryEmptyForAgentsAlias((outside, agentsPath) => { + fs.linkSync(outside, agentsPath); + }); }, ); }); diff --git a/src/agents/pi-tools-agent-config.test.ts b/src/agents/pi-tools-agent-config.test.ts index cf31823990b..e24186e0b30 100644 --- a/src/agents/pi-tools-agent-config.test.ts +++ b/src/agents/pi-tools-agent-config.test.ts @@ -28,6 +28,16 @@ describe("Agent-specific tool filtering", () => { stat: async () => null, }; + function expectReadOnlyToolSet(toolNames: string[], extraDenied: string[] = []) { + expect(toolNames).toContain("read"); + expect(toolNames).not.toContain("exec"); + expect(toolNames).not.toContain("write"); + expect(toolNames).not.toContain("apply_patch"); + for (const toolName of extraDenied) { + expect(toolNames).not.toContain(toolName); + } + } + async function withApplyPatchEscapeCase( opts: { workspaceOnly?: boolean }, run: (params: { @@ -250,12 +260,10 @@ describe("Agent-specific tool filtering", () => { agentDir: "/tmp/agent-restricted", }); - const toolNames = tools.map((t) => t.name); - expect(toolNames).toContain("read"); - expect(toolNames).not.toContain("exec"); - expect(toolNames).not.toContain("write"); - expect(toolNames).not.toContain("apply_patch"); - expect(toolNames).not.toContain("edit"); + expectReadOnlyToolSet( + tools.map((t) => t.name), + ["edit"], + ); }); it("should apply provider-specific tool policy", () => { @@ -279,11 +287,7 @@ describe("Agent-specific tool filtering", () => { modelId: "claude-opus-4-6-thinking", }); - const toolNames = tools.map((t) => t.name); - expect(toolNames).toContain("read"); - expect(toolNames).not.toContain("exec"); - expect(toolNames).not.toContain("write"); - expect(toolNames).not.toContain("apply_patch"); + expectReadOnlyToolSet(tools.map((t) => t.name)); }); it("should apply provider-specific tool profile overrides", () => { diff --git a/src/agents/pi-tools.create-openclaw-coding-tools.adds-claude-style-aliases-schemas-without-dropping.test.ts b/src/agents/pi-tools.create-openclaw-coding-tools.adds-claude-style-aliases-schemas-without-dropping.test.ts index 22d68f15ff8..5a7cb72ccb7 100644 --- a/src/agents/pi-tools.create-openclaw-coding-tools.adds-claude-style-aliases-schemas-without-dropping.test.ts +++ b/src/agents/pi-tools.create-openclaw-coding-tools.adds-claude-style-aliases-schemas-without-dropping.test.ts @@ -6,6 +6,7 @@ import { Type } from "@sinclair/typebox"; import { describe, expect, it, vi } from "vitest"; import "./test-helpers/fast-coding-tools.js"; import { createOpenClawTools } from "./openclaw-tools.js"; +import { findUnsupportedSchemaKeywords } from "./pi-embedded-runner/google.js"; import { __testing, createOpenClawCodingTools } from "./pi-tools.js"; import { createOpenClawReadTool, createSandboxedReadTool } from "./pi-tools.read.js"; import { createHostSandboxFsBridge } from "./test-helpers/host-sandbox-fs-bridge.js"; @@ -444,75 +445,12 @@ describe("createOpenClawCodingTools", () => { expect(names.has("read")).toBe(false); }); it("removes unsupported JSON Schema keywords for Cloud Code Assist API compatibility", () => { - // Helper to recursively check schema for unsupported keywords - const unsupportedKeywords = new Set([ - "patternProperties", - "additionalProperties", - "$schema", - "$id", - "$ref", - "$defs", - "definitions", - "examples", - "minLength", - "maxLength", - "minimum", - "maximum", - "multipleOf", - "pattern", - "format", - "minItems", - "maxItems", - "uniqueItems", - "minProperties", - "maxProperties", - ]); - - const findUnsupportedKeywords = (schema: unknown, path: string): string[] => { - const found: string[] = []; - if (!schema || typeof schema !== "object") { - return found; - } - if (Array.isArray(schema)) { - schema.forEach((item, i) => { - found.push(...findUnsupportedKeywords(item, `${path}[${i}]`)); - }); - return found; - } - - const record = schema as Record; - const properties = - record.properties && - typeof record.properties === "object" && - !Array.isArray(record.properties) - ? (record.properties as Record) - : undefined; - if (properties) { - for (const [key, value] of Object.entries(properties)) { - found.push(...findUnsupportedKeywords(value, `${path}.properties.${key}`)); - } - } - - for (const [key, value] of Object.entries(record)) { - if (key === "properties") { - continue; - } - if (unsupportedKeywords.has(key)) { - found.push(`${path}.${key}`); - } - if (value && typeof value === "object") { - found.push(...findUnsupportedKeywords(value, `${path}.${key}`)); - } - } - return found; - }; - const googleTools = createOpenClawCodingTools({ modelProvider: "google", senderIsOwner: true, }); for (const tool of googleTools) { - const violations = findUnsupportedKeywords(tool.parameters, `${tool.name}.parameters`); + const violations = findUnsupportedSchemaKeywords(tool.parameters, `${tool.name}.parameters`); expect(violations).toEqual([]); } }); diff --git a/src/agents/pi-tools.read.ts b/src/agents/pi-tools.read.ts index f0fa6d2e2e3..1c5ce11c0f0 100644 --- a/src/agents/pi-tools.read.ts +++ b/src/agents/pi-tools.read.ts @@ -763,6 +763,12 @@ function createSandboxEditOperations(params: SandboxToolParams) { } as const; } +async function writeHostFile(absolutePath: string, content: string) { + const resolved = path.resolve(absolutePath); + await fs.mkdir(path.dirname(resolved), { recursive: true }); + await fs.writeFile(resolved, content, "utf-8"); +} + function createHostWriteOperations(root: string, options?: { workspaceOnly?: boolean }) { const workspaceOnly = options?.workspaceOnly ?? false; @@ -773,12 +779,7 @@ function createHostWriteOperations(root: string, options?: { workspaceOnly?: boo const resolved = path.resolve(dir); await fs.mkdir(resolved, { recursive: true }); }, - writeFile: async (absolutePath: string, content: string) => { - const resolved = path.resolve(absolutePath); - const dir = path.dirname(resolved); - await fs.mkdir(dir, { recursive: true }); - await fs.writeFile(resolved, content, "utf-8"); - }, + writeFile: writeHostFile, } as const; } @@ -812,12 +813,7 @@ function createHostEditOperations(root: string, options?: { workspaceOnly?: bool const resolved = path.resolve(absolutePath); return await fs.readFile(resolved); }, - writeFile: async (absolutePath: string, content: string) => { - const resolved = path.resolve(absolutePath); - const dir = path.dirname(resolved); - await fs.mkdir(dir, { recursive: true }); - await fs.writeFile(resolved, content, "utf-8"); - }, + writeFile: writeHostFile, access: async (absolutePath: string) => { const resolved = path.resolve(absolutePath); await fs.access(resolved); diff --git a/src/agents/pi-tools.workspace-paths.test.ts b/src/agents/pi-tools.workspace-paths.test.ts index 4efa494555e..af17a896609 100644 --- a/src/agents/pi-tools.workspace-paths.test.ts +++ b/src/agents/pi-tools.workspace-paths.test.ts @@ -21,6 +21,35 @@ async function withTempDir(prefix: string, fn: (dir: string) => Promise) { } } +function createExecTool(workspaceDir: string) { + const tools = createOpenClawCodingTools({ + workspaceDir, + exec: { host: "gateway", ask: "off", security: "full" }, + }); + const execTool = tools.find((tool) => tool.name === "exec"); + expect(execTool).toBeDefined(); + return execTool; +} + +async function expectExecCwdResolvesTo( + execTool: ReturnType, + callId: string, + params: { command: string; workdir?: string }, + expectedDir: string, +) { + const result = await execTool?.execute(callId, params); + const cwd = + result?.details && typeof result.details === "object" && "cwd" in result.details + ? (result.details as { cwd?: string }).cwd + : undefined; + expect(cwd).toBeTruthy(); + const [resolvedOutput, resolvedExpected] = await Promise.all([ + fs.realpath(String(cwd)), + fs.realpath(expectedDir), + ]); + expect(resolvedOutput).toBe(resolvedExpected); +} + describe("workspace path resolution", () => { it("resolves relative read/write/edit paths against workspaceDir even after cwd changes", async () => { await withTempDir("openclaw-ws-", async (workspaceDir) => { @@ -88,53 +117,21 @@ describe("workspace path resolution", () => { it("defaults exec cwd to workspaceDir when workdir is omitted", async () => { await withTempDir("openclaw-ws-", async (workspaceDir) => { - const tools = createOpenClawCodingTools({ - workspaceDir, - exec: { host: "gateway", ask: "off", security: "full" }, - }); - const execTool = tools.find((tool) => tool.name === "exec"); - expect(execTool).toBeDefined(); - - const result = await execTool?.execute("ws-exec", { - command: "echo ok", - }); - const cwd = - result?.details && typeof result.details === "object" && "cwd" in result.details - ? (result.details as { cwd?: string }).cwd - : undefined; - expect(cwd).toBeTruthy(); - const [resolvedOutput, resolvedWorkspace] = await Promise.all([ - fs.realpath(String(cwd)), - fs.realpath(workspaceDir), - ]); - expect(resolvedOutput).toBe(resolvedWorkspace); + const execTool = createExecTool(workspaceDir); + await expectExecCwdResolvesTo(execTool, "ws-exec", { command: "echo ok" }, workspaceDir); }); }); it("lets exec workdir override the workspace default", async () => { await withTempDir("openclaw-ws-", async (workspaceDir) => { await withTempDir("openclaw-override-", async (overrideDir) => { - const tools = createOpenClawCodingTools({ - workspaceDir, - exec: { host: "gateway", ask: "off", security: "full" }, - }); - const execTool = tools.find((tool) => tool.name === "exec"); - expect(execTool).toBeDefined(); - - const result = await execTool?.execute("ws-exec-override", { - command: "echo ok", - workdir: overrideDir, - }); - const cwd = - result?.details && typeof result.details === "object" && "cwd" in result.details - ? (result.details as { cwd?: string }).cwd - : undefined; - expect(cwd).toBeTruthy(); - const [resolvedOutput, resolvedOverride] = await Promise.all([ - fs.realpath(String(cwd)), - fs.realpath(overrideDir), - ]); - expect(resolvedOutput).toBe(resolvedOverride); + const execTool = createExecTool(workspaceDir); + await expectExecCwdResolvesTo( + execTool, + "ws-exec-override", + { command: "echo ok", workdir: overrideDir }, + overrideDir, + ); }); }); }); diff --git a/src/agents/session-tool-result-guard.test.ts b/src/agents/session-tool-result-guard.test.ts index 0499ca79a48..49fbd7a6d81 100644 --- a/src/agents/session-tool-result-guard.test.ts +++ b/src/agents/session-tool-result-guard.test.ts @@ -26,6 +26,31 @@ function appendToolResultText(sm: SessionManager, text: string) { ); } +function appendAssistantToolCall( + sm: SessionManager, + params: { id: string; name: string; withArguments?: boolean }, +) { + const toolCall: { + type: "toolCall"; + id: string; + name: string; + arguments?: Record; + } = { + type: "toolCall", + id: params.id, + name: params.name, + }; + if (params.withArguments !== false) { + toolCall.arguments = {}; + } + sm.appendMessage( + asAppendMessage({ + role: "assistant", + content: [toolCall], + }), + ); +} + function getPersistedMessages(sm: SessionManager): AgentMessage[] { return sm .getEntries() @@ -273,19 +298,8 @@ describe("installSessionToolResultGuard", () => { const sm = SessionManager.inMemory(); installSessionToolResultGuard(sm); - sm.appendMessage( - asAppendMessage({ - role: "assistant", - content: [{ type: "toolCall", id: "call_1", name: "read", arguments: {} }], - }), - ); - - sm.appendMessage( - asAppendMessage({ - role: "assistant", - content: [{ type: "toolCall", id: "call_2", name: "read" }], - }), - ); + appendAssistantToolCall(sm, { id: "call_1", name: "read" }); + appendAssistantToolCall(sm, { id: "call_2", name: "read", withArguments: false }); expectPersistedRoles(sm, ["assistant", "toolResult"]); }); @@ -297,19 +311,8 @@ describe("installSessionToolResultGuard", () => { allowedToolNames: ["read"], }); - sm.appendMessage( - asAppendMessage({ - role: "assistant", - content: [{ type: "toolCall", id: "call_1", name: "read", arguments: {} }], - }), - ); - - sm.appendMessage( - asAppendMessage({ - role: "assistant", - content: [{ type: "toolCall", id: "call_2", name: "write", arguments: {} }], - }), - ); + appendAssistantToolCall(sm, { id: "call_1", name: "read" }); + appendAssistantToolCall(sm, { id: "call_2", name: "write" }); expectPersistedRoles(sm, ["assistant"]); expect(guard.getPendingIds()).toEqual([]); diff --git a/src/agents/sessions-spawn-hooks.test.ts b/src/agents/sessions-spawn-hooks.test.ts index 0a8c82ca60a..41f87dd33bd 100644 --- a/src/agents/sessions-spawn-hooks.test.ts +++ b/src/agents/sessions-spawn-hooks.test.ts @@ -65,6 +65,28 @@ function mockAgentStartFailure() { }); } +async function runSessionThreadSpawnAndGetError(params: { + toolCallId: string; + spawningResult: { status: "error"; error: string } | { status: "ok"; threadBindingReady: false }; +}): Promise<{ error?: string; childSessionKey?: string }> { + hookRunnerMocks.runSubagentSpawning.mockResolvedValueOnce(params.spawningResult); + const tool = await getSessionsSpawnTool({ + agentSessionKey: "main", + agentChannel: "discord", + agentAccountId: "work", + agentTo: "channel:123", + }); + + const result = await tool.execute(params.toolCallId, { + task: "do thing", + runTimeoutSeconds: 1, + thread: true, + mode: "session", + }); + expect(result.details).toMatchObject({ status: "error" }); + return result.details as { error?: string; childSessionKey?: string }; +} + describe("sessions_spawn subagent lifecycle hooks", () => { beforeEach(() => { resetSubagentRegistryForTests(); @@ -214,26 +236,13 @@ describe("sessions_spawn subagent lifecycle hooks", () => { }); it("returns error when thread binding cannot be created", async () => { - hookRunnerMocks.runSubagentSpawning.mockResolvedValueOnce({ - status: "error", - error: "Unable to create or bind a Discord thread for this subagent session.", + const details = await runSessionThreadSpawnAndGetError({ + toolCallId: "call4", + spawningResult: { + status: "error", + error: "Unable to create or bind a Discord thread for this subagent session.", + }, }); - const tool = await getSessionsSpawnTool({ - agentSessionKey: "main", - agentChannel: "discord", - agentAccountId: "work", - agentTo: "channel:123", - }); - - const result = await tool.execute("call4", { - task: "do thing", - runTimeoutSeconds: 1, - thread: true, - mode: "session", - }); - - expect(result.details).toMatchObject({ status: "error" }); - const details = result.details as { error?: string; childSessionKey?: string }; expect(details.error).toMatch(/thread/i); expect(hookRunnerMocks.runSubagentSpawned).not.toHaveBeenCalled(); expectSessionsDeleteWithoutAgentStart(); @@ -245,26 +254,13 @@ describe("sessions_spawn subagent lifecycle hooks", () => { }); it("returns error when thread binding is not marked ready", async () => { - hookRunnerMocks.runSubagentSpawning.mockResolvedValueOnce({ - status: "ok", - threadBindingReady: false, + const details = await runSessionThreadSpawnAndGetError({ + toolCallId: "call4b", + spawningResult: { + status: "ok", + threadBindingReady: false, + }, }); - const tool = await getSessionsSpawnTool({ - agentSessionKey: "main", - agentChannel: "discord", - agentAccountId: "work", - agentTo: "channel:123", - }); - - const result = await tool.execute("call4b", { - task: "do thing", - runTimeoutSeconds: 1, - thread: true, - mode: "session", - }); - - expect(result.details).toMatchObject({ status: "error" }); - const details = result.details as { error?: string; childSessionKey?: string }; expect(details.error).toMatch(/unable to create or bind a thread/i); expect(hookRunnerMocks.runSubagentSpawned).not.toHaveBeenCalled(); expectSessionsDeleteWithoutAgentStart(); diff --git a/src/agents/subagent-announce-dispatch.test.ts b/src/agents/subagent-announce-dispatch.test.ts index fcc2f992e2b..384e20615b8 100644 --- a/src/agents/subagent-announce-dispatch.test.ts +++ b/src/agents/subagent-announce-dispatch.test.ts @@ -28,15 +28,25 @@ describe("mapQueueOutcomeToDeliveryResult", () => { }); describe("runSubagentAnnounceDispatch", () => { - it("uses queue-first ordering for non-completion mode", async () => { - const queue = vi.fn(async () => "none" as const); - const direct = vi.fn(async () => ({ delivered: true, path: "direct" as const })); - + async function runNonCompletionDispatch(params: { + queueOutcome: "none" | "queued" | "steered"; + directDelivered?: boolean; + }) { + const queue = vi.fn(async () => params.queueOutcome); + const direct = vi.fn(async () => ({ + delivered: params.directDelivered ?? true, + path: "direct" as const, + })); const result = await runSubagentAnnounceDispatch({ expectsCompletionMessage: false, queue, direct, }); + return { queue, direct, result }; + } + + it("uses queue-first ordering for non-completion mode", async () => { + const { queue, direct, result } = await runNonCompletionDispatch({ queueOutcome: "none" }); expect(queue).toHaveBeenCalledTimes(1); expect(direct).toHaveBeenCalledTimes(1); @@ -49,14 +59,7 @@ describe("runSubagentAnnounceDispatch", () => { }); it("short-circuits direct send when non-completion queue delivers", async () => { - const queue = vi.fn(async () => "queued" as const); - const direct = vi.fn(async () => ({ delivered: true, path: "direct" as const })); - - const result = await runSubagentAnnounceDispatch({ - expectsCompletionMessage: false, - queue, - direct, - }); + const { queue, direct, result } = await runNonCompletionDispatch({ queueOutcome: "queued" }); expect(queue).toHaveBeenCalledTimes(1); expect(direct).not.toHaveBeenCalled(); diff --git a/src/agents/subagent-registry.persistence.test.ts b/src/agents/subagent-registry.persistence.test.ts index 1c3db23672f..468de55953c 100644 --- a/src/agents/subagent-registry.persistence.test.ts +++ b/src/agents/subagent-registry.persistence.test.ts @@ -115,6 +115,16 @@ describe("subagent registry persistence", () => { return registryPath; }; + const readPersistedRun = async ( + registryPath: string, + runId: string, + ): Promise => { + const parsed = JSON.parse(await fs.readFile(registryPath, "utf8")) as { + runs?: Record; + }; + return parsed.runs?.[runId] as T | undefined; + }; + const createPersistedEndedRun = (params: { runId: string; childSessionKey: string; @@ -316,11 +326,12 @@ describe("subagent registry persistence", () => { await restartRegistryAndFlush(); expect(announceSpy).toHaveBeenCalledTimes(1); - const afterFirst = JSON.parse(await fs.readFile(registryPath, "utf8")) as { - runs: Record; - }; - expect(afterFirst.runs["run-3"].cleanupHandled).toBe(false); - expect(afterFirst.runs["run-3"].cleanupCompletedAt).toBeUndefined(); + const afterFirst = await readPersistedRun<{ + cleanupHandled?: boolean; + cleanupCompletedAt?: number; + }>(registryPath, "run-3"); + expect(afterFirst?.cleanupHandled).toBe(false); + expect(afterFirst?.cleanupCompletedAt).toBeUndefined(); announceSpy.mockResolvedValueOnce(true); await restartRegistryAndFlush(); @@ -345,10 +356,8 @@ describe("subagent registry persistence", () => { await restartRegistryAndFlush(); expect(announceSpy).toHaveBeenCalledTimes(1); - const afterFirst = JSON.parse(await fs.readFile(registryPath, "utf8")) as { - runs: Record; - }; - expect(afterFirst.runs["run-4"]?.cleanupHandled).toBe(false); + const afterFirst = await readPersistedRun<{ cleanupHandled?: boolean }>(registryPath, "run-4"); + expect(afterFirst?.cleanupHandled).toBe(false); announceSpy.mockResolvedValueOnce(true); await restartRegistryAndFlush(); diff --git a/src/agents/tools/cron-tool.test.ts b/src/agents/tools/cron-tool.test.ts index 6d615b47945..28ab28626da 100644 --- a/src/agents/tools/cron-tool.test.ts +++ b/src/agents/tools/cron-tool.test.ts @@ -28,6 +28,27 @@ describe("cron tool", () => { return params?.payload?.text ?? ""; } + function expectSingleGatewayCallMethod(method: string) { + expect(callGatewayMock).toHaveBeenCalledTimes(1); + const call = readGatewayCall(0); + expect(call.method).toBe(method); + return call.params; + } + + function buildReminderAgentTurnJob(overrides: Record = {}): { + name: string; + schedule: { at: string }; + payload: { kind: "agentTurn"; message: string }; + delivery?: { mode: string; to?: string }; + } { + return { + name: "reminder", + schedule: { at: new Date(123).toISOString() }, + payload: { kind: "agentTurn", message: "hello" }, + ...overrides, + }; + } + async function executeAddAndReadDelivery(params: { callId: string; agentSessionKey: string; @@ -37,9 +58,7 @@ describe("cron tool", () => { await tool.execute(params.callId, { action: "add", job: { - name: "reminder", - schedule: { at: new Date(123).toISOString() }, - payload: { kind: "agentTurn", message: "hello" }, + ...buildReminderAgentTurnJob(), ...(params.delivery !== undefined ? { delivery: params.delivery } : {}), }, }); @@ -114,13 +133,8 @@ describe("cron tool", () => { const tool = createCronTool(); await tool.execute("call1", args); - expect(callGatewayMock).toHaveBeenCalledTimes(1); - const call = callGatewayMock.mock.calls[0]?.[0] as { - method?: string; - params?: unknown; - }; - expect(call.method).toBe(`cron.${action}`); - expect(call.params).toEqual(expectedParams); + const params = expectSingleGatewayCallMethod(`cron.${action}`); + expect(params).toEqual(expectedParams); }); it("prefers jobId over id when both are provided", async () => { @@ -131,10 +145,7 @@ describe("cron tool", () => { id: "job-legacy", }); - const call = callGatewayMock.mock.calls[0]?.[0] as { - params?: unknown; - }; - expect(call?.params).toEqual({ id: "job-primary", mode: "force" }); + expect(readGatewayCall().params).toEqual({ id: "job-primary", mode: "force" }); }); it("supports due-only run mode", async () => { @@ -145,10 +156,7 @@ describe("cron tool", () => { runMode: "due", }); - const call = callGatewayMock.mock.calls[0]?.[0] as { - params?: unknown; - }; - expect(call?.params).toEqual({ id: "job-due", mode: "due" }); + expect(readGatewayCall().params).toEqual({ id: "job-due", mode: "due" }); }); it("normalizes cron.add job payloads", async () => { @@ -164,13 +172,8 @@ describe("cron tool", () => { }, }); - expect(callGatewayMock).toHaveBeenCalledTimes(1); - const call = callGatewayMock.mock.calls[0]?.[0] as { - method?: string; - params?: unknown; - }; - expect(call.method).toBe("cron.add"); - expect(call.params).toEqual({ + const params = expectSingleGatewayCallMethod("cron.add"); + expect(params).toEqual({ name: "wake-up", enabled: true, deleteAfterRun: true, @@ -367,15 +370,12 @@ describe("cron tool", () => { payload: { kind: "agentTurn", message: "do stuff" }, }); - expect(callGatewayMock).toHaveBeenCalledTimes(1); - const call = callGatewayMock.mock.calls[0]?.[0] as { - method?: string; - params?: { name?: string; sessionTarget?: string; payload?: { kind?: string } }; - }; - expect(call.method).toBe("cron.add"); - expect(call.params?.name).toBe("flat-job"); - expect(call.params?.sessionTarget).toBe("isolated"); - expect(call.params?.payload?.kind).toBe("agentTurn"); + const params = expectSingleGatewayCallMethod("cron.add") as + | { name?: string; sessionTarget?: string; payload?: { kind?: string } } + | undefined; + expect(params?.name).toBe("flat-job"); + expect(params?.sessionTarget).toBe("isolated"); + expect(params?.payload?.kind).toBe("agentTurn"); }); it("recovers flat params when job is empty object", async () => { @@ -391,15 +391,12 @@ describe("cron tool", () => { payload: { kind: "systemEvent", text: "wake up" }, }); - expect(callGatewayMock).toHaveBeenCalledTimes(1); - const call = callGatewayMock.mock.calls[0]?.[0] as { - method?: string; - params?: { name?: string; sessionTarget?: string; payload?: { text?: string } }; - }; - expect(call.method).toBe("cron.add"); - expect(call.params?.name).toBe("empty-job"); - expect(call.params?.sessionTarget).toBe("main"); - expect(call.params?.payload?.text).toBe("wake up"); + const params = expectSingleGatewayCallMethod("cron.add") as + | { name?: string; sessionTarget?: string; payload?: { text?: string } } + | undefined; + expect(params?.name).toBe("empty-job"); + expect(params?.sessionTarget).toBe("main"); + expect(params?.payload?.text).toBe("wake up"); }); it("recovers flat message shorthand as agentTurn payload", async () => { @@ -412,16 +409,13 @@ describe("cron tool", () => { message: "do stuff", }); - expect(callGatewayMock).toHaveBeenCalledTimes(1); - const call = callGatewayMock.mock.calls[0]?.[0] as { - method?: string; - params?: { payload?: { kind?: string; message?: string }; sessionTarget?: string }; - }; - expect(call.method).toBe("cron.add"); + const params = expectSingleGatewayCallMethod("cron.add") as + | { payload?: { kind?: string; message?: string }; sessionTarget?: string } + | undefined; // normalizeCronJobCreate infers agentTurn from message and isolated from agentTurn - expect(call.params?.payload?.kind).toBe("agentTurn"); - expect(call.params?.payload?.message).toBe("do stuff"); - expect(call.params?.sessionTarget).toBe("isolated"); + expect(params?.payload?.kind).toBe("agentTurn"); + expect(params?.payload?.message).toBe("do stuff"); + expect(params?.sessionTarget).toBe("isolated"); }); it("does not recover flat params when no meaningful job field is present", async () => { @@ -486,9 +480,7 @@ describe("cron tool", () => { tool.execute("call-webhook-missing", { action: "add", job: { - name: "reminder", - schedule: { at: new Date(123).toISOString() }, - payload: { kind: "agentTurn", message: "hello" }, + ...buildReminderAgentTurnJob(), delivery: { mode: "webhook" }, }, }), @@ -503,9 +495,7 @@ describe("cron tool", () => { tool.execute("call-webhook-invalid", { action: "add", job: { - name: "reminder", - schedule: { at: new Date(123).toISOString() }, - payload: { kind: "agentTurn", message: "hello" }, + ...buildReminderAgentTurnJob(), delivery: { mode: "webhook", to: "ftp://example.invalid/cron-finished" }, }, }), @@ -524,15 +514,12 @@ describe("cron tool", () => { enabled: false, }); - expect(callGatewayMock).toHaveBeenCalledTimes(1); - const call = callGatewayMock.mock.calls[0]?.[0] as { - method?: string; - params?: { id?: string; patch?: { name?: string; enabled?: boolean } }; - }; - expect(call.method).toBe("cron.update"); - expect(call.params?.id).toBe("job-1"); - expect(call.params?.patch?.name).toBe("new-name"); - expect(call.params?.patch?.enabled).toBe(false); + const params = expectSingleGatewayCallMethod("cron.update") as + | { id?: string; patch?: { name?: string; enabled?: boolean } } + | undefined; + expect(params?.id).toBe("job-1"); + expect(params?.patch?.name).toBe("new-name"); + expect(params?.patch?.enabled).toBe(false); }); it("recovers additional flat patch params for update action", async () => { @@ -546,16 +533,17 @@ describe("cron tool", () => { failureAlert: { after: 3, cooldownMs: 60_000 }, }); - const call = callGatewayMock.mock.calls[0]?.[0] as { - method?: string; - params?: { - id?: string; - patch?: { sessionTarget?: string; failureAlert?: { after?: number; cooldownMs?: number } }; - }; - }; - expect(call.method).toBe("cron.update"); - expect(call.params?.id).toBe("job-2"); - expect(call.params?.patch?.sessionTarget).toBe("main"); - expect(call.params?.patch?.failureAlert).toEqual({ after: 3, cooldownMs: 60_000 }); + const params = expectSingleGatewayCallMethod("cron.update") as + | { + id?: string; + patch?: { + sessionTarget?: string; + failureAlert?: { after?: number; cooldownMs?: number }; + }; + } + | undefined; + expect(params?.id).toBe("job-2"); + expect(params?.patch?.sessionTarget).toBe("main"); + expect(params?.patch?.failureAlert).toEqual({ after: 3, cooldownMs: 60_000 }); }); }); diff --git a/src/agents/tools/pdf-tool.helpers.ts b/src/agents/tools/pdf-tool.helpers.ts index 4cb5fde9382..9e207c6add1 100644 --- a/src/agents/tools/pdf-tool.helpers.ts +++ b/src/agents/tools/pdf-tool.helpers.ts @@ -60,32 +60,38 @@ export function coercePdfAssistantText(params: { provider: string; model: string; }): string { - const stop = params.message.stopReason; + const label = `${params.provider}/${params.model}`; const errorMessage = params.message.errorMessage?.trim(); - if (stop === "error" || stop === "aborted") { + const fail = (message?: string) => { throw new Error( - errorMessage - ? `PDF model failed (${params.provider}/${params.model}): ${errorMessage}` - : `PDF model failed (${params.provider}/${params.model})`, + message ? `PDF model failed (${label}): ${message}` : `PDF model failed (${label})`, ); + }; + if (params.message.stopReason === "error" || params.message.stopReason === "aborted") { + fail(errorMessage); } if (errorMessage) { - throw new Error(`PDF model failed (${params.provider}/${params.model}): ${errorMessage}`); + fail(errorMessage); } const text = extractAssistantText(params.message); - if (text.trim()) { - return text.trim(); + const trimmed = text.trim(); + if (trimmed) { + return trimmed; } - throw new Error(`PDF model returned no text (${params.provider}/${params.model}).`); + throw new Error(`PDF model returned no text (${label}).`); } export function coercePdfModelConfig(cfg?: OpenClawConfig): PdfModelConfig { const primary = resolveAgentModelPrimaryValue(cfg?.agents?.defaults?.pdfModel); const fallbacks = resolveAgentModelFallbackValues(cfg?.agents?.defaults?.pdfModel); - return { - ...(primary?.trim() ? { primary: primary.trim() } : {}), - ...(fallbacks.length > 0 ? { fallbacks } : {}), - }; + const modelConfig: PdfModelConfig = {}; + if (primary?.trim()) { + modelConfig.primary = primary.trim(); + } + if (fallbacks.length > 0) { + modelConfig.fallbacks = fallbacks; + } + return modelConfig; } export function resolvePdfToolMaxTokens( diff --git a/src/agents/tools/telegram-actions.ts b/src/agents/tools/telegram-actions.ts index 795ac388d05..4a9de90725d 100644 --- a/src/agents/tools/telegram-actions.ts +++ b/src/agents/tools/telegram-actions.ts @@ -89,9 +89,14 @@ export async function handleTelegramAction( mediaLocalRoots?: readonly string[]; }, ): Promise> { - const action = readStringParam(params, "action", { required: true }); - const accountId = readStringParam(params, "accountId"); - const isActionEnabled = createTelegramActionGate({ cfg, accountId }); + const { action, accountId } = { + action: readStringParam(params, "action", { required: true }), + accountId: readStringParam(params, "accountId"), + }; + const isActionEnabled = createTelegramActionGate({ + cfg, + accountId, + }); if (action === "react") { // All react failures return soft results (jsonResult with ok:false) instead diff --git a/src/agents/zai.live.test.ts b/src/agents/zai.live.test.ts index fbca5a07e0a..0ec7e493b62 100644 --- a/src/agents/zai.live.test.ts +++ b/src/agents/zai.live.test.ts @@ -1,6 +1,10 @@ import { completeSimple, getModel } from "@mariozechner/pi-ai"; import { describe, expect, it } from "vitest"; import { isTruthyEnvValue } from "../infra/env.js"; +import { + createSingleUserPromptMessage, + extractNonEmptyAssistantText, +} from "./live-test-helpers.js"; const ZAI_KEY = process.env.ZAI_API_KEY ?? process.env.Z_AI_API_KEY ?? ""; const LIVE = isTruthyEnvValue(process.env.ZAI_LIVE_TEST) || isTruthyEnvValue(process.env.LIVE); @@ -12,20 +16,11 @@ async function expectModelReturnsAssistantText(modelId: "glm-4.7" | "glm-4.7-fla const res = await completeSimple( model, { - messages: [ - { - role: "user", - content: "Reply with the word ok.", - timestamp: Date.now(), - }, - ], + messages: createSingleUserPromptMessage(), }, { apiKey: ZAI_KEY, maxTokens: 64 }, ); - const text = res.content - .filter((block) => block.type === "text") - .map((block) => block.text.trim()) - .join(" "); + const text = extractNonEmptyAssistantText(res.content); expect(text.length).toBeGreaterThan(0); } diff --git a/src/auto-reply/reply.directive.directive-behavior.e2e-mocks.ts b/src/auto-reply/reply.directive.directive-behavior.e2e-mocks.ts index 87849f1bf49..77b86bee3d2 100644 --- a/src/auto-reply/reply.directive.directive-behavior.e2e-mocks.ts +++ b/src/auto-reply/reply.directive.directive-behavior.e2e-mocks.ts @@ -1,8 +1,10 @@ import { vi } from "vitest"; +export const runEmbeddedPiAgentMock = vi.fn(); + vi.mock("../agents/pi-embedded.js", () => ({ abortEmbeddedPiRun: vi.fn().mockReturnValue(false), - runEmbeddedPiAgent: vi.fn(), + runEmbeddedPiAgent: (...args: unknown[]) => runEmbeddedPiAgentMock(...args), queueEmbeddedPiMessage: vi.fn().mockReturnValue(false), resolveEmbeddedSessionLane: (key: string) => `session:${key.trim() || "main"}`, isEmbeddedPiRunActive: vi.fn().mockReturnValue(false), diff --git a/src/auto-reply/reply.heartbeat-typing.test.ts b/src/auto-reply/reply.heartbeat-typing.test.ts index 23535789860..f677885a701 100644 --- a/src/auto-reply/reply.heartbeat-typing.test.ts +++ b/src/auto-reply/reply.heartbeat-typing.test.ts @@ -1,23 +1,13 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; +import { runEmbeddedPiAgentMock } from "./reply.directive.directive-behavior.e2e-mocks.js"; import { createTempHomeHarness, makeReplyConfig } from "./reply.test-harness.js"; -const runEmbeddedPiAgentMock = vi.fn(); - vi.mock( "../agents/model-fallback.js", async () => await import("../test-utils/model-fallback.mock.js"), ); -vi.mock("../agents/pi-embedded.js", () => ({ - abortEmbeddedPiRun: vi.fn().mockReturnValue(false), - runEmbeddedPiAgent: (params: unknown) => runEmbeddedPiAgentMock(params), - queueEmbeddedPiMessage: vi.fn().mockReturnValue(false), - resolveEmbeddedSessionLane: (key: string) => `session:${key.trim() || "main"}`, - isEmbeddedPiRunActive: vi.fn().mockReturnValue(false), - isEmbeddedPiRunStreaming: vi.fn().mockReturnValue(false), -})); - const webMocks = vi.hoisted(() => ({ webAuthExists: vi.fn().mockResolvedValue(true), getWebAuthAgeMs: vi.fn().mockReturnValue(120_000), diff --git a/src/auto-reply/reply.raw-body.test.ts b/src/auto-reply/reply.raw-body.test.ts index dcf8a42af50..306d62eb88a 100644 --- a/src/auto-reply/reply.raw-body.test.ts +++ b/src/auto-reply/reply.raw-body.test.ts @@ -1,24 +1,15 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; +import { runEmbeddedPiAgentMock } from "./reply.directive.directive-behavior.e2e-mocks.js"; import { createTempHomeHarness, makeReplyConfig } from "./reply.test-harness.js"; const agentMocks = vi.hoisted(() => ({ - runEmbeddedPiAgent: vi.fn(), loadModelCatalog: vi.fn(), webAuthExists: vi.fn().mockResolvedValue(true), getWebAuthAgeMs: vi.fn().mockReturnValue(120_000), readWebSelfId: vi.fn().mockReturnValue({ e164: "+1999" }), })); -vi.mock("../agents/pi-embedded.js", () => ({ - abortEmbeddedPiRun: vi.fn().mockReturnValue(false), - runEmbeddedPiAgent: agentMocks.runEmbeddedPiAgent, - queueEmbeddedPiMessage: vi.fn().mockReturnValue(false), - resolveEmbeddedSessionLane: (key: string) => `session:${key.trim() || "main"}`, - isEmbeddedPiRunActive: vi.fn().mockReturnValue(false), - isEmbeddedPiRunStreaming: vi.fn().mockReturnValue(false), -})); - vi.mock("../agents/model-catalog.js", () => ({ loadModelCatalog: agentMocks.loadModelCatalog, })); @@ -36,7 +27,7 @@ const { withTempHome } = createTempHomeHarness({ prefix: "openclaw-rawbody-" }); describe("RawBody directive parsing", () => { beforeEach(() => { vi.stubEnv("OPENCLAW_TEST_FAST", "1"); - agentMocks.runEmbeddedPiAgent.mockClear(); + runEmbeddedPiAgentMock.mockClear(); agentMocks.loadModelCatalog.mockClear(); agentMocks.loadModelCatalog.mockResolvedValue([ { id: "claude-opus-4-5", name: "Opus 4.5", provider: "anthropic" }, @@ -49,7 +40,7 @@ describe("RawBody directive parsing", () => { it("handles directives and history in the prompt", async () => { await withTempHome(async (home) => { - agentMocks.runEmbeddedPiAgent.mockResolvedValue({ + runEmbeddedPiAgentMock.mockResolvedValue({ payloads: [{ text: "ok" }], meta: { durationMs: 1, @@ -79,10 +70,10 @@ describe("RawBody directive parsing", () => { const text = Array.isArray(res) ? res[0]?.text : res?.text; expect(text).toBe("ok"); - expect(agentMocks.runEmbeddedPiAgent).toHaveBeenCalledOnce(); + expect(runEmbeddedPiAgentMock).toHaveBeenCalledOnce(); const prompt = - (agentMocks.runEmbeddedPiAgent.mock.calls[0]?.[0] as { prompt?: string } | undefined) - ?.prompt ?? ""; + (runEmbeddedPiAgentMock.mock.calls[0]?.[0] as { prompt?: string } | undefined)?.prompt ?? + ""; expect(prompt).toContain("Chat history since last reply (untrusted, for context):"); expect(prompt).toContain('"sender": "Peter"'); expect(prompt).toContain('"body": "hello"'); diff --git a/src/auto-reply/reply/commands-acp.test.ts b/src/auto-reply/reply/commands-acp.test.ts index 1d808350381..444aec7f84c 100644 --- a/src/auto-reply/reply/commands-acp.test.ts +++ b/src/auto-reply/reply/commands-acp.test.ts @@ -84,8 +84,10 @@ vi.mock("../../acp/runtime/session-meta.js", () => ({ resolveSessionStorePathForAcp: (args: unknown) => hoisted.resolveSessionStorePathForAcpMock(args), })); -vi.mock("../../config/sessions.js", async (importOriginal) => { - const actual = await importOriginal(); +vi.mock("../../config/sessions.js", async () => { + const actual = await vi.importActual( + "../../config/sessions.js", + ); return { ...actual, loadSessionStore: (...args: unknown[]) => hoisted.loadSessionStoreMock(...args), diff --git a/src/browser/client-actions-state.ts b/src/browser/client-actions-state.ts index ad04b652c76..a5d87aaec2d 100644 --- a/src/browser/client-actions-state.ts +++ b/src/browser/client-actions-state.ts @@ -2,18 +2,76 @@ import type { BrowserActionOk, BrowserActionTargetOk } from "./client-actions-ty import { buildProfileQuery, withBaseUrl } from "./client-actions-url.js"; import { fetchBrowserJson } from "./client-fetch.js"; +type TargetedProfileOptions = { + targetId?: string; + profile?: string; +}; + +type HttpCredentialsOptions = TargetedProfileOptions & { + username?: string; + password?: string; + clear?: boolean; +}; + +type GeolocationOptions = TargetedProfileOptions & { + latitude?: number; + longitude?: number; + accuracy?: number; + origin?: string; + clear?: boolean; +}; + +function buildStateQuery(params: { targetId?: string; key?: string; profile?: string }): string { + const query = new URLSearchParams(); + if (params.targetId) { + query.set("targetId", params.targetId); + } + if (params.key) { + query.set("key", params.key); + } + if (params.profile) { + query.set("profile", params.profile); + } + const suffix = query.toString(); + return suffix ? `?${suffix}` : ""; +} + +async function postProfileJson( + baseUrl: string | undefined, + params: { path: string; profile?: string; body: unknown }, +): Promise { + const query = buildProfileQuery(params.profile); + return await fetchBrowserJson(withBaseUrl(baseUrl, `${params.path}${query}`), { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(params.body), + timeoutMs: 20000, + }); +} + +async function postTargetedProfileJson( + baseUrl: string | undefined, + params: { + path: string; + opts: { targetId?: string; profile?: string }; + body: Record; + }, +): Promise { + return await postProfileJson(baseUrl, { + path: params.path, + profile: params.opts.profile, + body: { + targetId: params.opts.targetId, + ...params.body, + }, + }); +} + export async function browserCookies( baseUrl: string | undefined, opts: { targetId?: string; profile?: string } = {}, ): Promise<{ ok: true; targetId: string; cookies: unknown[] }> { - const q = new URLSearchParams(); - if (opts.targetId) { - q.set("targetId", opts.targetId); - } - if (opts.profile) { - q.set("profile", opts.profile); - } - const suffix = q.toString() ? `?${q.toString()}` : ""; + const suffix = buildStateQuery({ targetId: opts.targetId, profile: opts.profile }); return await fetchBrowserJson<{ ok: true; targetId: string; @@ -29,12 +87,10 @@ export async function browserCookiesSet( profile?: string; }, ): Promise { - const q = buildProfileQuery(opts.profile); - return await fetchBrowserJson(withBaseUrl(baseUrl, `/cookies/set${q}`), { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ targetId: opts.targetId, cookie: opts.cookie }), - timeoutMs: 20000, + return await postProfileJson(baseUrl, { + path: "/cookies/set", + profile: opts.profile, + body: { targetId: opts.targetId, cookie: opts.cookie }, }); } @@ -42,12 +98,10 @@ export async function browserCookiesClear( baseUrl: string | undefined, opts: { targetId?: string; profile?: string } = {}, ): Promise { - const q = buildProfileQuery(opts.profile); - return await fetchBrowserJson(withBaseUrl(baseUrl, `/cookies/clear${q}`), { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ targetId: opts.targetId }), - timeoutMs: 20000, + return await postProfileJson(baseUrl, { + path: "/cookies/clear", + profile: opts.profile, + body: { targetId: opts.targetId }, }); } @@ -60,17 +114,7 @@ export async function browserStorageGet( profile?: string; }, ): Promise<{ ok: true; targetId: string; values: Record }> { - const q = new URLSearchParams(); - if (opts.targetId) { - q.set("targetId", opts.targetId); - } - if (opts.key) { - q.set("key", opts.key); - } - if (opts.profile) { - q.set("profile", opts.profile); - } - const suffix = q.toString() ? `?${q.toString()}` : ""; + const suffix = buildStateQuery({ targetId: opts.targetId, key: opts.key, profile: opts.profile }); return await fetchBrowserJson<{ ok: true; targetId: string; @@ -88,48 +132,36 @@ export async function browserStorageSet( profile?: string; }, ): Promise { - const q = buildProfileQuery(opts.profile); - return await fetchBrowserJson( - withBaseUrl(baseUrl, `/storage/${opts.kind}/set${q}`), - { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ - targetId: opts.targetId, - key: opts.key, - value: opts.value, - }), - timeoutMs: 20000, + return await postProfileJson(baseUrl, { + path: `/storage/${opts.kind}/set`, + profile: opts.profile, + body: { + targetId: opts.targetId, + key: opts.key, + value: opts.value, }, - ); + }); } export async function browserStorageClear( baseUrl: string | undefined, opts: { kind: "local" | "session"; targetId?: string; profile?: string }, ): Promise { - const q = buildProfileQuery(opts.profile); - return await fetchBrowserJson( - withBaseUrl(baseUrl, `/storage/${opts.kind}/clear${q}`), - { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ targetId: opts.targetId }), - timeoutMs: 20000, - }, - ); + return await postProfileJson(baseUrl, { + path: `/storage/${opts.kind}/clear`, + profile: opts.profile, + body: { targetId: opts.targetId }, + }); } export async function browserSetOffline( baseUrl: string | undefined, opts: { offline: boolean; targetId?: string; profile?: string }, ): Promise { - const q = buildProfileQuery(opts.profile); - return await fetchBrowserJson(withBaseUrl(baseUrl, `/set/offline${q}`), { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ targetId: opts.targetId, offline: opts.offline }), - timeoutMs: 20000, + return await postProfileJson(baseUrl, { + path: "/set/offline", + profile: opts.profile, + body: { targetId: opts.targetId, offline: opts.offline }, }); } @@ -141,71 +173,43 @@ export async function browserSetHeaders( profile?: string; }, ): Promise { - const q = buildProfileQuery(opts.profile); - return await fetchBrowserJson(withBaseUrl(baseUrl, `/set/headers${q}`), { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ targetId: opts.targetId, headers: opts.headers }), - timeoutMs: 20000, + return await postProfileJson(baseUrl, { + path: "/set/headers", + profile: opts.profile, + body: { targetId: opts.targetId, headers: opts.headers }, }); } export async function browserSetHttpCredentials( baseUrl: string | undefined, - opts: { - username?: string; - password?: string; - clear?: boolean; - targetId?: string; - profile?: string; - } = {}, + opts: HttpCredentialsOptions = {}, ): Promise { - const q = buildProfileQuery(opts.profile); - return await fetchBrowserJson( - withBaseUrl(baseUrl, `/set/credentials${q}`), - { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ - targetId: opts.targetId, - username: opts.username, - password: opts.password, - clear: opts.clear, - }), - timeoutMs: 20000, + return await postTargetedProfileJson(baseUrl, { + path: "/set/credentials", + opts, + body: { + username: opts.username, + password: opts.password, + clear: opts.clear, }, - ); + }); } export async function browserSetGeolocation( baseUrl: string | undefined, - opts: { - latitude?: number; - longitude?: number; - accuracy?: number; - origin?: string; - clear?: boolean; - targetId?: string; - profile?: string; - } = {}, + opts: GeolocationOptions = {}, ): Promise { - const q = buildProfileQuery(opts.profile); - return await fetchBrowserJson( - withBaseUrl(baseUrl, `/set/geolocation${q}`), - { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ - targetId: opts.targetId, - latitude: opts.latitude, - longitude: opts.longitude, - accuracy: opts.accuracy, - origin: opts.origin, - clear: opts.clear, - }), - timeoutMs: 20000, + return await postTargetedProfileJson(baseUrl, { + path: "/set/geolocation", + opts, + body: { + latitude: opts.latitude, + longitude: opts.longitude, + accuracy: opts.accuracy, + origin: opts.origin, + clear: opts.clear, }, - ); + }); } export async function browserSetMedia( @@ -216,15 +220,13 @@ export async function browserSetMedia( profile?: string; }, ): Promise { - const q = buildProfileQuery(opts.profile); - return await fetchBrowserJson(withBaseUrl(baseUrl, `/set/media${q}`), { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ + return await postProfileJson(baseUrl, { + path: "/set/media", + profile: opts.profile, + body: { targetId: opts.targetId, colorScheme: opts.colorScheme, - }), - timeoutMs: 20000, + }, }); } @@ -232,15 +234,13 @@ export async function browserSetTimezone( baseUrl: string | undefined, opts: { timezoneId: string; targetId?: string; profile?: string }, ): Promise { - const q = buildProfileQuery(opts.profile); - return await fetchBrowserJson(withBaseUrl(baseUrl, `/set/timezone${q}`), { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ + return await postProfileJson(baseUrl, { + path: "/set/timezone", + profile: opts.profile, + body: { targetId: opts.targetId, timezoneId: opts.timezoneId, - }), - timeoutMs: 20000, + }, }); } @@ -248,12 +248,10 @@ export async function browserSetLocale( baseUrl: string | undefined, opts: { locale: string; targetId?: string; profile?: string }, ): Promise { - const q = buildProfileQuery(opts.profile); - return await fetchBrowserJson(withBaseUrl(baseUrl, `/set/locale${q}`), { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ targetId: opts.targetId, locale: opts.locale }), - timeoutMs: 20000, + return await postProfileJson(baseUrl, { + path: "/set/locale", + profile: opts.profile, + body: { targetId: opts.targetId, locale: opts.locale }, }); } @@ -261,12 +259,10 @@ export async function browserSetDevice( baseUrl: string | undefined, opts: { name: string; targetId?: string; profile?: string }, ): Promise { - const q = buildProfileQuery(opts.profile); - return await fetchBrowserJson(withBaseUrl(baseUrl, `/set/device${q}`), { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ targetId: opts.targetId, name: opts.name }), - timeoutMs: 20000, + return await postProfileJson(baseUrl, { + path: "/set/device", + profile: opts.profile, + body: { targetId: opts.targetId, name: opts.name }, }); } @@ -274,11 +270,9 @@ export async function browserClearPermissions( baseUrl: string | undefined, opts: { targetId?: string; profile?: string } = {}, ): Promise { - const q = buildProfileQuery(opts.profile); - return await fetchBrowserJson(withBaseUrl(baseUrl, `/set/geolocation${q}`), { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ targetId: opts.targetId, clear: true }), - timeoutMs: 20000, + return await postProfileJson(baseUrl, { + path: "/set/geolocation", + profile: opts.profile, + body: { targetId: opts.targetId, clear: true }, }); } diff --git a/src/browser/paths.test.ts b/src/browser/paths.test.ts index f3ed376c413..14af336ff53 100644 --- a/src/browser/paths.test.ts +++ b/src/browser/paths.test.ts @@ -28,6 +28,17 @@ async function withFixtureRoot( } } +async function createAliasedUploadsRoot(baseDir: string): Promise<{ + canonicalUploadsDir: string; + aliasedUploadsDir: string; +}> { + const canonicalUploadsDir = path.join(baseDir, "canonical", "uploads"); + const aliasedUploadsDir = path.join(baseDir, "uploads-link"); + await fs.mkdir(canonicalUploadsDir, { recursive: true }); + await fs.symlink(canonicalUploadsDir, aliasedUploadsDir); + return { canonicalUploadsDir, aliasedUploadsDir }; +} + describe("resolveExistingPathsWithinRoot", () => { function expectInvalidResult( result: Awaited>, @@ -167,10 +178,7 @@ describe("resolveExistingPathsWithinRoot", () => { "accepts canonical absolute paths when upload root is a symlink alias", async () => { await withFixtureRoot(async ({ baseDir }) => { - const canonicalUploadsDir = path.join(baseDir, "canonical", "uploads"); - const aliasedUploadsDir = path.join(baseDir, "uploads-link"); - await fs.mkdir(canonicalUploadsDir, { recursive: true }); - await fs.symlink(canonicalUploadsDir, aliasedUploadsDir); + const { canonicalUploadsDir, aliasedUploadsDir } = await createAliasedUploadsRoot(baseDir); const filePath = path.join(canonicalUploadsDir, "ok.txt"); await fs.writeFile(filePath, "ok", "utf8"); @@ -198,10 +206,7 @@ describe("resolveExistingPathsWithinRoot", () => { "rejects canonical absolute paths outside symlinked upload root", async () => { await withFixtureRoot(async ({ baseDir }) => { - const canonicalUploadsDir = path.join(baseDir, "canonical", "uploads"); - const aliasedUploadsDir = path.join(baseDir, "uploads-link"); - await fs.mkdir(canonicalUploadsDir, { recursive: true }); - await fs.symlink(canonicalUploadsDir, aliasedUploadsDir); + const { aliasedUploadsDir } = await createAliasedUploadsRoot(baseDir); const outsideDir = path.join(baseDir, "outside"); await fs.mkdir(outsideDir, { recursive: true }); diff --git a/src/browser/profiles-service.test.ts b/src/browser/profiles-service.test.ts index 3477d6e8c13..38ed6e3c03c 100644 --- a/src/browser/profiles-service.test.ts +++ b/src/browser/profiles-service.test.ts @@ -45,15 +45,23 @@ function createCtx(resolved: BrowserServerState["resolved"]) { return { state, ctx }; } +async function createWorkProfileWithConfig(params: { + resolved: BrowserServerState["resolved"]; + browserConfig: Record; +}) { + const { ctx, state } = createCtx(params.resolved); + vi.mocked(loadConfig).mockReturnValue({ browser: params.browserConfig }); + const service = createBrowserProfilesService(ctx); + const result = await service.createProfile({ name: "work" }); + return { result, state }; +} + describe("BrowserProfilesService", () => { it("allocates next local port for new profiles", async () => { - const resolved = resolveBrowserConfig({}); - const { ctx, state } = createCtx(resolved); - - vi.mocked(loadConfig).mockReturnValue({ browser: { profiles: {} } }); - - const service = createBrowserProfilesService(ctx); - const result = await service.createProfile({ name: "work" }); + const { result, state } = await createWorkProfileWithConfig({ + resolved: resolveBrowserConfig({}), + browserConfig: { profiles: {} }, + }); expect(result.cdpPort).toBe(18801); expect(result.isRemote).toBe(false); @@ -74,12 +82,10 @@ describe("BrowserProfilesService", () => { ...baseWithoutRange, controlPort: 30000, } as BrowserServerState["resolved"]; - const { ctx, state } = createCtx(resolved); - - vi.mocked(loadConfig).mockReturnValue({ browser: { profiles: {} } }); - - const service = createBrowserProfilesService(ctx); - const result = await service.createProfile({ name: "work" }); + const { result, state } = await createWorkProfileWithConfig({ + resolved, + browserConfig: { profiles: {} }, + }); expect(result.cdpPort).toBe(30009); expect(state.resolved.profiles.work?.cdpPort).toBe(30009); @@ -87,13 +93,10 @@ describe("BrowserProfilesService", () => { }); it("allocates from configured cdpPortRangeStart for new local profiles", async () => { - const resolved = resolveBrowserConfig({ cdpPortRangeStart: 19000 }); - const { ctx, state } = createCtx(resolved); - - vi.mocked(loadConfig).mockReturnValue({ browser: { cdpPortRangeStart: 19000, profiles: {} } }); - - const service = createBrowserProfilesService(ctx); - const result = await service.createProfile({ name: "work" }); + const { result, state } = await createWorkProfileWithConfig({ + resolved: resolveBrowserConfig({ cdpPortRangeStart: 19000 }), + browserConfig: { cdpPortRangeStart: 19000, profiles: {} }, + }); expect(result.cdpPort).toBe(19001); expect(result.isRemote).toBe(false); diff --git a/src/browser/pw-session.ts b/src/browser/pw-session.ts index 073562d1c3c..b657bb2e252 100644 --- a/src/browser/pw-session.ts +++ b/src/browser/pw-session.ts @@ -456,6 +456,18 @@ async function findPageByTargetId( return null; } +async function resolvePageByTargetIdOrThrow(opts: { + cdpUrl: string; + targetId: string; +}): Promise { + const { browser } = await connectBrowser(opts.cdpUrl); + const page = await findPageByTargetId(browser, opts.targetId, opts.cdpUrl); + if (!page) { + throw new Error("tab not found"); + } + return page; +} + export async function getPageForTargetId(opts: { cdpUrl: string; targetId?: string; @@ -782,11 +794,7 @@ export async function closePageByTargetIdViaPlaywright(opts: { cdpUrl: string; targetId: string; }): Promise { - const { browser } = await connectBrowser(opts.cdpUrl); - const page = await findPageByTargetId(browser, opts.targetId, opts.cdpUrl); - if (!page) { - throw new Error("tab not found"); - } + const page = await resolvePageByTargetIdOrThrow(opts); await page.close(); } @@ -798,11 +806,7 @@ export async function focusPageByTargetIdViaPlaywright(opts: { cdpUrl: string; targetId: string; }): Promise { - const { browser } = await connectBrowser(opts.cdpUrl); - const page = await findPageByTargetId(browser, opts.targetId, opts.cdpUrl); - if (!page) { - throw new Error("tab not found"); - } + const page = await resolvePageByTargetIdOrThrow(opts); try { await page.bringToFront(); } catch (err) { diff --git a/src/browser/pw-tools-core.interactions.set-input-files.test.ts b/src/browser/pw-tools-core.interactions.set-input-files.test.ts index dfbd6f58563..93dbf0c44c5 100644 --- a/src/browser/pw-tools-core.interactions.set-input-files.test.ts +++ b/src/browser/pw-tools-core.interactions.set-input-files.test.ts @@ -41,6 +41,18 @@ vi.mock("./paths.js", () => { let setInputFilesViaPlaywright: typeof import("./pw-tools-core.interactions.js").setInputFilesViaPlaywright; +function seedSingleLocatorPage(): { setInputFiles: ReturnType } { + const setInputFiles = vi.fn(async () => {}); + locator = { + setInputFiles, + elementHandle: vi.fn(async () => null), + }; + page = { + locator: vi.fn(() => ({ first: () => locator })), + }; + return { setInputFiles }; +} + describe("setInputFilesViaPlaywright", () => { beforeAll(async () => { ({ setInputFilesViaPlaywright } = await import("./pw-tools-core.interactions.js")); @@ -57,14 +69,7 @@ describe("setInputFilesViaPlaywright", () => { }); it("revalidates upload paths and uses resolved canonical paths for inputRef", async () => { - const setInputFiles = vi.fn(async () => {}); - locator = { - setInputFiles, - elementHandle: vi.fn(async () => null), - }; - page = { - locator: vi.fn(() => ({ first: () => locator })), - }; + const { setInputFiles } = seedSingleLocatorPage(); await setInputFilesViaPlaywright({ cdpUrl: "http://127.0.0.1:18792", @@ -88,14 +93,7 @@ describe("setInputFilesViaPlaywright", () => { error: "Invalid path: must stay within uploads directory", }); - const setInputFiles = vi.fn(async () => {}); - locator = { - setInputFiles, - elementHandle: vi.fn(async () => null), - }; - page = { - locator: vi.fn(() => ({ first: () => locator })), - }; + const { setInputFiles } = seedSingleLocatorPage(); await expect( setInputFilesViaPlaywright({ diff --git a/src/browser/pw-tools-core.screenshots-element-selector.test.ts b/src/browser/pw-tools-core.screenshots-element-selector.test.ts index 1894d65912f..3eb7e333db0 100644 --- a/src/browser/pw-tools-core.screenshots-element-selector.test.ts +++ b/src/browser/pw-tools-core.screenshots-element-selector.test.ts @@ -14,6 +14,17 @@ installPwToolsCoreTestHooks(); const sessionMocks = getPwToolsCoreSessionMocks(); const mod = await import("./pw-tools-core.js"); +function createFileChooserPageMocks() { + const fileChooser = { setFiles: vi.fn(async () => {}) }; + const press = vi.fn(async () => {}); + const waitForEvent = vi.fn(async () => fileChooser); + setPwToolsCoreCurrentPage({ + waitForEvent, + keyboard: { press }, + }); + return { fileChooser, press, waitForEvent }; +} + describe("pw-tools-core", () => { it("screenshots an element selector", async () => { const elementScreenshot = vi.fn(async () => Buffer.from("E")); @@ -118,13 +129,7 @@ describe("pw-tools-core", () => { }); it("revalidates file-chooser paths at use-time and cancels missing files", async () => { const missingPath = path.join(DEFAULT_UPLOAD_DIR, `vitest-missing-${crypto.randomUUID()}.txt`); - const fileChooser = { setFiles: vi.fn(async () => {}) }; - const press = vi.fn(async () => {}); - const waitForEvent = vi.fn(async () => fileChooser); - setPwToolsCoreCurrentPage({ - waitForEvent, - keyboard: { press }, - }); + const { fileChooser, press } = createFileChooserPageMocks(); await mod.armFileUploadViaPlaywright({ cdpUrl: "http://127.0.0.1:18792", @@ -139,13 +144,7 @@ describe("pw-tools-core", () => { expect(fileChooser.setFiles).not.toHaveBeenCalled(); }); it("arms the next file chooser and escapes if no paths provided", async () => { - const fileChooser = { setFiles: vi.fn(async () => {}) }; - const press = vi.fn(async () => {}); - const waitForEvent = vi.fn(async () => fileChooser); - setPwToolsCoreCurrentPage({ - waitForEvent, - keyboard: { press }, - }); + const { fileChooser, press } = createFileChooserPageMocks(); await mod.armFileUploadViaPlaywright({ cdpUrl: "http://127.0.0.1:18792", diff --git a/src/browser/server-context.ensure-browser-available.waits-for-cdp-ready.test.ts b/src/browser/server-context.ensure-browser-available.waits-for-cdp-ready.test.ts index f475d84d00b..47df8607043 100644 --- a/src/browser/server-context.ensure-browser-available.waits-for-cdp-ready.test.ts +++ b/src/browser/server-context.ensure-browser-available.waits-for-cdp-ready.test.ts @@ -1,17 +1,7 @@ import type { ChildProcessWithoutNullStreams } from "node:child_process"; import { EventEmitter } from "node:events"; import { afterEach, describe, expect, it, vi } from "vitest"; - -vi.mock("./chrome.js", () => ({ - isChromeCdpReady: vi.fn(async () => true), - isChromeReachable: vi.fn(async () => true), - launchOpenClawChrome: vi.fn(async () => { - throw new Error("unexpected launch"); - }), - resolveOpenClawUserDataDir: vi.fn(() => "/tmp/openclaw-test"), - stopOpenClawChrome: vi.fn(async () => {}), -})); - +import "./server-context.chrome-test-harness.js"; import * as chromeModule from "./chrome.js"; import type { RunningChrome } from "./chrome.js"; import type { BrowserServerState } from "./server-context.js"; @@ -63,6 +53,22 @@ function mockLaunchedChrome( }); } +function setupEnsureBrowserAvailableHarness() { + vi.useFakeTimers(); + + const launchOpenClawChrome = vi.mocked(chromeModule.launchOpenClawChrome); + const stopOpenClawChrome = vi.mocked(chromeModule.stopOpenClawChrome); + const isChromeReachable = vi.mocked(chromeModule.isChromeReachable); + const isChromeCdpReady = vi.mocked(chromeModule.isChromeCdpReady); + isChromeReachable.mockResolvedValue(false); + + const state = makeBrowserState(); + const ctx = createBrowserRouteContext({ getState: () => state }); + const profile = ctx.forProfile("openclaw"); + + return { launchOpenClawChrome, stopOpenClawChrome, isChromeCdpReady, profile }; +} + afterEach(() => { vi.useRealTimers(); vi.clearAllMocks(); @@ -71,21 +77,11 @@ afterEach(() => { describe("browser server-context ensureBrowserAvailable", () => { it("waits for CDP readiness after launching to avoid follow-up PortInUseError races (#21149)", async () => { - vi.useFakeTimers(); - - const launchOpenClawChrome = vi.mocked(chromeModule.launchOpenClawChrome); - const stopOpenClawChrome = vi.mocked(chromeModule.stopOpenClawChrome); - const isChromeReachable = vi.mocked(chromeModule.isChromeReachable); - const isChromeCdpReady = vi.mocked(chromeModule.isChromeCdpReady); - - isChromeReachable.mockResolvedValue(false); + const { launchOpenClawChrome, stopOpenClawChrome, isChromeCdpReady, profile } = + setupEnsureBrowserAvailableHarness(); isChromeCdpReady.mockResolvedValueOnce(false).mockResolvedValue(true); mockLaunchedChrome(launchOpenClawChrome, 123); - const state = makeBrowserState(); - const ctx = createBrowserRouteContext({ getState: () => state }); - const profile = ctx.forProfile("openclaw"); - const promise = profile.ensureBrowserAvailable(); await vi.advanceTimersByTimeAsync(100); await expect(promise).resolves.toBeUndefined(); @@ -96,21 +92,11 @@ describe("browser server-context ensureBrowserAvailable", () => { }); it("stops launched chrome when CDP readiness never arrives", async () => { - vi.useFakeTimers(); - - const launchOpenClawChrome = vi.mocked(chromeModule.launchOpenClawChrome); - const stopOpenClawChrome = vi.mocked(chromeModule.stopOpenClawChrome); - const isChromeReachable = vi.mocked(chromeModule.isChromeReachable); - const isChromeCdpReady = vi.mocked(chromeModule.isChromeCdpReady); - - isChromeReachable.mockResolvedValue(false); + const { launchOpenClawChrome, stopOpenClawChrome, isChromeCdpReady, profile } = + setupEnsureBrowserAvailableHarness(); isChromeCdpReady.mockResolvedValue(false); mockLaunchedChrome(launchOpenClawChrome, 321); - const state = makeBrowserState(); - const ctx = createBrowserRouteContext({ getState: () => state }); - const profile = ctx.forProfile("openclaw"); - const promise = profile.ensureBrowserAvailable(); const rejected = expect(promise).rejects.toThrow("not reachable after start"); await vi.advanceTimersByTimeAsync(8100); diff --git a/src/browser/server-context.reset.test.ts b/src/browser/server-context.reset.test.ts index 1796fa3f68b..09a20b48edf 100644 --- a/src/browser/server-context.reset.test.ts +++ b/src/browser/server-context.reset.test.ts @@ -24,23 +24,41 @@ afterEach(() => { vi.clearAllMocks(); }); +function localOpenClawProfile(): Parameters[0]["profile"] { + return { + name: "openclaw", + cdpUrl: "http://127.0.0.1:18800", + cdpHost: "127.0.0.1", + cdpIsLoopback: true, + cdpPort: 18800, + color: "#f60", + driver: "openclaw", + attachOnly: false, + }; +} + +function createLocalOpenClawResetOps( + params: Omit[0], "profile">, +) { + return createProfileResetOps({ profile: localOpenClawProfile(), ...params }); +} + +function createStatelessResetOps(profile: Parameters[0]["profile"]) { + return createProfileResetOps({ + profile, + getProfileState: () => ({ profile: {} as never, running: null }), + stopRunningBrowser: vi.fn(async () => ({ stopped: false })), + isHttpReachable: vi.fn(async () => false), + resolveOpenClawUserDataDir: (name: string) => `/tmp/${name}`, + }); +} + describe("createProfileResetOps", () => { it("stops extension relay for extension profiles", async () => { - const ops = createProfileResetOps({ - profile: { - name: "chrome", - cdpUrl: "http://127.0.0.1:18800", - cdpHost: "127.0.0.1", - cdpIsLoopback: true, - cdpPort: 18800, - color: "#f60", - driver: "extension", - attachOnly: false, - }, - getProfileState: () => ({ profile: {} as never, running: null }), - stopRunningBrowser: vi.fn(async () => ({ stopped: false })), - isHttpReachable: vi.fn(async () => false), - resolveOpenClawUserDataDir: (name: string) => `/tmp/${name}`, + const ops = createStatelessResetOps({ + ...localOpenClawProfile(), + name: "chrome", + driver: "extension", }); await expect(ops.resetProfile()).resolves.toEqual({ @@ -54,21 +72,14 @@ describe("createProfileResetOps", () => { }); it("rejects remote non-extension profiles", async () => { - const ops = createProfileResetOps({ - profile: { - name: "remote", - cdpUrl: "https://browserless.example/chrome", - cdpHost: "browserless.example", - cdpIsLoopback: false, - cdpPort: 443, - color: "#0f0", - driver: "openclaw", - attachOnly: false, - }, - getProfileState: () => ({ profile: {} as never, running: null }), - stopRunningBrowser: vi.fn(async () => ({ stopped: false })), - isHttpReachable: vi.fn(async () => false), - resolveOpenClawUserDataDir: (name: string) => `/tmp/${name}`, + const ops = createStatelessResetOps({ + ...localOpenClawProfile(), + name: "remote", + cdpUrl: "https://browserless.example/chrome", + cdpHost: "browserless.example", + cdpIsLoopback: false, + cdpPort: 443, + color: "#0f0", }); await expect(ops.resetProfile()).rejects.toThrow(/only supported for local profiles/i); @@ -86,17 +97,7 @@ describe("createProfileResetOps", () => { running: { pid: 1 } as never, })); - const ops = createProfileResetOps({ - profile: { - name: "openclaw", - cdpUrl: "http://127.0.0.1:18800", - cdpHost: "127.0.0.1", - cdpIsLoopback: true, - cdpPort: 18800, - color: "#f60", - driver: "openclaw", - attachOnly: false, - }, + const ops = createLocalOpenClawResetOps({ getProfileState, stopRunningBrowser, isHttpReachable, @@ -121,17 +122,7 @@ describe("createProfileResetOps", () => { fs.mkdirSync(profileDir, { recursive: true }); const stopRunningBrowser = vi.fn(async () => ({ stopped: false })); - const ops = createProfileResetOps({ - profile: { - name: "openclaw", - cdpUrl: "http://127.0.0.1:18800", - cdpHost: "127.0.0.1", - cdpIsLoopback: true, - cdpPort: 18800, - color: "#f60", - driver: "openclaw", - attachOnly: false, - }, + const ops = createLocalOpenClawResetOps({ getProfileState: () => ({ profile: {} as never, running: null }), stopRunningBrowser, isHttpReachable: vi.fn(async () => true), diff --git a/src/browser/server-context.tab-selection-state.suite.ts b/src/browser/server-context.tab-selection-state.suite.ts index d9541d91afe..a9729af8a89 100644 --- a/src/browser/server-context.tab-selection-state.suite.ts +++ b/src/browser/server-context.tab-selection-state.suite.ts @@ -54,6 +54,34 @@ function createOldTabCleanupFetchMock( }); } +function createManagedTabListFetchMock(params: { + existingTabs: ReturnType; + onClose: (url: string) => Response | Promise; +}): ReturnType { + return vi.fn(async (url: unknown) => { + const value = String(url); + if (value.includes("/json/list")) { + return { ok: true, json: async () => params.existingTabs } as unknown as Response; + } + if (value.includes("/json/close/")) { + return await params.onClose(value); + } + throw new Error(`unexpected fetch: ${value}`); + }); +} + +async function openManagedTabWithRunningProfile(params: { + fetchMock: ReturnType; + url?: string; +}) { + global.fetch = withFetchPreconnect(params.fetchMock); + const state = makeState("openclaw"); + seedRunningProfileState(state); + const ctx = createBrowserRouteContext({ getState: () => state }); + const openclaw = ctx.forProfile("openclaw"); + return await openclaw.openTab(params.url ?? "http://127.0.0.1:3009"); +} + describe("browser server-context tab selection state", () => { it("updates lastTargetId when openTab is created via CDP", async () => { const createTargetViaCdp = vi @@ -99,13 +127,7 @@ describe("browser server-context tab selection state", () => { const existingTabs = makeManagedTabsWithNew(); const fetchMock = createOldTabCleanupFetchMock(existingTabs); - global.fetch = withFetchPreconnect(fetchMock); - const state = makeState("openclaw"); - seedRunningProfileState(state); - const ctx = createBrowserRouteContext({ getState: () => state }); - const openclaw = ctx.forProfile("openclaw"); - - const opened = await openclaw.openTab("http://127.0.0.1:3009"); + const opened = await openManagedTabWithRunningProfile({ fetchMock }); expect(opened.targetId).toBe("NEW"); await expectOldManagedTabClose(fetchMock); }); @@ -115,13 +137,7 @@ describe("browser server-context tab selection state", () => { const existingTabs = makeManagedTabsWithNew({ newFirst: true }); const fetchMock = createOldTabCleanupFetchMock(existingTabs, { rejectNewTabClose: true }); - global.fetch = withFetchPreconnect(fetchMock); - const state = makeState("openclaw"); - seedRunningProfileState(state); - const ctx = createBrowserRouteContext({ getState: () => state }); - const openclaw = ctx.forProfile("openclaw"); - - const opened = await openclaw.openTab("http://127.0.0.1:3009"); + const opened = await openManagedTabWithRunningProfile({ fetchMock }); expect(opened.targetId).toBe("NEW"); await expectOldManagedTabClose(fetchMock); expect(fetchMock).not.toHaveBeenCalledWith( @@ -170,16 +186,11 @@ describe("browser server-context tab selection state", () => { it("does not run managed tab cleanup in attachOnly mode", async () => { vi.spyOn(cdpModule, "createTargetViaCdp").mockResolvedValue({ targetId: "NEW" }); const existingTabs = makeManagedTabsWithNew(); - - const fetchMock = vi.fn(async (url: unknown) => { - const value = String(url); - if (value.includes("/json/list")) { - return { ok: true, json: async () => existingTabs } as unknown as Response; - } - if (value.includes("/json/close/")) { + const fetchMock = createManagedTabListFetchMock({ + existingTabs, + onClose: () => { throw new Error("should not close tabs in attachOnly mode"); - } - throw new Error(`unexpected fetch: ${value}`); + }, }); global.fetch = withFetchPreconnect(fetchMock); @@ -199,26 +210,18 @@ describe("browser server-context tab selection state", () => { it("does not block openTab on slow best-effort cleanup closes", async () => { vi.spyOn(cdpModule, "createTargetViaCdp").mockResolvedValue({ targetId: "NEW" }); const existingTabs = makeManagedTabsWithNew(); - - const fetchMock = vi.fn(async (url: unknown) => { - const value = String(url); - if (value.includes("/json/list")) { - return { ok: true, json: async () => existingTabs } as unknown as Response; - } - if (value.includes("/json/close/OLD1")) { - return new Promise(() => {}); - } - throw new Error(`unexpected fetch: ${value}`); + const fetchMock = createManagedTabListFetchMock({ + existingTabs, + onClose: (url) => { + if (url.includes("/json/close/OLD1")) { + return new Promise(() => {}); + } + throw new Error(`unexpected fetch: ${url}`); + }, }); - global.fetch = withFetchPreconnect(fetchMock); - const state = makeState("openclaw"); - seedRunningProfileState(state); - const ctx = createBrowserRouteContext({ getState: () => state }); - const openclaw = ctx.forProfile("openclaw"); - const opened = await Promise.race([ - openclaw.openTab("http://127.0.0.1:3009"), + openManagedTabWithRunningProfile({ fetchMock }), new Promise((_, reject) => setTimeout(() => reject(new Error("openTab timed out waiting for cleanup")), 300), ), diff --git a/src/cli/browser-cli-debug.ts b/src/cli/browser-cli-debug.ts index a0b7004b832..c10b308e0e2 100644 --- a/src/cli/browser-cli-debug.ts +++ b/src/cli/browser-cli-debug.ts @@ -5,6 +5,15 @@ import { shortenHomePath } from "../utils.js"; import { callBrowserRequest, type BrowserParentOpts } from "./browser-cli-shared.js"; import { runCommandWithRuntime } from "./cli-utils.js"; +const BROWSER_DEBUG_TIMEOUT_MS = 20000; + +type BrowserRequestParams = Parameters[1]; + +type DebugContext = { + parent: BrowserParentOpts; + profile?: string; +}; + function runBrowserDebug(action: () => Promise) { return runCommandWithRuntime(defaultRuntime, action, (err) => { defaultRuntime.error(danger(String(err))); @@ -12,6 +21,39 @@ function runBrowserDebug(action: () => Promise) { }); } +async function withDebugContext( + cmd: Command, + parentOpts: (cmd: Command) => BrowserParentOpts, + action: (context: DebugContext) => Promise, +) { + const parent = parentOpts(cmd); + await runBrowserDebug(() => + action({ + parent, + profile: parent.browserProfile, + }), + ); +} + +function printJsonResult(parent: BrowserParentOpts, result: unknown): boolean { + if (!parent.json) { + return false; + } + defaultRuntime.log(JSON.stringify(result, null, 2)); + return true; +} + +async function callDebugRequest( + parent: BrowserParentOpts, + params: BrowserRequestParams, +): Promise { + return callBrowserRequest(parent, params, { timeoutMs: BROWSER_DEBUG_TIMEOUT_MS }); +} + +function resolveProfileQuery(profile?: string) { + return profile ? { profile } : undefined; +} + function resolveDebugQuery(params: { targetId?: unknown; clear?: unknown; @@ -36,24 +78,17 @@ export function registerBrowserDebugCommands( .argument("", "Ref id from snapshot") .option("--target-id ", "CDP target id (or unique prefix)") .action(async (ref: string, opts, cmd) => { - const parent = parentOpts(cmd); - const profile = parent?.browserProfile; - await runBrowserDebug(async () => { - const result = await callBrowserRequest( - parent, - { - method: "POST", - path: "/highlight", - query: profile ? { profile } : undefined, - body: { - ref: ref.trim(), - targetId: opts.targetId?.trim() || undefined, - }, + await withDebugContext(cmd, parentOpts, async ({ parent, profile }) => { + const result = await callDebugRequest(parent, { + method: "POST", + path: "/highlight", + query: resolveProfileQuery(profile), + body: { + ref: ref.trim(), + targetId: opts.targetId?.trim() || undefined, }, - { timeoutMs: 20000 }, - ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); + }); + if (printJsonResult(parent, result)) { return; } defaultRuntime.log(`highlighted ${ref.trim()}`); @@ -66,26 +101,19 @@ export function registerBrowserDebugCommands( .option("--clear", "Clear stored errors after reading", false) .option("--target-id ", "CDP target id (or unique prefix)") .action(async (opts, cmd) => { - const parent = parentOpts(cmd); - const profile = parent?.browserProfile; - await runBrowserDebug(async () => { - const result = await callBrowserRequest<{ + await withDebugContext(cmd, parentOpts, async ({ parent, profile }) => { + const result = await callDebugRequest<{ errors: Array<{ timestamp: string; name?: string; message: string }>; - }>( - parent, - { - method: "GET", - path: "/errors", - query: resolveDebugQuery({ - targetId: opts.targetId, - clear: opts.clear, - profile, - }), - }, - { timeoutMs: 20000 }, - ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); + }>(parent, { + method: "GET", + path: "/errors", + query: resolveDebugQuery({ + targetId: opts.targetId, + clear: opts.clear, + profile, + }), + }); + if (printJsonResult(parent, result)) { return; } if (!result.errors.length) { @@ -107,10 +135,8 @@ export function registerBrowserDebugCommands( .option("--clear", "Clear stored requests after reading", false) .option("--target-id ", "CDP target id (or unique prefix)") .action(async (opts, cmd) => { - const parent = parentOpts(cmd); - const profile = parent?.browserProfile; - await runBrowserDebug(async () => { - const result = await callBrowserRequest<{ + await withDebugContext(cmd, parentOpts, async ({ parent, profile }) => { + const result = await callDebugRequest<{ requests: Array<{ timestamp: string; method: string; @@ -119,22 +145,17 @@ export function registerBrowserDebugCommands( url: string; failureText?: string; }>; - }>( - parent, - { - method: "GET", - path: "/requests", - query: resolveDebugQuery({ - targetId: opts.targetId, - filter: opts.filter, - clear: opts.clear, - profile, - }), - }, - { timeoutMs: 20000 }, - ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); + }>(parent, { + method: "GET", + path: "/requests", + query: resolveDebugQuery({ + targetId: opts.targetId, + filter: opts.filter, + clear: opts.clear, + profile, + }), + }); + if (printJsonResult(parent, result)) { return; } if (!result.requests.length) { @@ -164,26 +185,19 @@ export function registerBrowserDebugCommands( .option("--no-snapshots", "Disable snapshots") .option("--sources", "Include sources (bigger traces)", false) .action(async (opts, cmd) => { - const parent = parentOpts(cmd); - const profile = parent?.browserProfile; - await runBrowserDebug(async () => { - const result = await callBrowserRequest( - parent, - { - method: "POST", - path: "/trace/start", - query: profile ? { profile } : undefined, - body: { - targetId: opts.targetId?.trim() || undefined, - screenshots: Boolean(opts.screenshots), - snapshots: Boolean(opts.snapshots), - sources: Boolean(opts.sources), - }, + await withDebugContext(cmd, parentOpts, async ({ parent, profile }) => { + const result = await callDebugRequest(parent, { + method: "POST", + path: "/trace/start", + query: resolveProfileQuery(profile), + body: { + targetId: opts.targetId?.trim() || undefined, + screenshots: Boolean(opts.screenshots), + snapshots: Boolean(opts.snapshots), + sources: Boolean(opts.sources), }, - { timeoutMs: 20000 }, - ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); + }); + if (printJsonResult(parent, result)) { return; } defaultRuntime.log("trace started"); @@ -199,24 +213,17 @@ export function registerBrowserDebugCommands( ) .option("--target-id ", "CDP target id (or unique prefix)") .action(async (opts, cmd) => { - const parent = parentOpts(cmd); - const profile = parent?.browserProfile; - await runBrowserDebug(async () => { - const result = await callBrowserRequest<{ path: string }>( - parent, - { - method: "POST", - path: "/trace/stop", - query: profile ? { profile } : undefined, - body: { - targetId: opts.targetId?.trim() || undefined, - path: opts.out?.trim() || undefined, - }, + await withDebugContext(cmd, parentOpts, async ({ parent, profile }) => { + const result = await callDebugRequest<{ path: string }>(parent, { + method: "POST", + path: "/trace/stop", + query: resolveProfileQuery(profile), + body: { + targetId: opts.targetId?.trim() || undefined, + path: opts.out?.trim() || undefined, }, - { timeoutMs: 20000 }, - ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); + }); + if (printJsonResult(parent, result)) { return; } defaultRuntime.log(`TRACE:${shortenHomePath(result.path)}`); diff --git a/src/cli/browser-cli-manage.timeout-option.test.ts b/src/cli/browser-cli-manage.timeout-option.test.ts index bb4d6469c71..134f13bc3c3 100644 --- a/src/cli/browser-cli-manage.timeout-option.test.ts +++ b/src/cli/browser-cli-manage.timeout-option.test.ts @@ -2,28 +2,36 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { registerBrowserManageCommands } from "./browser-cli-manage.js"; import { createBrowserProgram } from "./browser-cli-test-helpers.js"; -const mocks = vi.hoisted(() => ({ - callBrowserRequest: vi.fn(async (_opts: unknown, req: { path?: string }) => - req.path === "/" - ? { - enabled: true, - running: true, - pid: 1, - cdpPort: 18800, - chosenBrowser: "chrome", - userDataDir: "/tmp/openclaw", - color: "blue", - headless: true, - attachOnly: false, - } - : {}, - ), - runtime: { - log: vi.fn(), - error: vi.fn(), - exit: vi.fn(), - }, -})); +const mocks = vi.hoisted(() => { + const runtimeLog = vi.fn(); + const runtimeError = vi.fn(); + const runtimeExit = vi.fn(); + return { + callBrowserRequest: vi.fn(async (_opts: unknown, req: { path?: string }) => + req.path === "/" + ? { + enabled: true, + running: true, + pid: 1, + cdpPort: 18800, + chosenBrowser: "chrome", + userDataDir: "/tmp/openclaw", + color: "blue", + headless: true, + attachOnly: false, + } + : {}, + ), + runtimeLog, + runtimeError, + runtimeExit, + runtime: { + log: runtimeLog, + error: runtimeError, + exit: runtimeExit, + }, + }; +}); vi.mock("./browser-cli-shared.js", () => ({ callBrowserRequest: mocks.callBrowserRequest, @@ -51,9 +59,9 @@ describe("browser manage start timeout option", () => { beforeEach(() => { mocks.callBrowserRequest.mockClear(); - mocks.runtime.log.mockClear(); - mocks.runtime.error.mockClear(); - mocks.runtime.exit.mockClear(); + mocks.runtimeLog.mockClear(); + mocks.runtimeError.mockClear(); + mocks.runtimeExit.mockClear(); }); it("uses parent --timeout for browser start instead of hardcoded 15s", async () => { diff --git a/src/cli/browser-cli-manage.ts b/src/cli/browser-cli-manage.ts index cea1ea24cc3..53b83ca3f97 100644 --- a/src/cli/browser-cli-manage.ts +++ b/src/cli/browser-cli-manage.ts @@ -13,6 +13,35 @@ import { shortenHomePath } from "../utils.js"; import { callBrowserRequest, type BrowserParentOpts } from "./browser-cli-shared.js"; import { runCommandWithRuntime } from "./cli-utils.js"; +function resolveProfileQuery(profile?: string) { + return profile ? { profile } : undefined; +} + +function printJsonResult(parent: BrowserParentOpts, payload: unknown): boolean { + if (!parent?.json) { + return false; + } + defaultRuntime.log(JSON.stringify(payload, null, 2)); + return true; +} + +async function callTabAction( + parent: BrowserParentOpts, + profile: string | undefined, + body: { action: "new" | "select" | "close"; index?: number }, +) { + return callBrowserRequest( + parent, + { + method: "POST", + path: "/tabs/action", + query: resolveProfileQuery(profile), + body, + }, + { timeoutMs: 10_000 }, + ); +} + async function fetchBrowserStatus( parent: BrowserParentOpts, profile?: string, @@ -22,7 +51,7 @@ async function fetchBrowserStatus( { method: "GET", path: "/", - query: profile ? { profile } : undefined, + query: resolveProfileQuery(profile), }, { timeoutMs: 1500, @@ -37,11 +66,10 @@ async function runBrowserToggle( await callBrowserRequest(parent, { method: "POST", path: params.path, - query: params.profile ? { profile: params.profile } : undefined, + query: resolveProfileQuery(params.profile), }); const status = await fetchBrowserStatus(parent, params.profile); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(status, null, 2)); + if (printJsonResult(parent, status)) { return; } const name = status.profile ?? "openclaw"; @@ -82,8 +110,7 @@ export function registerBrowserManageCommands( const parent = parentOpts(cmd); await runBrowserCommand(async () => { const status = await fetchBrowserStatus(parent, parent?.browserProfile); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(status, null, 2)); + if (printJsonResult(parent, status)) { return; } const detectedPath = status.detectedExecutablePath ?? status.executablePath; @@ -139,12 +166,11 @@ export function registerBrowserManageCommands( { method: "POST", path: "/reset-profile", - query: profile ? { profile } : undefined, + query: resolveProfileQuery(profile), }, { timeoutMs: 20000 }, ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); + if (printJsonResult(parent, result)) { return; } if (!result.moved) { @@ -168,7 +194,7 @@ export function registerBrowserManageCommands( { method: "GET", path: "/tabs", - query: profile ? { profile } : undefined, + query: resolveProfileQuery(profile), }, { timeoutMs: 3000 }, ); @@ -189,7 +215,7 @@ export function registerBrowserManageCommands( { method: "POST", path: "/tabs/action", - query: profile ? { profile } : undefined, + query: resolveProfileQuery(profile), body: { action: "list", }, @@ -208,18 +234,8 @@ export function registerBrowserManageCommands( const parent = parentOpts(cmd); const profile = parent?.browserProfile; await runBrowserCommand(async () => { - const result = await callBrowserRequest( - parent, - { - method: "POST", - path: "/tabs/action", - query: profile ? { profile } : undefined, - body: { action: "new" }, - }, - { timeoutMs: 10_000 }, - ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); + const result = await callTabAction(parent, profile, { action: "new" }); + if (printJsonResult(parent, result)) { return; } defaultRuntime.log("opened new tab"); @@ -239,18 +255,11 @@ export function registerBrowserManageCommands( return; } await runBrowserCommand(async () => { - const result = await callBrowserRequest( - parent, - { - method: "POST", - path: "/tabs/action", - query: profile ? { profile } : undefined, - body: { action: "select", index: Math.floor(index) - 1 }, - }, - { timeoutMs: 10_000 }, - ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); + const result = await callTabAction(parent, profile, { + action: "select", + index: Math.floor(index) - 1, + }); + if (printJsonResult(parent, result)) { return; } defaultRuntime.log(`selected tab ${Math.floor(index)}`); @@ -272,18 +281,8 @@ export function registerBrowserManageCommands( return; } await runBrowserCommand(async () => { - const result = await callBrowserRequest( - parent, - { - method: "POST", - path: "/tabs/action", - query: profile ? { profile } : undefined, - body: { action: "close", index: idx }, - }, - { timeoutMs: 10_000 }, - ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); + const result = await callTabAction(parent, profile, { action: "close", index: idx }); + if (printJsonResult(parent, result)) { return; } defaultRuntime.log("closed tab"); @@ -303,13 +302,12 @@ export function registerBrowserManageCommands( { method: "POST", path: "/tabs/open", - query: profile ? { profile } : undefined, + query: resolveProfileQuery(profile), body: { url }, }, { timeoutMs: 15000 }, ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(tab, null, 2)); + if (printJsonResult(parent, tab)) { return; } defaultRuntime.log(`opened: ${tab.url}\nid: ${tab.targetId}`); @@ -329,13 +327,12 @@ export function registerBrowserManageCommands( { method: "POST", path: "/tabs/focus", - query: profile ? { profile } : undefined, + query: resolveProfileQuery(profile), body: { targetId }, }, { timeoutMs: 5000 }, ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify({ ok: true }, null, 2)); + if (printJsonResult(parent, { ok: true })) { return; } defaultRuntime.log(`focused tab ${targetId}`); @@ -356,7 +353,7 @@ export function registerBrowserManageCommands( { method: "DELETE", path: `/tabs/${encodeURIComponent(targetId.trim())}`, - query: profile ? { profile } : undefined, + query: resolveProfileQuery(profile), }, { timeoutMs: 5000 }, ); @@ -366,14 +363,13 @@ export function registerBrowserManageCommands( { method: "POST", path: "/act", - query: profile ? { profile } : undefined, + query: resolveProfileQuery(profile), body: { kind: "close" }, }, { timeoutMs: 20000 }, ); } - if (parent?.json) { - defaultRuntime.log(JSON.stringify({ ok: true }, null, 2)); + if (printJsonResult(parent, { ok: true })) { return; } defaultRuntime.log("closed tab"); @@ -396,8 +392,7 @@ export function registerBrowserManageCommands( { timeoutMs: 3000 }, ); const profiles = result.profiles ?? []; - if (parent?.json) { - defaultRuntime.log(JSON.stringify({ profiles }, null, 2)); + if (printJsonResult(parent, { profiles })) { return; } if (profiles.length === 0) { @@ -444,8 +439,7 @@ export function registerBrowserManageCommands( }, { timeoutMs: 10_000 }, ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); + if (printJsonResult(parent, result)) { return; } const loc = result.isRemote ? ` cdpUrl: ${result.cdpUrl}` : ` port: ${result.cdpPort}`; @@ -475,8 +469,7 @@ export function registerBrowserManageCommands( }, { timeoutMs: 20_000 }, ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); + if (printJsonResult(parent, result)) { return; } const msg = result.deleted diff --git a/src/cli/nodes-cli.coverage.test.ts b/src/cli/nodes-cli.coverage.test.ts index 3a10b43b71c..686a5a0e860 100644 --- a/src/cli/nodes-cli.coverage.test.ts +++ b/src/cli/nodes-cli.coverage.test.ts @@ -1,5 +1,6 @@ import { Command } from "commander"; import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { buildSystemRunPreparePayload } from "../test-utils/system-run-prepare-payload.js"; import { createCliRuntimeCapture } from "./test-runtime-capture.js"; type NodeInvokeCall = { @@ -40,25 +41,7 @@ const callGateway = vi.fn(async (opts: NodeInvokeCall) => { cwd?: unknown; agentId?: unknown; }; - const argv = Array.isArray(params.command) - ? params.command.map((entry) => String(entry)) - : []; - const rawCommand = - typeof params.rawCommand === "string" && params.rawCommand.trim().length > 0 - ? params.rawCommand - : null; - return { - payload: { - cmdText: rawCommand ?? argv.join(" "), - plan: { - argv, - cwd: typeof params.cwd === "string" ? params.cwd : null, - rawCommand, - agentId: typeof params.agentId === "string" ? params.agentId : null, - sessionKey: null, - }, - }, - }; + return buildSystemRunPreparePayload(params); } return { payload: { diff --git a/src/plugin-sdk/keyed-async-queue.test.ts b/src/plugin-sdk/keyed-async-queue.test.ts index 50038f5bc93..081225e840c 100644 --- a/src/plugin-sdk/keyed-async-queue.test.ts +++ b/src/plugin-sdk/keyed-async-queue.test.ts @@ -2,12 +2,7 @@ import { describe, expect, it, vi } from "vitest"; import { enqueueKeyedTask, KeyedAsyncQueue } from "./keyed-async-queue.js"; function deferred() { - let resolve!: (value: T | PromiseLike) => void; - let reject!: (reason?: unknown) => void; - const promise = new Promise((res, rej) => { - resolve = res; - reject = rej; - }); + const { promise, resolve, reject } = Promise.withResolvers(); return { promise, resolve, reject }; } diff --git a/src/providers/google-shared.ensures-function-call-comes-after-user-turn.test.ts b/src/providers/google-shared.ensures-function-call-comes-after-user-turn.test.ts index 9f209f3b082..888496fbd96 100644 --- a/src/providers/google-shared.ensures-function-call-comes-after-user-turn.test.ts +++ b/src/providers/google-shared.ensures-function-call-comes-after-user-turn.test.ts @@ -3,6 +3,7 @@ import type { Context } from "@mariozechner/pi-ai/dist/types.js"; import { describe, expect, it } from "vitest"; import { asRecord, + expectConvertedRoles, makeGeminiCliAssistantMessage, makeGeminiCliModel, makeGoogleAssistantMessage, @@ -31,10 +32,7 @@ describe("google-shared convertTools", () => { } as unknown as Context; const contents = convertMessages(model, context); - expect(contents).toHaveLength(3); - expect(contents[0].role).toBe("user"); - expect(contents[1].role).toBe("model"); - expect(contents[2].role).toBe("model"); + expectConvertedRoles(contents, ["user", "model", "model"]); const toolCallPart = contents[2].parts?.find( (part) => typeof part === "object" && part !== null && "functionCall" in part, ); diff --git a/src/providers/google-shared.preserves-parameters-type-is-missing.test.ts b/src/providers/google-shared.preserves-parameters-type-is-missing.test.ts index 3dc27a4c2a0..95f7c155b58 100644 --- a/src/providers/google-shared.preserves-parameters-type-is-missing.test.ts +++ b/src/providers/google-shared.preserves-parameters-type-is-missing.test.ts @@ -3,6 +3,7 @@ import type { Context, Tool } from "@mariozechner/pi-ai/dist/types.js"; import { describe, expect, it } from "vitest"; import { asRecord, + expectConvertedRoles, getFirstToolParameters, makeGoogleAssistantMessage, makeModel, @@ -232,10 +233,7 @@ describe("google-shared convertMessages", () => { } as unknown as Context; const contents = convertMessages(model, context); - expect(contents).toHaveLength(3); - expect(contents[0].role).toBe("user"); - expect(contents[1].role).toBe("model"); - expect(contents[2].role).toBe("model"); + expectConvertedRoles(contents, ["user", "model", "model"]); expect(contents[1].parts).toHaveLength(1); expect(contents[2].parts).toHaveLength(1); }); diff --git a/src/providers/google-shared.test-helpers.ts b/src/providers/google-shared.test-helpers.ts index 80839ae6085..6867f879617 100644 --- a/src/providers/google-shared.test-helpers.ts +++ b/src/providers/google-shared.test-helpers.ts @@ -74,3 +74,10 @@ export function makeGeminiCliAssistantMessage(model: string, content: unknown) { timestamp: 0, }; } + +export function expectConvertedRoles(contents: Array<{ role?: string }>, expectedRoles: string[]) { + expect(contents).toHaveLength(expectedRoles.length); + for (const [index, role] of expectedRoles.entries()) { + expect(contents[index]?.role).toBe(role); + } +} diff --git a/src/test-utils/system-run-prepare-payload.ts b/src/test-utils/system-run-prepare-payload.ts new file mode 100644 index 00000000000..26fea1609ce --- /dev/null +++ b/src/test-utils/system-run-prepare-payload.ts @@ -0,0 +1,27 @@ +type SystemRunPrepareInput = { + command?: unknown; + rawCommand?: unknown; + cwd?: unknown; + agentId?: unknown; + sessionKey?: unknown; +}; + +export function buildSystemRunPreparePayload(params: SystemRunPrepareInput) { + const argv = Array.isArray(params.command) ? params.command.map(String) : []; + const rawCommand = + typeof params.rawCommand === "string" && params.rawCommand.trim().length > 0 + ? params.rawCommand + : null; + return { + payload: { + cmdText: rawCommand ?? argv.join(" "), + plan: { + argv, + cwd: typeof params.cwd === "string" ? params.cwd : null, + rawCommand, + agentId: typeof params.agentId === "string" ? params.agentId : null, + sessionKey: typeof params.sessionKey === "string" ? params.sessionKey : null, + }, + }, + }; +}