From 6281dd7379424d7618fc0679b0b1886830fbddad Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 10 Apr 2026 23:09:24 +0100 Subject: [PATCH] perf: reduce test import overhead --- docs/help/testing.md | 2 +- src/agents/bash-tools.descriptions.ts | 76 +++++++++++++++++++ src/agents/bash-tools.exec.ts | 70 +---------------- src/agents/bash-tools.process.ts | 13 +--- src/agents/bash-tools.ts | 5 +- ...i-tools.deferred-followup-guidance.test.ts | 10 ++- src/agents/pi-tools.deferred-followup.ts | 24 ++++++ src/agents/pi-tools.ts | 25 +----- test/vitest-unit-fast-config.test.ts | 7 +- test/vitest/vitest.unit-fast-paths.mjs | 6 ++ 10 files changed, 127 insertions(+), 111 deletions(-) create mode 100644 src/agents/bash-tools.descriptions.ts create mode 100644 src/agents/pi-tools.deferred-followup.ts diff --git a/docs/help/testing.md b/docs/help/testing.md index d6d59448857..f8c5d8f76de 100644 --- a/docs/help/testing.md +++ b/docs/help/testing.md @@ -88,7 +88,7 @@ Think of the suites as “increasing realism” (and increasing flakiness/cost): - `pnpm test --watch` still uses the native root `vitest.config.ts` project graph, because a multi-shard watch loop is not practical. - `pnpm test`, `pnpm test:watch`, and `pnpm test:perf:imports` route explicit file/directory targets through scoped lanes first, so `pnpm test extensions/discord/src/monitor/message-handler.preflight.test.ts` avoids paying the full root project startup tax. - `pnpm test:changed` expands changed git paths into the same scoped lanes when the diff only touches routable source/test files; config/setup edits still fall back to the broad root-project rerun. - - Selected `plugin-sdk` and `commands` tests also route through dedicated light lanes that skip `test/setup-openclaw-runtime.ts`; stateful/runtime-heavy files stay on the existing lanes. + - Import-light unit tests from agents, commands, plugins, auto-reply helpers, `plugin-sdk`, and similar pure utility areas route through the `unit-fast` lane, which skips `test/setup-openclaw-runtime.ts`; stateful/runtime-heavy files stay on the existing lanes. - Selected `plugin-sdk` and `commands` helper source files also map changed-mode runs to explicit sibling tests in those light lanes, so helper edits avoid rerunning the full heavy suite for that directory. - `auto-reply` now has three dedicated buckets: top-level core helpers, top-level `reply.*` integration tests, and the `src/auto-reply/reply/**` subtree. This keeps the heaviest reply harness work off the cheap status/chunk/token tests. - Embedded runner note: diff --git a/src/agents/bash-tools.descriptions.ts b/src/agents/bash-tools.descriptions.ts new file mode 100644 index 00000000000..e64314fd2fb --- /dev/null +++ b/src/agents/bash-tools.descriptions.ts @@ -0,0 +1,76 @@ +import path from "node:path"; +import { loadExecApprovals, resolveExecApprovalsFromFile } from "../infra/exec-approvals.js"; + +/** + * Show the exact approved token in hints. Absolute paths stay absolute so the + * hint cannot imply an equivalent PATH lookup that resolves to a different binary. + */ +function deriveExecShortName(fullPath: string): string { + if (path.isAbsolute(fullPath)) { + return fullPath; + } + const base = path.basename(fullPath); + return base.replace(/\.exe$/i, "") || base; +} + +export function describeExecTool(params?: { agentId?: string; hasCronTool?: boolean }): string { + const base = [ + "Execute shell commands with background continuation for work that starts now.", + "Use yieldMs/background to continue later via process tool.", + "For long-running work started now, rely on automatic completion wake when it is enabled and the command emits output or fails; otherwise use process to confirm completion. Use process whenever you need logs, status, input, or intervention.", + params?.hasCronTool + ? "Do not use exec sleep or delay loops for reminders or deferred follow-ups; use cron instead." + : undefined, + "Use pty=true for TTY-required commands (terminal UIs, coding agents).", + ] + .filter(Boolean) + .join(" "); + if (process.platform !== "win32") { + return base; + } + const lines: string[] = [base]; + lines.push( + "IMPORTANT (Windows): Run executables directly; do NOT wrap commands in `cmd /c`, `powershell -Command`, `& ` prefix, or WSL. Use backslash paths (C:\\path), not forward slashes. Use short executable names (e.g. `node`, `python3`) instead of full paths.", + ); + try { + const approvalsFile = loadExecApprovals(); + const approvals = resolveExecApprovalsFromFile({ + file: approvalsFile, + agentId: params?.agentId, + }); + const allowlist = approvals.allowlist.filter((entry) => { + const pattern = entry.pattern?.trim() ?? ""; + return ( + pattern.length > 0 && + pattern !== "*" && + !pattern.startsWith("=command:") && + (pattern.includes("/") || pattern.includes("\\") || pattern.includes("~")) + ); + }); + if (allowlist.length > 0) { + lines.push( + "Pre-approved executables (exact arguments are enforced at runtime; no approval prompt needed when args match):", + ); + for (const entry of allowlist.slice(0, 10)) { + const shortName = deriveExecShortName(entry.pattern); + const argNote = entry.argPattern ? "(restricted args)" : "(any arguments)"; + lines.push(` ${shortName} ${argNote}`); + } + } + } catch { + // Allowlist loading is best-effort; don't block tool creation. + } + return lines.join("\n"); +} + +export function describeProcessTool(params?: { hasCronTool?: boolean }): string { + return [ + "Manage running exec sessions for commands already started: list, poll, log, write, send-keys, submit, paste, kill.", + "Use poll/log when you need status, logs, quiet-success confirmation, or completion confirmation when automatic completion wake is unavailable. Use write/send-keys/submit/paste/kill for input or intervention.", + params?.hasCronTool + ? "Do not use process polling to emulate timers or reminders; use cron for scheduled follow-ups." + : undefined, + ] + .filter(Boolean) + .join(" "); +} diff --git a/src/agents/bash-tools.exec.ts b/src/agents/bash-tools.exec.ts index 58be5a18650..bac25c7af55 100644 --- a/src/agents/bash-tools.exec.ts +++ b/src/agents/bash-tools.exec.ts @@ -1,13 +1,7 @@ import path from "node:path"; import type { AgentToolResult } from "@mariozechner/pi-agent-core"; import { analyzeShellCommand } from "../infra/exec-approvals-analysis.js"; -import { - type ExecHost, - loadExecApprovals, - maxAsk, - minSecurity, - resolveExecApprovalsFromFile, -} from "../infra/exec-approvals.js"; +import { type ExecHost, loadExecApprovals, maxAsk, minSecurity } from "../infra/exec-approvals.js"; import { resolveExecSafeBinRuntimePolicy } from "../infra/exec-safe-bin-runtime-policy.js"; import { SafeOpenError, readFileWithinRoot } from "../infra/fs-safe.js"; import { sanitizeHostExecEnvWithDiagnostics } from "../infra/host-env-security.js"; @@ -24,6 +18,7 @@ import { } from "../shared/string-coerce.js"; import { splitShellArgs } from "../utils/shell-argv.js"; import { markBackgrounded } from "./bash-process-registry.js"; +import { describeExecTool } from "./bash-tools.descriptions.js"; import { processGatewayAllowlist } from "./bash-tools.exec-host-gateway.js"; import { executeNodeHostCommand } from "./bash-tools.exec-host-node.js"; import { @@ -1282,67 +1277,6 @@ function rejectExecApprovalShellCommand(command: string): void { } } -/** - * Show the exact approved token in hints. Absolute paths stay absolute so the - * hint cannot imply an equivalent PATH lookup that resolves to a different binary. - */ -function deriveExecShortName(fullPath: string): string { - if (path.isAbsolute(fullPath)) { - return fullPath; - } - const base = path.basename(fullPath); - return base.replace(/\.exe$/i, "") || base; -} - -export function describeExecTool(params?: { agentId?: string; hasCronTool?: boolean }): string { - const base = [ - "Execute shell commands with background continuation for work that starts now.", - "Use yieldMs/background to continue later via process tool.", - "For long-running work started now, rely on automatic completion wake when it is enabled and the command emits output or fails; otherwise use process to confirm completion. Use process whenever you need logs, status, input, or intervention.", - params?.hasCronTool - ? "Do not use exec sleep or delay loops for reminders or deferred follow-ups; use cron instead." - : undefined, - "Use pty=true for TTY-required commands (terminal UIs, coding agents).", - ] - .filter(Boolean) - .join(" "); - if (process.platform !== "win32") { - return base; - } - const lines: string[] = [base]; - lines.push( - "IMPORTANT (Windows): Run executables directly — do NOT wrap commands in `cmd /c`, `powershell -Command`, `& ` prefix, or WSL. Use backslash paths (C:\\path), not forward slashes. Use short executable names (e.g. `node`, `python3`) instead of full paths.", - ); - try { - const approvalsFile = loadExecApprovals(); - const approvals = resolveExecApprovalsFromFile({ - file: approvalsFile, - agentId: params?.agentId, - }); - const allowlist = approvals.allowlist.filter((entry) => { - const pattern = entry.pattern?.trim() ?? ""; - return ( - pattern.length > 0 && - pattern !== "*" && - !pattern.startsWith("=command:") && - (pattern.includes("/") || pattern.includes("\\") || pattern.includes("~")) - ); - }); - if (allowlist.length > 0) { - lines.push( - "Pre-approved executables (exact arguments are enforced at runtime; no approval prompt needed when args match):", - ); - for (const entry of allowlist.slice(0, 10)) { - const shortName = deriveExecShortName(entry.pattern); - const argNote = entry.argPattern ? "(restricted args)" : "(any arguments)"; - lines.push(` ${shortName} ${argNote}`); - } - } - } catch { - // Allowlist loading is best-effort; don't block tool creation. - } - return lines.join("\n"); -} export function createExecTool( defaults?: ExecToolDefaults, ): AgentToolWithMeta { diff --git a/src/agents/bash-tools.process.ts b/src/agents/bash-tools.process.ts index 39e2d5a037f..8f9108aace3 100644 --- a/src/agents/bash-tools.process.ts +++ b/src/agents/bash-tools.process.ts @@ -15,6 +15,7 @@ import { markExited, setJobTtlMs, } from "./bash-process-registry.js"; +import { describeProcessTool } from "./bash-tools.descriptions.js"; import { deriveSessionName, pad, sliceLogLines, truncateMiddle } from "./bash-tools.shared.js"; import { recordCommandPoll, resetCommandPollCount } from "./command-poll-backoff.js"; import { encodeKeySequence, encodePaste, hasCursorModeSensitiveKeys } from "./pty-keys.js"; @@ -119,18 +120,6 @@ function resetPollRetrySuggestion(sessionId: string): void { } } -export function describeProcessTool(params?: { hasCronTool?: boolean }): string { - return [ - "Manage running exec sessions for commands already started: list, poll, log, write, send-keys, submit, paste, kill.", - "Use poll/log when you need status, logs, quiet-success confirmation, or completion confirmation when automatic completion wake is unavailable. Use write/send-keys/submit/paste/kill for input or intervention.", - params?.hasCronTool - ? "Do not use process polling to emulate timers or reminders; use cron for scheduled follow-ups." - : undefined, - ] - .filter(Boolean) - .join(" "); -} - export function createProcessTool( defaults?: ProcessToolDefaults, ): AgentToolWithMeta { diff --git a/src/agents/bash-tools.ts b/src/agents/bash-tools.ts index f8801afcfe6..5bd029e7483 100644 --- a/src/agents/bash-tools.ts +++ b/src/agents/bash-tools.ts @@ -4,6 +4,7 @@ export type { ExecToolDefaults, ExecToolDetails, } from "./bash-tools.exec.js"; -export { createExecTool, describeExecTool, execTool } from "./bash-tools.exec.js"; +export { describeExecTool, describeProcessTool } from "./bash-tools.descriptions.js"; +export { createExecTool, execTool } from "./bash-tools.exec.js"; export type { ProcessToolDefaults } from "./bash-tools.process.js"; -export { createProcessTool, describeProcessTool, processTool } from "./bash-tools.process.js"; +export { createProcessTool, processTool } from "./bash-tools.process.js"; diff --git a/src/agents/pi-tools.deferred-followup-guidance.test.ts b/src/agents/pi-tools.deferred-followup-guidance.test.ts index 3d7d4174293..fc1dce427b1 100644 --- a/src/agents/pi-tools.deferred-followup-guidance.test.ts +++ b/src/agents/pi-tools.deferred-followup-guidance.test.ts @@ -1,9 +1,13 @@ import { describe, expect, it } from "vitest"; -import "./test-helpers/fast-openclaw-tools.js"; -import { createOpenClawCodingTools } from "./pi-tools.js"; +import { applyDeferredFollowupToolDescriptions } from "./pi-tools.deferred-followup.js"; +import type { AnyAgentTool } from "./pi-tools.types.js"; function findToolDescription(toolName: string, senderIsOwner: boolean) { - const tools = createOpenClawCodingTools({ senderIsOwner }); + const tools = applyDeferredFollowupToolDescriptions([ + { name: "exec", description: "exec base" }, + { name: "process", description: "process base" }, + ...(senderIsOwner ? [{ name: "cron", description: "cron base" }] : []), + ] as AnyAgentTool[]); const tool = tools.find((entry) => entry.name === toolName); return { toolNames: tools.map((entry) => entry.name), diff --git a/src/agents/pi-tools.deferred-followup.ts b/src/agents/pi-tools.deferred-followup.ts new file mode 100644 index 00000000000..ff582a3c2cb --- /dev/null +++ b/src/agents/pi-tools.deferred-followup.ts @@ -0,0 +1,24 @@ +import { describeExecTool, describeProcessTool } from "./bash-tools.descriptions.js"; +import type { AnyAgentTool } from "./pi-tools.types.js"; + +export function applyDeferredFollowupToolDescriptions( + tools: AnyAgentTool[], + params?: { agentId?: string }, +): AnyAgentTool[] { + const hasCronTool = tools.some((tool) => tool.name === "cron"); + return tools.map((tool) => { + if (tool.name === "exec") { + return { + ...tool, + description: describeExecTool({ agentId: params?.agentId, hasCronTool }), + }; + } + if (tool.name === "process") { + return { + ...tool, + description: describeProcessTool({ hasCronTool }), + }; + } + return tool; + }); +} diff --git a/src/agents/pi-tools.ts b/src/agents/pi-tools.ts index 3718b7e83ec..b64f35770f1 100644 --- a/src/agents/pi-tools.ts +++ b/src/agents/pi-tools.ts @@ -16,8 +16,6 @@ import { createApplyPatchTool } from "./apply-patch.js"; import { createExecTool, createProcessTool, - describeExecTool, - describeProcessTool, type ExecToolDefaults, type ProcessToolDefaults, } from "./bash-tools.js"; @@ -28,6 +26,7 @@ import type { ModelAuthMode } from "./model-auth.js"; import { createOpenClawTools } from "./openclaw-tools.js"; import { wrapToolWithAbortSignal } from "./pi-tools.abort.js"; import { wrapToolWithBeforeToolCallHook } from "./pi-tools.before-tool-call.js"; +import { applyDeferredFollowupToolDescriptions } from "./pi-tools.deferred-followup.js"; import { filterToolsByMessageProvider } from "./pi-tools.message-provider-policy.js"; import { isToolAllowedByPolicies, @@ -97,28 +96,6 @@ function applyModelProviderToolPolicy( return tools; } -function applyDeferredFollowupToolDescriptions( - tools: AnyAgentTool[], - params?: { agentId?: string }, -): AnyAgentTool[] { - const hasCronTool = tools.some((tool) => tool.name === "cron"); - return tools.map((tool) => { - if (tool.name === "exec") { - return { - ...tool, - description: describeExecTool({ agentId: params?.agentId, hasCronTool }), - }; - } - if (tool.name === "process") { - return { - ...tool, - description: describeProcessTool({ hasCronTool }), - }; - } - return tool; - }); -} - function isApplyPatchAllowedForModel(params: { modelProvider?: string; modelId?: string; diff --git a/test/vitest-unit-fast-config.test.ts b/test/vitest-unit-fast-config.test.ts index 9c852544af3..926fb14a6bf 100644 --- a/test/vitest-unit-fast-config.test.ts +++ b/test/vitest-unit-fast-config.test.ts @@ -19,8 +19,12 @@ describe("unit-fast vitest lane", () => { expect(config.test?.isolate).toBe(false); expect(config.test?.runner).toBeUndefined(); expect(config.test?.setupFiles).toEqual([]); - expect(config.test?.include).toContain("src/plugin-sdk/provider-entry.test.ts"); + expect(config.test?.include).toContain( + "src/agents/pi-tools.deferred-followup-guidance.test.ts", + ); expect(config.test?.include).toContain("src/commands/status-overview-values.test.ts"); + expect(config.test?.include).toContain("src/plugins/config-policy.test.ts"); + expect(config.test?.include).toContain("src/plugin-sdk/provider-entry.test.ts"); }); it("does not treat moved config paths as CLI include filters", () => { @@ -37,6 +41,7 @@ describe("unit-fast vitest lane", () => { it("keeps obvious stateful files out of the unit-fast lane", () => { expect(isUnitFastTestFile("src/plugin-sdk/temp-path.test.ts")).toBe(false); + expect(isUnitFastTestFile("src/agents/sandbox.resolveSandboxContext.test.ts")).toBe(false); expect(resolveUnitFastTestIncludePattern("src/plugin-sdk/temp-path.ts")).toBeNull(); expect(classifyUnitFastTestFileContent("vi.resetModules(); await import('./x.js')")).toEqual([ "module-mocking", diff --git a/test/vitest/vitest.unit-fast-paths.mjs b/test/vitest/vitest.unit-fast-paths.mjs index 34ead658bf6..efe94fa2c7a 100644 --- a/test/vitest/vitest.unit-fast-paths.mjs +++ b/test/vitest/vitest.unit-fast-paths.mjs @@ -12,9 +12,12 @@ const unitFastCandidateGlobs = [ "packages/memory-host-sdk/**/*.test.ts", "packages/plugin-package-contract/**/*.test.ts", "src/acp/**/*.test.ts", + "src/agents/**/*.test.ts", + "src/auto-reply/**/*.test.ts", "src/bootstrap/**/*.test.ts", "src/channels/**/*.test.ts", "src/cli/**/*.test.ts", + "src/commands/**/*.test.ts", "src/config/**/*.test.ts", "src/daemon/**/*.test.ts", "src/i18n/**/*.test.ts", @@ -32,6 +35,7 @@ const unitFastCandidateGlobs = [ "src/music-generation/**/*.test.ts", "src/node-host/**/*.test.ts", "src/plugin-sdk/**/*.test.ts", + "src/plugins/**/*.test.ts", "src/poll-params.test.ts", "src/polls.test.ts", "src/process/**/*.test.ts", @@ -58,10 +62,12 @@ const broadUnitFastCandidateSkipGlobs = [ "**/*.live.test.ts", "test/fixtures/**/*.test.ts", "test/setup-home-isolation.test.ts", + "src/agents/sandbox.resolveSandboxContext.test.ts", "src/channels/plugins/contracts/**/*.test.ts", "src/config/**/*.test.ts", "src/gateway/**/*.test.ts", "src/media-generation/**/*.contract.test.ts", + "src/plugins/contracts/**/*.test.ts", "src/plugin-sdk/browser-subpaths.test.ts", "src/security/**/*.test.ts", "src/secrets/**/*.test.ts",