From 9e27d04dc3af246277fa8b3330717d0f0f92fe6e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 18 Apr 2026 19:50:56 +0100 Subject: [PATCH] test: narrow agent control mocks --- src/agents/acp-spawn.test.ts | 126 +++++++++--------- src/agents/context.eager-warmup.test.ts | 7 +- src/agents/live-model-switch.test.ts | 14 +- ...s.subagents.sessions-spawn.test-harness.ts | 31 ++++- src/agents/subagent-control.test.ts | 92 ++++++++++++- src/agents/subagent-control.ts | 5 +- .../subagent-registry.steer-restart.test.ts | 2 + 7 files changed, 198 insertions(+), 79 deletions(-) diff --git a/src/agents/acp-spawn.test.ts b/src/agents/acp-spawn.test.ts index 6bdb7c00ee9..2a56c7a6f8c 100644 --- a/src/agents/acp-spawn.test.ts +++ b/src/agents/acp-spawn.test.ts @@ -2,19 +2,12 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import * as acpSessionManager from "../acp/control-plane/manager.js"; import type { AcpInitializeSessionInput } from "../acp/control-plane/manager.types.js"; -import * as channelPlugins from "../channels/plugins/index.js"; import { clearRuntimeConfigSnapshot, setRuntimeConfigSnapshot, type OpenClawConfig, } from "../config/config.js"; -import * as sessionPaths from "../config/sessions/paths.js"; -import * as sessionStore from "../config/sessions/store.js"; -import * as sessionTranscript from "../config/sessions/transcript.js"; -import * as gatewayCall from "../gateway/call.js"; -import * as heartbeatWake from "../infra/heartbeat-wake.js"; import { __testing as sessionBindingServiceTesting, registerSessionBindingAdapter, @@ -22,8 +15,6 @@ import { type SessionBindingPlacement, type SessionBindingRecord, } from "../infra/outbound/session-binding-service.js"; -import { resetTaskRegistryForTests } from "../tasks/task-registry.js"; -import * as acpSpawnParentStream from "./acp-spawn-parent-stream.js"; function createDefaultSpawnConfig(): OpenClawConfig { return { @@ -55,12 +46,21 @@ const hoisted = vi.hoisted(() => { const sessionBindingListBySessionMock = vi.fn(); const closeSessionMock = vi.fn(); const initializeSessionMock = vi.fn(); + const getAcpSessionManagerMock = vi.fn(); const startAcpSpawnParentStreamRelayMock = vi.fn(); const resolveAcpSpawnStreamLogPathMock = vi.fn(); const loadSessionStoreMock = vi.fn(); const resolveStorePathMock = vi.fn(); const resolveSessionTranscriptFileMock = vi.fn(); const areHeartbeatsEnabledMock = vi.fn(); + const getChannelPluginMock = vi.fn(); + const getLoadedChannelPluginMock = vi.fn(); + const normalizeChannelIdMock = vi.fn((channelId: string) => { + const normalized = channelId.trim().toLowerCase(); + return normalized || null; + }); + const cleanupFailedAcpSpawnMock = vi.fn(); + const createRunningTaskRunMock = vi.fn(); const state = { cfg: createDefaultSpawnConfig(), }; @@ -72,31 +72,64 @@ const hoisted = vi.hoisted(() => { sessionBindingListBySessionMock, closeSessionMock, initializeSessionMock, + getAcpSessionManagerMock, startAcpSpawnParentStreamRelayMock, resolveAcpSpawnStreamLogPathMock, loadSessionStoreMock, resolveStorePathMock, resolveSessionTranscriptFileMock, areHeartbeatsEnabledMock, + getChannelPluginMock, + getLoadedChannelPluginMock, + normalizeChannelIdMock, + cleanupFailedAcpSpawnMock, + createRunningTaskRunMock, state, }; }); -const callGatewaySpy = vi.spyOn(gatewayCall, "callGateway"); -const getChannelPluginSpy = vi.spyOn(channelPlugins, "getChannelPlugin"); -const getAcpSessionManagerSpy = vi.spyOn(acpSessionManager, "getAcpSessionManager"); -const loadSessionStoreSpy = vi.spyOn(sessionStore, "loadSessionStore"); -const resolveStorePathSpy = vi.spyOn(sessionPaths, "resolveStorePath"); -const resolveSessionTranscriptFileSpy = vi.spyOn(sessionTranscript, "resolveSessionTranscriptFile"); -const areHeartbeatsEnabledSpy = vi.spyOn(heartbeatWake, "areHeartbeatsEnabled"); -const startAcpSpawnParentStreamRelaySpy = vi.spyOn( - acpSpawnParentStream, - "startAcpSpawnParentStreamRelay", -); -const resolveAcpSpawnStreamLogPathSpy = vi.spyOn( - acpSpawnParentStream, - "resolveAcpSpawnStreamLogPath", -); +vi.mock("../acp/control-plane/manager.js", () => ({ + getAcpSessionManager: hoisted.getAcpSessionManagerMock, +})); + +vi.mock("../acp/control-plane/spawn.js", () => ({ + cleanupFailedAcpSpawn: hoisted.cleanupFailedAcpSpawnMock, +})); + +vi.mock("../channels/plugins/index.js", () => ({ + getChannelPlugin: hoisted.getChannelPluginMock, + getLoadedChannelPlugin: hoisted.getLoadedChannelPluginMock, + normalizeChannelId: hoisted.normalizeChannelIdMock, +})); + +vi.mock("../config/sessions/paths.js", () => ({ + resolveStorePath: hoisted.resolveStorePathMock, +})); + +vi.mock("../config/sessions/store.js", () => ({ + loadSessionStore: hoisted.loadSessionStoreMock, +})); + +vi.mock("../config/sessions/transcript.js", () => ({ + resolveSessionTranscriptFile: hoisted.resolveSessionTranscriptFileMock, +})); + +vi.mock("../gateway/call.js", () => ({ + callGateway: hoisted.callGatewayMock, +})); + +vi.mock("../infra/heartbeat-wake.js", () => ({ + areHeartbeatsEnabled: hoisted.areHeartbeatsEnabledMock, +})); + +vi.mock("../tasks/task-executor.js", () => ({ + createRunningTaskRun: hoisted.createRunningTaskRunMock, +})); + +vi.mock("./acp-spawn-parent-stream.js", () => ({ + resolveAcpSpawnStreamLogPath: hoisted.resolveAcpSpawnStreamLogPathMock, + startAcpSpawnParentStreamRelay: hoisted.startAcpSpawnParentStreamRelayMock, +})); const { isSpawnAcpAcceptedResult, spawnAcpDirect } = await import("./acp-spawn.js"); type SpawnRequest = Parameters[0]; @@ -350,9 +383,11 @@ function enableTelegramCurrentConversationBindings(): void { describe("spawnAcpDirect", () => { beforeEach(() => { replaceSpawnConfig(createDefaultSpawnConfig()); - resetTaskRegistryForTests(); hoisted.areHeartbeatsEnabledMock.mockReset().mockReturnValue(true); - getChannelPluginSpy.mockReset().mockReturnValue(undefined); + hoisted.getChannelPluginMock.mockReset().mockReturnValue(undefined); + hoisted.getLoadedChannelPluginMock.mockReset().mockReturnValue(undefined); + hoisted.cleanupFailedAcpSpawnMock.mockReset().mockResolvedValue(undefined); + hoisted.createRunningTaskRunMock.mockReset().mockReturnValue(undefined); hoisted.callGatewayMock.mockReset(); hoisted.callGatewayMock.mockImplementation(async (argsUnknown: unknown) => { @@ -368,26 +403,16 @@ describe("spawnAcpDirect", () => { } return {}; }); - callGatewaySpy.mockReset().mockImplementation(async (argsUnknown: unknown) => { - return await hoisted.callGatewayMock(argsUnknown); - }); hoisted.closeSessionMock.mockReset().mockResolvedValue({ runtimeClosed: true, metaCleared: false, }); - getAcpSessionManagerSpy.mockReset().mockReturnValue({ - initializeSession: async ( - params: Parameters< - ReturnType["initializeSession"] - >[0], - ) => await hoisted.initializeSessionMock(params), - closeSession: async ( - params: Parameters< - ReturnType["closeSession"] - >[0], - ) => await hoisted.closeSessionMock(params), - } as unknown as ReturnType); + hoisted.getAcpSessionManagerMock.mockReset().mockReturnValue({ + initializeSession: async (params: AcpInitializeSessionInput) => + await hoisted.initializeSessionMock(params), + closeSession: async (params: unknown) => await hoisted.closeSessionMock(params), + }); hoisted.initializeSessionMock.mockReset().mockImplementation(async (argsUnknown: unknown) => { const args = argsUnknown as AcpInitializeSessionInput; const runtimeSessionName = `${args.sessionKey}:runtime`; @@ -464,19 +489,10 @@ describe("spawnAcpDirect", () => { hoisted.startAcpSpawnParentStreamRelayMock .mockReset() .mockImplementation(() => createRelayHandle()); - startAcpSpawnParentStreamRelaySpy - .mockReset() - .mockImplementation((...args) => hoisted.startAcpSpawnParentStreamRelayMock(...args)); hoisted.resolveAcpSpawnStreamLogPathMock .mockReset() .mockReturnValue("/tmp/sess-main.acp-stream.jsonl"); - resolveAcpSpawnStreamLogPathSpy - .mockReset() - .mockImplementation((...args) => hoisted.resolveAcpSpawnStreamLogPathMock(...args)); hoisted.resolveStorePathMock.mockReset().mockReturnValue("/tmp/codex-sessions.json"); - resolveStorePathSpy - .mockReset() - .mockImplementation((store, opts) => hoisted.resolveStorePathMock(store, opts)); hoisted.loadSessionStoreMock.mockReset().mockImplementation(() => { const store: Record = {}; return new Proxy(store, { @@ -488,9 +504,6 @@ describe("spawnAcpDirect", () => { }, }); }); - loadSessionStoreSpy - .mockReset() - .mockImplementation((storePath) => hoisted.loadSessionStoreMock(storePath)); hoisted.resolveSessionTranscriptFileMock .mockReset() .mockImplementation(async (params: unknown) => { @@ -507,16 +520,9 @@ describe("spawnAcpDirect", () => { }, }; }); - resolveSessionTranscriptFileSpy - .mockReset() - .mockImplementation(async (params) => await hoisted.resolveSessionTranscriptFileMock(params)); - areHeartbeatsEnabledSpy - .mockReset() - .mockImplementation(() => hoisted.areHeartbeatsEnabledMock()); }); afterEach(() => { - resetTaskRegistryForTests(); sessionBindingServiceTesting.resetSessionBindingAdaptersForTests(); clearRuntimeConfigSnapshot(); }); diff --git a/src/agents/context.eager-warmup.test.ts b/src/agents/context.eager-warmup.test.ts index 919b77576d8..6be84291bcf 100644 --- a/src/agents/context.eager-warmup.test.ts +++ b/src/agents/context.eager-warmup.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { importFreshModule } from "../../test/helpers/import-fresh.js"; const loadConfigMock = vi.hoisted(() => vi.fn()); @@ -20,16 +21,14 @@ describe("agents/context eager warmup", () => { ["agent", ["node", "openclaw", "agent", "--message", "ok"]], ])("does not eager-load config for %s commands on import", async (_label, argv) => { process.argv = argv; - vi.resetModules(); - await import("./context.js"); + await importFreshModule(import.meta.url, `./context.js?scope=${_label}`); expect(loadConfigMock).not.toHaveBeenCalled(); }); it("does not eager-load config when plugin-sdk command-auth is imported", async () => { process.argv = ["node", "openclaw", "onboard"]; - vi.resetModules(); - await import("../plugin-sdk/command-auth.js"); + await importFreshModule(import.meta.url, "../plugin-sdk/command-auth.js?scope=onboard"); expect(loadConfigMock).not.toHaveBeenCalled(); }); diff --git a/src/agents/live-model-switch.test.ts b/src/agents/live-model-switch.test.ts index fcb466998d0..882f101c463 100644 --- a/src/agents/live-model-switch.test.ts +++ b/src/agents/live-model-switch.test.ts @@ -1,5 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; -import { importFreshModule } from "../../test/helpers/import-fresh.js"; +import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; const state = vi.hoisted(() => ({ abortEmbeddedPiRunMock: vi.fn(), @@ -65,14 +64,17 @@ vi.mock("../config/sessions.js", () => ({ updateSessionStore: (...args: unknown[]) => state.updateSessionStoreMock(...args), })); +let mod: typeof import("./live-model-switch.js"); + async function loadModule() { - return await importFreshModule( - import.meta.url, - `./live-model-switch.js?scope=${Math.random().toString(36).slice(2)}`, - ); + return mod; } describe("live model switch", () => { + beforeAll(async () => { + mod = await import("./live-model-switch.js"); + }); + beforeEach(() => { state.abortEmbeddedPiRunMock.mockReset().mockReturnValue(false); state.requestEmbeddedRunModelSwitchMock.mockReset(); diff --git a/src/agents/openclaw-tools.subagents.sessions-spawn.test-harness.ts b/src/agents/openclaw-tools.subagents.sessions-spawn.test-harness.ts index 2e82f3ac9f3..c1b0ee313ec 100644 --- a/src/agents/openclaw-tools.subagents.sessions-spawn.test-harness.ts +++ b/src/agents/openclaw-tools.subagents.sessions-spawn.test-harness.ts @@ -9,6 +9,8 @@ type CaptureSubagentCompletionReply = type RunSubagentAnnounceFlow = (typeof import("./subagent-announce.js"))["runSubagentAnnounceFlow"]; type CreateSessionsSpawnTool = (typeof import("./tools/sessions-spawn-tool.js"))["createSessionsSpawnTool"]; +type SubagentRegistryTesting = (typeof import("./subagent-registry.js"))["__testing"]; +type SubagentSpawnTesting = (typeof import("./subagent-spawn.js"))["__testing"]; export type CreateOpenClawToolsOpts = Parameters[0]; export type GatewayRequest = { method?: string; params?: unknown }; export type AgentWaitCall = { runId?: string; timeoutMs?: number }; @@ -101,6 +103,8 @@ const hoisted = vi.hoisted(() => { }); let cachedCreateSessionsSpawnTool: CreateSessionsSpawnTool | null = null; +let cachedSubagentRegistryTesting: SubagentRegistryTesting | null = null; +let cachedSubagentSpawnTesting: SubagentSpawnTesting | null = null; export function getCallGatewayMock(): Mock { return hoisted.callGatewayMock; @@ -143,18 +147,30 @@ export function setSessionsSpawnAnnounceFlowOverride(next: RunSubagentAnnounceFl } export async function getSessionsSpawnTool(opts: CreateOpenClawToolsOpts) { - const [{ __testing: subagentSpawnTesting }, { __testing: subagentRegistryTesting }] = - await Promise.all([import("./subagent-spawn.js"), import("./subagent-registry.js")]); - subagentSpawnTesting.setDepsForTest({ + if (!cachedSubagentSpawnTesting || !cachedSubagentRegistryTesting) { + const [{ __testing: subagentSpawnTesting }, { __testing: subagentRegistryTesting }] = + await Promise.all([import("./subagent-spawn.js"), import("./subagent-registry.js")]); + cachedSubagentSpawnTesting = subagentSpawnTesting; + cachedSubagentRegistryTesting = subagentRegistryTesting; + } + cachedSubagentSpawnTesting.setDepsForTest({ callGateway: (optsUnknown) => hoisted.callGatewayMock(optsUnknown), getGlobalHookRunner: () => hoisted.state.hookRunnerOverride, loadConfig: () => hoisted.state.configOverride, updateSessionStore: async (_storePath, mutator) => mutator({}), }); - subagentRegistryTesting.setDepsForTest({ + cachedSubagentRegistryTesting.setDepsForTest({ callGateway: (optsUnknown) => hoisted.callGatewayMock(optsUnknown), loadConfig: () => hoisted.state.configOverride, cleanupBrowserSessionsForLifecycleEnd: async () => {}, + ensureContextEnginesInitialized: () => {}, + ensureRuntimePluginsLoaded: () => {}, + resolveContextEngine: async () => ({ + info: { id: "test", name: "Test" }, + assemble: async ({ messages }) => ({ messages, estimatedTokens: 0 }), + compact: async () => ({ ok: true, compacted: false }), + ingest: async () => ({ ingested: false }), + }), captureSubagentCompletionReply: (sessionKey) => hoisted.state.captureSubagentCompletionReplyOverride(sessionKey), runSubagentAnnounceFlow: (params) => hoisted.state.runSubagentAnnounceFlowOverride(params), @@ -291,6 +307,13 @@ vi.mock("../config/sessions.js", () => ({ }, })); +vi.mock("../tasks/task-executor.js", () => ({ + completeTaskRunByRunId: vi.fn(), + createRunningTaskRun: vi.fn(), + failTaskRunByRunId: vi.fn(), + setDetachedTaskDeliveryStatusByRunId: vi.fn(), +})); + // Same module, different specifier (used by tools under src/agents/tools/*). vi.mock("../../config/config.js", () => ({ loadConfig: () => hoisted.state.configOverride, diff --git a/src/agents/subagent-control.test.ts b/src/agents/subagent-control.test.ts index 4e42309beb2..1a69b686454 100644 --- a/src/agents/subagent-control.test.ts +++ b/src/agents/subagent-control.test.ts @@ -2,8 +2,8 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import type { OpenClawConfig } from "../config/config.js"; -import * as sessions from "../config/sessions.js"; +import * as sessionStore from "../config/sessions/store.js"; +import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { CallGatewayOptions } from "../gateway/call.js"; import { __testing, @@ -19,6 +19,92 @@ import { resetSubagentRegistryForTests, } from "./subagent-registry.js"; +vi.mock("../gateway/call.js", () => ({ + callGateway: vi.fn(), +})); + +vi.mock("./run-wait.js", () => { + const readLatestAssistantReplySnapshot = async (params: { + sessionKey: string; + limit?: number; + callGateway?: (request: CallGatewayOptions) => Promise<{ messages?: unknown[] }>; + }) => { + const history = await params.callGateway?.({ + method: "chat.history", + params: { sessionKey: params.sessionKey, limit: params.limit ?? 50 }, + }); + const messages = Array.isArray(history?.messages) ? history.messages : []; + for (let i = messages.length - 1; i >= 0; i -= 1) { + const message = messages[i]; + if (!message || typeof message !== "object") { + continue; + } + if ((message as { role?: unknown }).role !== "assistant") { + continue; + } + const content = (message as { content?: unknown }).content; + const text = Array.isArray(content) + ? content + .map((block) => + block && + typeof block === "object" && + typeof (block as { text?: unknown }).text === "string" + ? (block as { text: string }).text + : "", + ) + .filter(Boolean) + .join("\n") + : typeof content === "string" + ? content + : ""; + if (text.trim()) { + return { text, fingerprint: JSON.stringify(message) }; + } + } + return {}; + }; + + return { + readLatestAssistantReplySnapshot, + waitForAgentRunAndReadUpdatedAssistantReply: async (params: { + runId: string; + sessionKey: string; + timeoutMs: number; + limit?: number; + baseline?: { fingerprint?: string }; + callGateway?: (request: CallGatewayOptions) => Promise>; + }) => { + const wait = await params.callGateway?.({ + method: "agent.wait", + params: { + runId: params.runId, + timeoutMs: Math.max(1, Math.floor(params.timeoutMs)), + }, + timeoutMs: Math.max(1, Math.floor(params.timeoutMs)) + 2000, + }); + const status = wait?.status; + if (status === "timeout" || status === "pending" || status === "error") { + return { status, error: typeof wait?.error === "string" ? wait.error : undefined }; + } + const latestReply = await readLatestAssistantReplySnapshot({ + sessionKey: params.sessionKey, + limit: params.limit, + callGateway: params.callGateway as + | ((request: CallGatewayOptions) => Promise<{ messages?: unknown[] }>) + | undefined, + }); + return { + status: "ok", + replyText: + latestReply.text && + (!params.baseline?.fingerprint || latestReply.fingerprint !== params.baseline.fingerprint) + ? latestReply.text + : undefined, + }; + }, + }; +}); + function setSubagentControlDepsForTest( overrides: Parameters[0] = {}, ) { @@ -518,7 +604,7 @@ describe("killSubagentRunAdmin", () => { }); const updateSessionStoreSpy = vi - .spyOn(sessions, "updateSessionStore") + .spyOn(sessionStore, "updateSessionStore") .mockRejectedValueOnce(new Error("session store unavailable")); try { diff --git a/src/agents/subagent-control.ts b/src/agents/subagent-control.ts index 48b4e499484..50f4e14214f 100644 --- a/src/agents/subagent-control.ts +++ b/src/agents/subagent-control.ts @@ -6,8 +6,9 @@ import { sortSubagentRuns, type SubagentTargetResolution, } from "../auto-reply/reply/subagents-utils.js"; -import type { SessionEntry } from "../config/sessions.js"; -import { loadSessionStore, resolveStorePath, updateSessionStore } from "../config/sessions.js"; +import { resolveStorePath } from "../config/sessions/paths.js"; +import { loadSessionStore, updateSessionStore } from "../config/sessions/store.js"; +import type { SessionEntry } from "../config/sessions/types.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import { callGateway } from "../gateway/call.js"; import { logVerbose } from "../globals.js"; diff --git a/src/agents/subagent-registry.steer-restart.test.ts b/src/agents/subagent-registry.steer-restart.test.ts index 2da4d01a878..be0ee7a0afe 100644 --- a/src/agents/subagent-registry.steer-restart.test.ts +++ b/src/agents/subagent-registry.steer-restart.test.ts @@ -106,6 +106,7 @@ describe("subagent registry steer restarts", () => { }); beforeEach(() => { + vi.useRealTimers(); lifecycleHandler = undefined; announceSpy.mockReset(); announceSpy.mockResolvedValue(true); @@ -242,6 +243,7 @@ describe("subagent registry steer restarts", () => { }; afterEach(async () => { + vi.useRealTimers(); announceSpy.mockReset(); announceSpy.mockResolvedValue(true); runSubagentEndedHookMock.mockReset();