perf: lazy-load skills install extraction seams

This commit is contained in:
Peter Steinberger
2026-04-18 18:32:45 +01:00
parent df525b90f2
commit 6d776593ea
3 changed files with 53 additions and 58 deletions

View File

@@ -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<typeof import("./skills-install-extract.js")> | 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,

View File

@@ -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<typeof import("../shared/config-eval.js")>(
"../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"]);

View File

@@ -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<string | undefined> {
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<SkillInstallResult | undefined> {
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<SkillInstallResult |
});
}
if (!hasBinary("sudo")) {
if (!getSkillsInstallDeps().hasBinary("sudo")) {
return createInstallFailure({
message:
"go not installed — apt-get is available but sudo is not installed. Install manually: https://go.dev/doc/install",
@@ -350,7 +367,7 @@ async function ensureGoInstalled(params: {
brewExe?: string;
timeoutMs: number;
}): Promise<SkillInstallResult | undefined> {
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<SkillIn
);
}
const brewExe = hasBinary("brew") ? "brew" : resolveBrewExecutable();
const deps = getSkillsInstallDeps();
const brewExe = deps.hasBinary("brew") ? "brew" : deps.resolveBrewExecutable();
if (spec.kind === "brew" && !brewExe) {
return withWarnings(resolveBrewMissingFailure(spec), warnings);
}
@@ -512,3 +530,12 @@ export async function installSkill(params: SkillInstallRequest): Promise<SkillIn
return withWarnings(await executeInstallCommand({ argv, timeoutMs, env }), warnings);
}
export const __testing = {
setDepsForTest(overrides?: Partial<SkillsInstallDeps>): void {
skillsInstallDeps = {
...defaultSkillsInstallDeps,
...overrides,
};
},
};