diff --git a/src/agents/skills-install-download.ts b/src/agents/skills-install-download.ts index ace39ae01e9..836ba9d524f 100644 --- a/src/agents/skills-install-download.ts +++ b/src/agents/skills-install-download.ts @@ -12,12 +12,18 @@ import { fetchWithSsrFGuard } from "../infra/net/fetch-guard.js"; import { isWithinDir } from "../infra/path-safety.js"; import { normalizeOptionalLowercaseString } from "../shared/string-coerce.js"; import { ensureDir, resolveUserPath } from "../utils.js"; -import { extractArchive } from "./skills-install-extract.js"; import { formatInstallFailureMessage } from "./skills-install-output.js"; import type { SkillInstallResult } from "./skills-install.types.js"; import type { SkillEntry, SkillInstallSpec } from "./skills.js"; import { resolveSkillToolsRootDir } from "./skills/tools-dir.js"; +let extractModulePromise: Promise | undefined; + +async function loadExtractModule() { + extractModulePromise ??= import("./skills-install-extract.js"); + return await extractModulePromise; +} + function isNodeReadableStream(value: unknown): value is NodeJS.ReadableStream { return Boolean(value && typeof (value as NodeJS.ReadableStream).pipe === "function"); } @@ -223,6 +229,7 @@ export async function installDownloadSpec(params: { return { ok: false, message, stdout: "", stderr: message, code: null }; } + const { extractArchive } = await loadExtractModule(); const extractResult = await extractArchive({ archivePath, archiveType, diff --git a/src/agents/skills-install-fallback.test.ts b/src/agents/skills-install-fallback.test.ts index bab195d316e..63f9218a4c1 100644 --- a/src/agents/skills-install-fallback.test.ts +++ b/src/agents/skills-install-fallback.test.ts @@ -21,26 +21,11 @@ vi.mock("../security/skill-scanner.js", () => ({ scanDirectoryWithSummary: (...args: unknown[]) => scanDirectoryWithSummaryMock(...args), })); -vi.mock("../shared/config-eval.js", async () => { - const actual = await vi.importActual( - "../shared/config-eval.js", - ); - return { - ...actual, - hasBinary: (bin: string) => hasBinaryMock(bin), - }; -}); - -vi.mock("../infra/brew.js", () => ({ - resolveBrewExecutable: () => undefined, -})); - let installSkill: typeof import("./skills-install.js").installSkill; -let buildWorkspaceSkillStatus: typeof import("./skills-status.js").buildWorkspaceSkillStatus; +let skillsInstallTesting: typeof import("./skills-install.js").__testing; async function loadSkillsInstallModulesForTest() { - ({ installSkill } = await import("./skills-install.js")); - ({ buildWorkspaceSkillStatus } = await import("./skills-status.js")); + ({ installSkill, __testing: skillsInstallTesting } = await import("./skills-install.js")); } async function writeSkillWithInstallers( @@ -95,10 +80,6 @@ describe("skills-install fallback edge cases", () => { await writeSkillWithInstaller(workspaceDir, "go-tool-single", "go", { module: "example.com/tool@latest", }); - await writeSkillWithInstallers(workspaceDir, "go-tool-multi", [ - { id: "brew", kind: "brew", formula: "go" }, - { id: "go", kind: "go", module: "example.com/tool@latest" }, - ]); await writeSkillWithInstaller(workspaceDir, "py-tool", "uv", { package: "example-package", }); @@ -106,13 +87,18 @@ describe("skills-install fallback edge cases", () => { }); beforeEach(() => { - runCommandWithTimeoutMock.mockClear(); - scanDirectoryWithSummaryMock.mockClear(); - hasBinaryMock.mockClear(); + runCommandWithTimeoutMock.mockReset(); + scanDirectoryWithSummaryMock.mockReset(); + hasBinaryMock.mockReset(); scanDirectoryWithSummaryMock.mockResolvedValue({ critical: 0, warn: 0, findings: [] }); + skillsInstallTesting.setDepsForTest({ + hasBinary: (bin: string) => hasBinaryMock(bin), + resolveBrewExecutable: () => undefined, + }); }); afterAll(async () => { + skillsInstallTesting.setDepsForTest(); await fs.rm(workspaceDir, { recursive: true, force: true }).catch(() => undefined); }); @@ -167,31 +153,6 @@ describe("skills-install fallback edge cases", () => { } }); - it("status-selected go installer fails gracefully when apt fallback needs sudo", async () => { - vi.spyOn(process, "getuid").mockReturnValue(1000); - mockAvailableBinaries(["apt-get", "sudo"]); - - runCommandWithTimeoutMock.mockResolvedValueOnce({ - code: 1, - stdout: "", - stderr: "sudo: a password is required", - }); - - const status = buildWorkspaceSkillStatus(workspaceDir); - const skill = status.skills.find((entry) => entry.name === "go-tool-multi"); - expect(skill?.install[0]?.id).toBe("go"); - - const result = await installSkill({ - workspaceDir, - skillName: "go-tool-multi", - installId: skill?.install[0]?.id ?? "", - }); - - expect(result.ok).toBe(false); - expect(result.message).toContain("sudo is not usable"); - expect(result.stderr).toContain("sudo: a password is required"); - }); - it("uv not installed and no brew returns helpful error without curl auto-install", async () => { mockAvailableBinaries(["curl"]); diff --git a/src/agents/skills-install.ts b/src/agents/skills-install.ts index 8bcad459a1c..d5cb4f5a2bd 100644 --- a/src/agents/skills-install.ts +++ b/src/agents/skills-install.ts @@ -1,7 +1,7 @@ import fs from "node:fs"; import path from "node:path"; import type { OpenClawConfig } from "../config/types.openclaw.js"; -import { resolveBrewExecutable } from "../infra/brew.js"; +import { resolveBrewExecutable as defaultResolveBrewExecutable } from "../infra/brew.js"; import { formatErrorMessage } from "../infra/errors.js"; import { type InstallSafetyOverrides, @@ -14,7 +14,7 @@ import { installDownloadSpec } from "./skills-install-download.js"; import { formatInstallFailureMessage } from "./skills-install-output.js"; import type { SkillInstallResult } from "./skills-install.types.js"; import { - hasBinary, + hasBinary as defaultHasBinary, loadWorkspaceSkillEntries, resolveSkillsInstallPreferences, type SkillEntry, @@ -32,6 +32,22 @@ export type SkillInstallRequest = InstallSafetyOverrides & { }; export type { SkillInstallResult } from "./skills-install.types.js"; +type SkillsInstallDeps = { + hasBinary: (bin: string) => boolean; + resolveBrewExecutable: () => string | undefined; +}; + +const defaultSkillsInstallDeps: SkillsInstallDeps = { + hasBinary: defaultHasBinary, + resolveBrewExecutable: defaultResolveBrewExecutable, +}; + +let skillsInstallDeps = defaultSkillsInstallDeps; + +function getSkillsInstallDeps(): SkillsInstallDeps { + return skillsInstallDeps; +} + function withWarnings(result: SkillInstallResult, warnings: string[]): SkillInstallResult { if (warnings.length === 0) { return result; @@ -164,7 +180,8 @@ function buildInstallCommand( } async function resolveBrewBinDir(timeoutMs: number, brewExe?: string): Promise { - const exe = brewExe ?? (hasBinary("brew") ? "brew" : resolveBrewExecutable()); + const deps = getSkillsInstallDeps(); + const exe = brewExe ?? (deps.hasBinary("brew") ? "brew" : deps.resolveBrewExecutable()); if (!exe) { return undefined; } @@ -268,7 +285,7 @@ async function ensureUvInstalled(params: { brewExe?: string; timeoutMs: number; }): Promise { - if (params.spec.kind !== "uv" || hasBinary("uv")) { + if (params.spec.kind !== "uv" || getSkillsInstallDeps().hasBinary("uv")) { return undefined; } @@ -312,7 +329,7 @@ async function installGoViaApt(timeoutMs: number): Promise { - if (params.spec.kind !== "go" || hasBinary("go")) { + if (params.spec.kind !== "go" || getSkillsInstallDeps().hasBinary("go")) { return undefined; } @@ -367,7 +384,7 @@ async function ensureGoInstalled(params: { }); } - if (hasBinary("apt-get")) { + if (getSkillsInstallDeps().hasBinary("apt-get")) { return installGoViaApt(params.timeoutMs); } @@ -481,7 +498,8 @@ export async function installSkill(params: SkillInstallRequest): Promise): void { + skillsInstallDeps = { + ...defaultSkillsInstallDeps, + ...overrides, + }; + }, +};