From c2f7b66d22adff7b2010fa65d35d3c3293138fc0 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Feb 2026 16:04:43 +0000 Subject: [PATCH] perf(test): replace module resets with direct spies and runtime seams --- src/agents/sandbox-skills.e2e.test.ts | 59 ++++------------- src/agents/workspace.defaults.e2e.test.ts | 8 +-- src/cli/gateway-cli.coverage.e2e.test.ts | 2 - ...tion.rejects-routing-allowfrom.e2e.test.ts | 4 -- src/config/paths.test.ts | 65 ++----------------- src/hooks/hooks-install.e2e.test.ts | 52 +++++++-------- src/hooks/loader.ts | 10 ++- src/plugins/install.e2e.test.ts | 20 ++---- src/plugins/install.ts | 4 +- src/security/audit-extra.async.ts | 31 +++++---- src/security/audit.test.ts | 20 ++---- 11 files changed, 80 insertions(+), 195 deletions(-) diff --git a/src/agents/sandbox-skills.e2e.test.ts b/src/agents/sandbox-skills.e2e.test.ts index 80fe6ce650..ae37f2a9fe 100644 --- a/src/agents/sandbox-skills.e2e.test.ts +++ b/src/agents/sandbox-skills.e2e.test.ts @@ -1,49 +1,21 @@ -import { EventEmitter } from "node:events"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { Readable } from "node:stream"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../config/config.js"; +import { resolveSandboxContext } from "./sandbox.js"; -type SpawnCall = { - command: string; - args: string[]; -}; +vi.mock("./sandbox/docker.js", () => ({ + ensureSandboxContainer: vi.fn(async () => "openclaw-sbx-test"), +})); -const spawnCalls: SpawnCall[] = []; +vi.mock("./sandbox/browser.js", () => ({ + ensureSandboxBrowser: vi.fn(async () => null), +})); -vi.mock("node:child_process", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - spawn: (command: string, args: string[]) => { - spawnCalls.push({ command, args }); - const child = new EventEmitter() as { - stdout?: Readable; - stderr?: Readable; - on: (event: string, cb: (...args: unknown[]) => void) => void; - }; - child.stdout = new Readable({ read() {} }); - child.stderr = new Readable({ read() {} }); - - const dockerArgs = command === "docker" ? args : []; - const shouldFailContainerInspect = - dockerArgs[0] === "inspect" && - dockerArgs[1] === "-f" && - dockerArgs[2] === "{{.State.Running}}"; - const shouldSucceedImageInspect = dockerArgs[0] === "image" && dockerArgs[1] === "inspect"; - - const code = shouldFailContainerInspect ? 1 : 0; - if (shouldSucceedImageInspect) { - queueMicrotask(() => child.emit("close", 0)); - } else { - queueMicrotask(() => child.emit("close", code)); - } - return child; - }, - }; -}); +vi.mock("./sandbox/prune.js", () => ({ + maybePruneSandboxes: vi.fn(async () => undefined), +})); async function writeSkill(params: { dir: string; name: string; description: string }) { const { dir, name, description } = params; @@ -74,25 +46,18 @@ describe("sandbox skill mirroring", () => { let envSnapshot: Record; beforeEach(() => { - spawnCalls.length = 0; envSnapshot = { ...process.env }; }); afterEach(() => { restoreEnv(envSnapshot); - vi.resetModules(); }); const runContext = async (workspaceAccess: "none" | "ro") => { - const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-state-")); - const bundledDir = path.join(stateDir, "bundled-skills"); + const bundledDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-bundled-skills-")); await fs.mkdir(bundledDir, { recursive: true }); - process.env.OPENCLAW_STATE_DIR = stateDir; process.env.OPENCLAW_BUNDLED_SKILLS_DIR = bundledDir; - vi.resetModules(); - - const { resolveSandboxContext } = await import("./sandbox.js"); const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-workspace-")); await writeSkill({ @@ -108,7 +73,7 @@ describe("sandbox skill mirroring", () => { mode: "all", scope: "session", workspaceAccess, - workspaceRoot: path.join(stateDir, "sandboxes"), + workspaceRoot: path.join(bundledDir, "sandboxes"), }, }, }, diff --git a/src/agents/workspace.defaults.e2e.test.ts b/src/agents/workspace.defaults.e2e.test.ts index 58bf14ccc9..492af363c7 100644 --- a/src/agents/workspace.defaults.e2e.test.ts +++ b/src/agents/workspace.defaults.e2e.test.ts @@ -1,20 +1,18 @@ import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { resolveDefaultAgentWorkspaceDir } from "./workspace.js"; afterEach(() => { vi.unstubAllEnvs(); - vi.resetModules(); }); describe("DEFAULT_AGENT_WORKSPACE_DIR", () => { - it("uses OPENCLAW_HOME at module import time", async () => { + it("uses OPENCLAW_HOME when resolving the default workspace dir", () => { const home = path.join(path.sep, "srv", "openclaw-home"); vi.stubEnv("OPENCLAW_HOME", home); vi.stubEnv("HOME", path.join(path.sep, "home", "other")); - vi.resetModules(); - const mod = await import("./workspace.js"); - expect(mod.DEFAULT_AGENT_WORKSPACE_DIR).toBe( + expect(resolveDefaultAgentWorkspaceDir()).toBe( path.join(path.resolve(home), ".openclaw", "workspace"), ); }); diff --git a/src/cli/gateway-cli.coverage.e2e.test.ts b/src/cli/gateway-cli.coverage.e2e.test.ts index da8cc5f593..b9d26804dc 100644 --- a/src/cli/gateway-cli.coverage.e2e.test.ts +++ b/src/cli/gateway-cli.coverage.e2e.test.ts @@ -38,7 +38,6 @@ async function withEnvOverride( process.env[key] = overrides[key]; } } - vi.resetModules(); try { return await fn(); } finally { @@ -49,7 +48,6 @@ async function withEnvOverride( process.env[key] = saved[key]; } } - vi.resetModules(); } } diff --git a/src/config/config.legacy-config-detection.rejects-routing-allowfrom.e2e.test.ts b/src/config/config.legacy-config-detection.rejects-routing-allowfrom.e2e.test.ts index 6b74d77676..c2bd040504 100644 --- a/src/config/config.legacy-config-detection.rejects-routing-allowfrom.e2e.test.ts +++ b/src/config/config.legacy-config-detection.rejects-routing-allowfrom.e2e.test.ts @@ -120,8 +120,6 @@ describe("legacy config detection", () => { expect(res.config?.routing).toBeUndefined(); }); it("migrates audio.transcription with custom script names", async () => { - vi.resetModules(); - const { migrateLegacyConfig } = await import("./config.js"); const res = migrateLegacyConfig({ audio: { transcription: { @@ -144,8 +142,6 @@ describe("legacy config detection", () => { expect(res.config?.audio).toBeUndefined(); }); it("rejects audio.transcription when command contains non-string parts", async () => { - vi.resetModules(); - const { migrateLegacyConfig } = await import("./config.js"); const res = migrateLegacyConfig({ audio: { transcription: { diff --git a/src/config/paths.test.ts b/src/config/paths.test.ts index fb956a10a7..5e49ff6b4f 100644 --- a/src/config/paths.test.ts +++ b/src/config/paths.test.ts @@ -1,9 +1,10 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { describe, expect, it, vi } from "vitest"; +import { describe, expect, it } from "vitest"; import { resolveDefaultConfigCandidates, + resolveConfigPathCandidate, resolveConfigPath, resolveOAuthDir, resolveOAuthPath, @@ -108,74 +109,16 @@ describe("state + config path candidates", () => { it("CONFIG_PATH prefers existing config when present", async () => { const root = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-config-")); - const previousHome = process.env.HOME; - const previousUserProfile = process.env.USERPROFILE; - const previousHomeDrive = process.env.HOMEDRIVE; - const previousHomePath = process.env.HOMEPATH; - const previousOpenClawConfig = process.env.OPENCLAW_CONFIG_PATH; - const previousOpenClawState = process.env.OPENCLAW_STATE_DIR; try { const legacyDir = path.join(root, ".openclaw"); await fs.mkdir(legacyDir, { recursive: true }); const legacyPath = path.join(legacyDir, "openclaw.json"); await fs.writeFile(legacyPath, "{}", "utf-8"); - process.env.HOME = root; - if (process.platform === "win32") { - process.env.USERPROFILE = root; - const parsed = path.win32.parse(root); - process.env.HOMEDRIVE = parsed.root.replace(/\\$/, ""); - process.env.HOMEPATH = root.slice(parsed.root.length - 1); - } - delete process.env.OPENCLAW_CONFIG_PATH; - delete process.env.OPENCLAW_STATE_DIR; - - vi.resetModules(); - const { CONFIG_PATH } = await import("./paths.js"); - expect(CONFIG_PATH).toBe(legacyPath); + const resolved = resolveConfigPathCandidate({} as NodeJS.ProcessEnv, () => root); + expect(resolved).toBe(legacyPath); } finally { - if (previousHome === undefined) { - delete process.env.HOME; - } else { - process.env.HOME = previousHome; - } - if (previousUserProfile === undefined) { - delete process.env.USERPROFILE; - } else { - process.env.USERPROFILE = previousUserProfile; - } - if (previousHomeDrive === undefined) { - delete process.env.HOMEDRIVE; - } else { - process.env.HOMEDRIVE = previousHomeDrive; - } - if (previousHomePath === undefined) { - delete process.env.HOMEPATH; - } else { - process.env.HOMEPATH = previousHomePath; - } - if (previousOpenClawConfig === undefined) { - delete process.env.OPENCLAW_CONFIG_PATH; - } else { - process.env.OPENCLAW_CONFIG_PATH = previousOpenClawConfig; - } - if (previousOpenClawConfig === undefined) { - delete process.env.OPENCLAW_CONFIG_PATH; - } else { - process.env.OPENCLAW_CONFIG_PATH = previousOpenClawConfig; - } - if (previousOpenClawState === undefined) { - delete process.env.OPENCLAW_STATE_DIR; - } else { - process.env.OPENCLAW_STATE_DIR = previousOpenClawState; - } - if (previousOpenClawState === undefined) { - delete process.env.OPENCLAW_STATE_DIR; - } else { - process.env.OPENCLAW_STATE_DIR = previousOpenClawState; - } await fs.rm(root, { recursive: true, force: true }); - vi.resetModules(); } }); diff --git a/src/hooks/hooks-install.e2e.test.ts b/src/hooks/hooks-install.e2e.test.ts index 0bb0fcc637..98afa7319c 100644 --- a/src/hooks/hooks-install.e2e.test.ts +++ b/src/hooks/hooks-install.e2e.test.ts @@ -1,7 +1,14 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { installHooksFromPath } from "./install.js"; +import { + clearInternalHooks, + createInternalHookEvent, + triggerInternalHook, +} from "./internal-hooks.js"; +import { loadInternalHooks } from "./loader.js"; const tempDirs: string[] = []; @@ -12,36 +19,15 @@ async function makeTempDir() { } describe("hooks install (e2e)", () => { - let prevStateDir: string | undefined; - let prevBundledDir: string | undefined; let workspaceDir: string; beforeEach(async () => { const baseDir = await makeTempDir(); workspaceDir = path.join(baseDir, "workspace"); await fs.mkdir(workspaceDir, { recursive: true }); - - prevStateDir = process.env.OPENCLAW_STATE_DIR; - prevBundledDir = process.env.OPENCLAW_BUNDLED_HOOKS_DIR; - process.env.OPENCLAW_STATE_DIR = path.join(baseDir, "state"); - process.env.OPENCLAW_BUNDLED_HOOKS_DIR = path.join(baseDir, "bundled-none"); - vi.resetModules(); }); afterEach(async () => { - if (prevStateDir === undefined) { - delete process.env.OPENCLAW_STATE_DIR; - } else { - process.env.OPENCLAW_STATE_DIR = prevStateDir; - } - - if (prevBundledDir === undefined) { - delete process.env.OPENCLAW_BUNDLED_HOOKS_DIR; - } else { - process.env.OPENCLAW_BUNDLED_HOOKS_DIR = prevBundledDir; - } - - vi.resetModules(); for (const dir of tempDirs.splice(0)) { try { await fs.rm(dir, { recursive: true, force: true }); @@ -92,23 +78,29 @@ describe("hooks install (e2e)", () => { "utf-8", ); - const { installHooksFromPath } = await import("./install.js"); - const installResult = await installHooksFromPath({ path: packDir }); + const hooksDir = path.join(baseDir, "managed-hooks"); + const installResult = await installHooksFromPath({ path: packDir, hooksDir }); expect(installResult.ok).toBe(true); if (!installResult.ok) { return; } - const { clearInternalHooks, createInternalHookEvent, triggerInternalHook } = - await import("./internal-hooks.js"); - const { loadInternalHooks } = await import("./loader.js"); - clearInternalHooks(); + const bundledHooksDir = path.join(baseDir, "bundled-none"); + await fs.mkdir(bundledHooksDir, { recursive: true }); const loaded = await loadInternalHooks( - { hooks: { internal: { enabled: true } } }, + { + hooks: { + internal: { + enabled: true, + load: { extraDirs: [hooksDir] }, + }, + }, + }, workspaceDir, + { managedHooksDir: hooksDir, bundledHooksDir }, ); - expect(loaded).toBe(1); + expect(loaded).toBeGreaterThanOrEqual(1); const event = createInternalHookEvent("command", "new", "test-session"); await triggerInternalHook(event); diff --git a/src/hooks/loader.ts b/src/hooks/loader.ts index 2b7bc5395e..9f558b8f6b 100644 --- a/src/hooks/loader.ts +++ b/src/hooks/loader.ts @@ -36,6 +36,10 @@ import { loadWorkspaceHookEntries } from "./workspace.js"; export async function loadInternalHooks( cfg: OpenClawConfig, workspaceDir: string, + opts?: { + managedHooksDir?: string; + bundledHooksDir?: string; + }, ): Promise { // Check if hooks are enabled if (!cfg.hooks?.internal?.enabled) { @@ -46,7 +50,11 @@ export async function loadInternalHooks( // 1. Load hooks from directories (new system) try { - const hookEntries = loadWorkspaceHookEntries(workspaceDir, { config: cfg }); + const hookEntries = loadWorkspaceHookEntries(workspaceDir, { + config: cfg, + managedHooksDir: opts?.managedHooksDir, + bundledHooksDir: opts?.bundledHooksDir, + }); // Filter by eligibility const eligible = hookEntries.filter((entry) => shouldIncludeHook({ entry, config: cfg })); diff --git a/src/plugins/install.e2e.test.ts b/src/plugins/install.e2e.test.ts index 9ed17f2743..b81d7fc563 100644 --- a/src/plugins/install.e2e.test.ts +++ b/src/plugins/install.e2e.test.ts @@ -5,6 +5,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; +import * as skillScanner from "../security/skill-scanner.js"; vi.mock("../process/exec.js", () => ({ runCommandWithTimeout: vi.fn(), @@ -449,18 +450,9 @@ describe("installPluginFromArchive", () => { }); it("continues install when scanner throws", async () => { - vi.resetModules(); - vi.doMock("../security/skill-scanner.js", async () => { - const actual = await vi.importActual( - "../security/skill-scanner.js", - ); - return { - ...actual, - scanDirectoryWithSummary: async () => { - throw new Error("scanner exploded"); - }, - }; - }); + const scanSpy = vi + .spyOn(skillScanner, "scanDirectoryWithSummary") + .mockRejectedValueOnce(new Error("scanner exploded")); const tmpDir = makeTempDir(); const pluginDir = path.join(tmpDir, "plugin-src"); @@ -492,9 +484,7 @@ describe("installPluginFromArchive", () => { expect(result.ok).toBe(true); expect(warnings.some((w) => w.includes("code safety scan failed"))).toBe(true); - - vi.doUnmock("../security/skill-scanner.js"); - vi.resetModules(); + scanSpy.mockRestore(); }); }); diff --git a/src/plugins/install.ts b/src/plugins/install.ts index 761d5fa6a4..6d661d97e8 100644 --- a/src/plugins/install.ts +++ b/src/plugins/install.ts @@ -10,7 +10,7 @@ import { resolvePackedRootDir, } from "../infra/archive.js"; import { runCommandWithTimeout } from "../process/exec.js"; -import { scanDirectoryWithSummary } from "../security/skill-scanner.js"; +import * as skillScanner from "../security/skill-scanner.js"; import { CONFIG_DIR, resolveUserPath } from "../utils.js"; type PluginInstallLogger = { @@ -196,7 +196,7 @@ async function installPluginFromPackageDir(params: { // Scan plugin source for dangerous code patterns (warn-only; never blocks install) try { - const scanSummary = await scanDirectoryWithSummary(params.packageDir, { + const scanSummary = await skillScanner.scanDirectoryWithSummary(params.packageDir, { includeFiles: forcedScanEntries, }); if (scanSummary.critical > 0) { diff --git a/src/security/audit-extra.async.ts b/src/security/audit-extra.async.ts index 25f098c892..5553386293 100644 --- a/src/security/audit-extra.async.ts +++ b/src/security/audit-extra.async.ts @@ -9,6 +9,7 @@ import path from "node:path"; import type { SandboxToolPolicy } from "../agents/sandbox/types.js"; import type { OpenClawConfig, ConfigFileSnapshot } from "../config/config.js"; import type { AgentToolsConfig } from "../config/types.tools.js"; +import type { SkillScanFinding } from "./skill-scanner.js"; import type { ExecFn } from "./windows-acl.js"; import { resolveAgentWorkspaceDir, resolveDefaultAgentId } from "../agents/agent-scope.js"; import { isToolAllowedByPolicies } from "../agents/pi-tools.policy.js"; @@ -31,7 +32,7 @@ import { inspectPathPermissions, safeStat, } from "./audit-fs.js"; -import { scanDirectoryWithSummary, type SkillScanFinding } from "./skill-scanner.js"; +import * as skillScanner from "./skill-scanner.js"; export type SecurityAuditFinding = { checkId: string; @@ -812,19 +813,21 @@ export async function collectPluginsCodeSafetyFindings(params: { }); } - const summary = await scanDirectoryWithSummary(pluginPath, { - includeFiles: forcedScanEntries, - }).catch((err) => { - findings.push({ - checkId: "plugins.code_safety.scan_failed", - severity: "warn", - title: `Plugin "${pluginName}" code scan failed`, - detail: `Static code scan could not complete: ${String(err)}`, - remediation: - "Check file permissions and plugin layout, then rerun `openclaw security audit --deep`.", + const summary = await skillScanner + .scanDirectoryWithSummary(pluginPath, { + includeFiles: forcedScanEntries, + }) + .catch((err) => { + findings.push({ + checkId: "plugins.code_safety.scan_failed", + severity: "warn", + title: `Plugin "${pluginName}" code scan failed`, + detail: `Static code scan could not complete: ${String(err)}`, + remediation: + "Check file permissions and plugin layout, then rerun `openclaw security audit --deep`.", + }); + return null; }); - return null; - }); if (!summary) { continue; } @@ -885,7 +888,7 @@ export async function collectInstalledSkillsCodeSafetyFindings(params: { scannedSkillDirs.add(skillDir); const skillName = entry.skill.name; - const summary = await scanDirectoryWithSummary(skillDir).catch((err) => { + const summary = await skillScanner.scanDirectoryWithSummary(skillDir).catch((err) => { findings.push({ checkId: "skills.code_safety.scan_failed", severity: "warn", diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index d13dbff697..be566016b3 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -7,7 +7,9 @@ import type { OpenClawConfig } from "../config/config.js"; import { discordPlugin } from "../../extensions/discord/src/channel.js"; import { slackPlugin } from "../../extensions/slack/src/channel.js"; import { telegramPlugin } from "../../extensions/telegram/src/channel.js"; +import { collectPluginsCodeSafetyFindings } from "./audit-extra.js"; import { runSecurityAudit } from "./audit.js"; +import * as skillScanner from "./skill-scanner.js"; const isWindows = process.platform === "win32"; @@ -1492,17 +1494,9 @@ description: test skill }); it("reports scan_failed when plugin code scanner throws during deep audit", async () => { - vi.resetModules(); - vi.doMock("./skill-scanner.js", async () => { - const actual = - await vi.importActual("./skill-scanner.js"); - return { - ...actual, - scanDirectoryWithSummary: async () => { - throw new Error("boom"); - }, - }; - }); + const scanSpy = vi + .spyOn(skillScanner, "scanDirectoryWithSummary") + .mockRejectedValueOnce(new Error("boom")); const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-audit-scanner-")); try { @@ -1517,12 +1511,10 @@ description: test skill ); await fs.writeFile(path.join(pluginDir, "index.js"), "export {};"); - const { collectPluginsCodeSafetyFindings } = await import("./audit-extra.js"); const findings = await collectPluginsCodeSafetyFindings({ stateDir: tmpDir }); expect(findings.some((f) => f.checkId === "plugins.code_safety.scan_failed")).toBe(true); } finally { - vi.doUnmock("./skill-scanner.js"); - vi.resetModules(); + scanSpy.mockRestore(); await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => undefined); } });