diff --git a/src/agents/skills-install.download.test.ts b/src/agents/skills-install.download.test.ts index 2cbbe7e42..1eaf1cf14 100644 --- a/src/agents/skills-install.download.test.ts +++ b/src/agents/skills-install.download.test.ts @@ -2,14 +2,15 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; -import { createTempHomeEnv } from "../test-utils/temp-home.js"; -import { setTempStateDir, writeDownloadSkill } from "./skills-install.download-test-utils.js"; -import { installSkill } from "./skills-install.js"; +import { installDownloadSpec } from "./skills-install-download.js"; +import { setTempStateDir } from "./skills-install.download-test-utils.js"; import { fetchWithSsrFGuardMock, + hasBinaryMock, runCommandWithTimeoutMock, - scanDirectoryWithSummaryMock, } from "./skills-install.test-mocks.js"; +import { resolveSkillToolsRootDir } from "./skills/tools-dir.js"; +import type { SkillEntry, SkillInstallSpec } from "./skills/types.js"; vi.mock("../process/exec.js", () => ({ runCommandWithTimeout: (...args: unknown[]) => runCommandWithTimeoutMock(...args), @@ -19,9 +20,9 @@ vi.mock("../infra/net/fetch-guard.js", () => ({ fetchWithSsrFGuard: (...args: unknown[]) => fetchWithSsrFGuardMock(...args), })); -vi.mock("../security/skill-scanner.js", async (importOriginal) => ({ - ...(await importOriginal()), - scanDirectoryWithSummary: (...args: unknown[]) => scanDirectoryWithSummaryMock(...args), +vi.mock("./skills.js", async (importOriginal) => ({ + ...(await importOriginal()), + hasBinary: (bin: string) => hasBinaryMock(bin), })); async function fileExists(filePath: string): Promise { @@ -51,6 +52,54 @@ const TAR_GZ_TRAVERSAL_BUFFER = Buffer.from( "base64", ); +function buildEntry(name: string): SkillEntry { + const skillDir = path.join(workspaceDir, "skills", name); + return { + skill: { + name, + description: `${name} test skill`, + source: "openclaw-workspace", + filePath: path.join(skillDir, "SKILL.md"), + baseDir: skillDir, + disableModelInvocation: false, + }, + frontmatter: {}, + }; +} + +function buildDownloadSpec(params: { + url: string; + archive: "tar.gz" | "tar.bz2" | "zip"; + targetDir: string; + stripComponents?: number; +}): SkillInstallSpec { + return { + kind: "download", + id: "dl", + url: params.url, + archive: params.archive, + extract: true, + targetDir: params.targetDir, + ...(typeof params.stripComponents === "number" + ? { stripComponents: params.stripComponents } + : {}), + }; +} + +async function installDownloadSkill(params: { + name: string; + url: string; + archive: "tar.gz" | "tar.bz2" | "zip"; + targetDir: string; + stripComponents?: number; +}) { + return installDownloadSpec({ + entry: buildEntry(params.name), + spec: buildDownloadSpec(params), + timeoutMs: 30_000, + }); +} + function mockArchiveResponse(buffer: Uint8Array): void { const blobPart = Uint8Array.from(buffer); fetchWithSsrFGuardMock.mockResolvedValue({ @@ -93,61 +142,10 @@ function mockTarExtractionFlow(params: { }); } -function seedZipDownloadResponse() { - mockArchiveResponse(new Uint8Array(SAFE_ZIP_BUFFER)); -} - -async function installZipDownloadSkill(params: { - workspaceDir: string; - name: string; - targetDir: string; -}) { - const url = "https://example.invalid/good.zip"; - seedZipDownloadResponse(); - await writeDownloadSkill({ - workspaceDir: params.workspaceDir, - name: params.name, - installId: "dl", - url, - archive: "zip", - targetDir: params.targetDir, - }); - - return installSkill({ - workspaceDir: params.workspaceDir, - skillName: params.name, - installId: "dl", - }); -} - -async function writeTarBz2Skill(params: { - workspaceDir: string; - stateDir: string; - name: string; - url: string; - stripComponents?: number; -}) { - const targetDir = path.join(params.stateDir, "tools", params.name, "target"); - await writeDownloadSkill({ - workspaceDir: params.workspaceDir, - name: params.name, - installId: "dl", - url: params.url, - archive: "tar.bz2", - ...(typeof params.stripComponents === "number" - ? { stripComponents: params.stripComponents } - : {}), - targetDir, - }); -} - let workspaceDir = ""; let stateDir = ""; -let restoreTempHome: (() => Promise) | null = null; beforeAll(async () => { - const tempHome = await createTempHomeEnv("openclaw-skills-install-home-"); - restoreTempHome = () => tempHome.restore(); workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skills-install-")); stateDir = setTempStateDir(workspaceDir); }); @@ -158,27 +156,17 @@ afterAll(async () => { workspaceDir = ""; stateDir = ""; } - if (restoreTempHome) { - await restoreTempHome(); - restoreTempHome = null; - } }); -beforeEach(async () => { +beforeEach(() => { runCommandWithTimeoutMock.mockReset(); runCommandWithTimeoutMock.mockResolvedValue(runCommandResult()); - scanDirectoryWithSummaryMock.mockReset(); fetchWithSsrFGuardMock.mockReset(); - scanDirectoryWithSummaryMock.mockResolvedValue({ - scannedFiles: 0, - critical: 0, - warn: 0, - info: 0, - findings: [], - }); + hasBinaryMock.mockReset(); + hasBinaryMock.mockReturnValue(true); }); -describe("installSkill download extraction safety", () => { +describe("installDownloadSpec extraction safety", () => { it("rejects archive traversal writes outside targetDir", async () => { for (const testCase of [ { @@ -196,23 +184,15 @@ describe("installSkill download extraction safety", () => { buffer: TAR_GZ_TRAVERSAL_BUFFER, }, ]) { - const targetDir = path.join(stateDir, "tools", testCase.name, "target"); + const entry = buildEntry(testCase.name); + const targetDir = path.join(resolveSkillToolsRootDir(entry), "target"); const outsideWritePath = path.join(workspaceDir, "outside-write", "pwned.txt"); mockArchiveResponse(new Uint8Array(testCase.buffer)); - await writeDownloadSkill({ - workspaceDir, - name: testCase.name, - installId: "dl", - url: testCase.url, - archive: testCase.archive, - targetDir, - }); - const result = await installSkill({ - workspaceDir, - skillName: testCase.name, - installId: "dl", + const result = await installDownloadSkill({ + ...testCase, + targetDir, }); expect(result.ok, testCase.label).toBe(false); expect(await fileExists(outsideWritePath), testCase.label).toBe(false); @@ -220,68 +200,60 @@ describe("installSkill download extraction safety", () => { }); it("extracts zip with stripComponents safely", async () => { - const targetDir = path.join(stateDir, "tools", "zip-good", "target"); - const url = "https://example.invalid/good.zip"; + const entry = buildEntry("zip-good"); + const targetDir = path.join(resolveSkillToolsRootDir(entry), "target"); mockArchiveResponse(new Uint8Array(STRIP_COMPONENTS_ZIP_BUFFER)); - await writeDownloadSkill({ - workspaceDir, + const result = await installDownloadSkill({ name: "zip-good", - installId: "dl", - url, + url: "https://example.invalid/good.zip", archive: "zip", stripComponents: 1, targetDir, }); - - const result = await installSkill({ workspaceDir, skillName: "zip-good", installId: "dl" }); expect(result.ok).toBe(true); expect(await fs.readFile(path.join(targetDir, "hello.txt"), "utf-8")).toBe("hi"); }); it("rejects targetDir escapes outside the per-skill tools root", async () => { - for (const testCase of [{ name: "relative-traversal", targetDir: "../outside" }]) { - mockArchiveResponse(new Uint8Array(SAFE_ZIP_BUFFER)); - await writeDownloadSkill({ - workspaceDir, - name: testCase.name, - installId: "dl", - url: "https://example.invalid/good.zip", - archive: "zip", - targetDir: testCase.targetDir, - }); - const beforeFetchCalls = fetchWithSsrFGuardMock.mock.calls.length; - const result = await installSkill({ - workspaceDir, - skillName: testCase.name, - installId: "dl", - }); - expect(result.ok).toBe(false); - expect(result.stderr).toContain("Refusing to install outside the skill tools directory"); - expect(fetchWithSsrFGuardMock.mock.calls.length).toBe(beforeFetchCalls); - } + mockArchiveResponse(new Uint8Array(SAFE_ZIP_BUFFER)); + const beforeFetchCalls = fetchWithSsrFGuardMock.mock.calls.length; + const result = await installDownloadSkill({ + name: "relative-traversal", + url: "https://example.invalid/good.zip", + archive: "zip", + targetDir: "../outside", + }); + + expect(result.ok).toBe(false); + expect(result.stderr).toContain("Refusing to install outside the skill tools directory"); + expect(fetchWithSsrFGuardMock.mock.calls.length).toBe(beforeFetchCalls); expect(stateDir.length).toBeGreaterThan(0); }); it("allows relative targetDir inside the per-skill tools root", async () => { - const result = await installZipDownloadSkill({ - workspaceDir, + mockArchiveResponse(new Uint8Array(SAFE_ZIP_BUFFER)); + const entry = buildEntry("relative-targetdir"); + + const result = await installDownloadSkill({ name: "relative-targetdir", + url: "https://example.invalid/good.zip", + archive: "zip", targetDir: "runtime", }); expect(result.ok).toBe(true); expect( await fs.readFile( - path.join(stateDir, "tools", "relative-targetdir", "runtime", "hello.txt"), + path.join(resolveSkillToolsRootDir(entry), "runtime", "hello.txt"), "utf-8", ), ).toBe("hi"); }); }); -describe("installSkill download extraction safety (tar.bz2)", () => { +describe("installDownloadSpec extraction safety (tar.bz2)", () => { it("handles tar.bz2 extraction safety edge-cases", async () => { for (const testCase of [ { @@ -318,7 +290,10 @@ describe("installSkill download extraction safety (tar.bz2)", () => { expectedExtract: false, }, ]) { + const entry = buildEntry(testCase.name); + const targetDir = path.join(resolveSkillToolsRootDir(entry), "target"); const commandCallCount = runCommandWithTimeoutMock.mock.calls.length; + mockArchiveResponse(new Uint8Array([1, 2, 3])); mockTarExtractionFlow({ listOutput: testCase.listOutput, @@ -326,20 +301,12 @@ describe("installSkill download extraction safety (tar.bz2)", () => { extract: testCase.extract, }); - await writeTarBz2Skill({ - workspaceDir, - stateDir, + const result = await installDownloadSkill({ name: testCase.name, url: testCase.url, - ...(typeof testCase.stripComponents === "number" - ? { stripComponents: testCase.stripComponents } - : {}), - }); - - const result = await installSkill({ - workspaceDir, - skillName: testCase.name, - installId: "dl", + archive: "tar.bz2", + stripComponents: testCase.stripComponents, + targetDir, }); expect(result.ok, testCase.label).toBe(testCase.expectedOk); diff --git a/src/agents/skills.build-workspace-skills-prompt.prefers-workspace-skills-managed-skills.test.ts b/src/agents/skills.build-workspace-skills-prompt.prefers-workspace-skills-managed-skills.test.ts index e063404f6..03204705c 100644 --- a/src/agents/skills.build-workspace-skills-prompt.prefers-workspace-skills-managed-skills.test.ts +++ b/src/agents/skills.build-workspace-skills-prompt.prefers-workspace-skills-managed-skills.test.ts @@ -1,4 +1,3 @@ -import fs from "node:fs/promises"; import path from "node:path"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; import { withEnv } from "../test-utils/env.js"; @@ -44,20 +43,21 @@ describe("buildWorkspaceSkillsPrompt", () => { body: "# Workspace\n", }); - const prompt = buildWorkspaceSkillsPrompt(workspaceDir, { - managedSkillsDir: managedDir, - bundledSkillsDir: bundledDir, - }); + const prompt = withEnv({ HOME: workspaceDir, PATH: "" }, () => + buildWorkspaceSkillsPrompt(workspaceDir, { + managedSkillsDir: managedDir, + bundledSkillsDir: bundledDir, + }), + ); expect(prompt).toContain("Workspace version"); - expect(prompt).toContain(path.join(workspaceSkillDir, "SKILL.md")); - expect(prompt).not.toContain(path.join(managedSkillDir, "SKILL.md")); - expect(prompt).not.toContain(path.join(bundledSkillDir, "SKILL.md")); + expect(prompt).toContain("demo-skill/SKILL.md"); + expect(prompt).not.toContain("Managed version"); + expect(prompt).not.toContain("Bundled version"); }); it("gates by bins, config, and always", async () => { const workspaceDir = await fixtureSuite.createCaseDir("workspace"); const skillsDir = path.join(workspaceDir, "skills"); - const binDir = path.join(workspaceDir, "bin"); await writeSkill({ dir: path.join(skillsDir, "bin-skill"), @@ -91,9 +91,17 @@ describe("buildWorkspaceSkillsPrompt", () => { }); const managedSkillsDir = path.join(workspaceDir, ".managed"); - const defaultPrompt = withEnv({ PATH: "" }, () => + const defaultPrompt = withEnv({ HOME: workspaceDir, PATH: "" }, () => buildWorkspaceSkillsPrompt(workspaceDir, { managedSkillsDir, + eligibility: { + remote: { + platforms: ["linux"], + hasBin: () => false, + hasAnyBin: () => false, + note: "", + }, + }, }), ); expect(defaultPrompt).toContain("always-skill"); @@ -102,18 +110,21 @@ describe("buildWorkspaceSkillsPrompt", () => { expect(defaultPrompt).not.toContain("anybin-skill"); expect(defaultPrompt).not.toContain("env-skill"); - await fs.mkdir(binDir, { recursive: true }); - const fakebinPath = path.join(binDir, "fakebin"); - await fs.writeFile(fakebinPath, "#!/bin/sh\nexit 0\n", "utf-8"); - await fs.chmod(fakebinPath, 0o755); - - const gatedPrompt = withEnv({ PATH: binDir }, () => + const gatedPrompt = withEnv({ HOME: workspaceDir, PATH: "" }, () => buildWorkspaceSkillsPrompt(workspaceDir, { managedSkillsDir, config: { browser: { enabled: false }, skills: { entries: { "env-skill": { apiKey: "ok" } } }, }, + eligibility: { + remote: { + platforms: ["linux"], + hasBin: (bin: string) => bin === "fakebin", + hasAnyBin: (bins: string[]) => bins.includes("fakebin"), + note: "", + }, + }, }), ); expect(gatedPrompt).toContain("bin-skill"); @@ -132,10 +143,12 @@ describe("buildWorkspaceSkillsPrompt", () => { metadata: '{"openclaw":{"skillKey":"alias"}}', }); - const prompt = buildWorkspaceSkillsPrompt(workspaceDir, { - managedSkillsDir: path.join(workspaceDir, ".managed"), - config: { skills: { entries: { alias: { enabled: false } } } }, - }); + const prompt = withEnv({ HOME: workspaceDir, PATH: "" }, () => + buildWorkspaceSkillsPrompt(workspaceDir, { + managedSkillsDir: path.join(workspaceDir, ".managed"), + config: { skills: { entries: { alias: { enabled: false } } } }, + }), + ); expect(prompt).not.toContain("alias-skill"); }); }); diff --git a/src/agents/skills.build-workspace-skills-prompt.syncs-merged-skills-into-target-workspace.test.ts b/src/agents/skills.build-workspace-skills-prompt.syncs-merged-skills-into-target-workspace.test.ts index 9ad7efbe5..9b4ae4093 100644 --- a/src/agents/skills.build-workspace-skills-prompt.syncs-merged-skills-into-target-workspace.test.ts +++ b/src/agents/skills.build-workspace-skills-prompt.syncs-merged-skills-into-target-workspace.test.ts @@ -33,6 +33,12 @@ afterAll(async () => { }); describe("buildWorkspaceSkillsPrompt", () => { + const buildPrompt = ( + workspaceDir: string, + opts?: Parameters[1], + ) => + withEnv({ HOME: workspaceDir, PATH: "" }, () => buildWorkspaceSkillsPrompt(workspaceDir, opts)); + it("syncs merged skills into a target workspace", async () => { const sourceWorkspace = await createCaseDir("source"); const targetWorkspace = await createCaseDir("target"); @@ -61,15 +67,17 @@ describe("buildWorkspaceSkillsPrompt", () => { description: "Workspace version", }); - await syncSkillsToWorkspace({ - sourceWorkspaceDir: sourceWorkspace, - targetWorkspaceDir: targetWorkspace, - config: { skills: { load: { extraDirs: [extraDir] } } }, - bundledSkillsDir: bundledDir, - managedSkillsDir: managedDir, - }); + await withEnv({ HOME: sourceWorkspace, PATH: "" }, () => + syncSkillsToWorkspace({ + sourceWorkspaceDir: sourceWorkspace, + targetWorkspaceDir: targetWorkspace, + config: { skills: { load: { extraDirs: [extraDir] } } }, + bundledSkillsDir: bundledDir, + managedSkillsDir: managedDir, + }), + ); - const prompt = buildWorkspaceSkillsPrompt(targetWorkspace, { + const prompt = buildPrompt(targetWorkspace, { bundledSkillsDir: path.join(targetWorkspace, ".bundled"), managedSkillsDir: path.join(targetWorkspace, ".managed"), }); @@ -78,7 +86,7 @@ describe("buildWorkspaceSkillsPrompt", () => { expect(prompt).not.toContain("Managed version"); expect(prompt).not.toContain("Bundled version"); expect(prompt).not.toContain("Extra version"); - expect(prompt).toContain(path.join(targetWorkspace, "skills", "demo-skill", "SKILL.md")); + expect(prompt).toContain("demo-skill/SKILL.md"); }); it("keeps synced skills confined under target workspace when frontmatter name uses traversal", async () => { const sourceWorkspace = await createCaseDir("source"); @@ -98,12 +106,14 @@ describe("buildWorkspaceSkillsPrompt", () => { ); expect(await pathExists(escapedDest)).toBe(false); - await syncSkillsToWorkspace({ - sourceWorkspaceDir: sourceWorkspace, - targetWorkspaceDir: targetWorkspace, - bundledSkillsDir: path.join(sourceWorkspace, ".bundled"), - managedSkillsDir: path.join(sourceWorkspace, ".managed"), - }); + await withEnv({ HOME: sourceWorkspace, PATH: "" }, () => + syncSkillsToWorkspace({ + sourceWorkspaceDir: sourceWorkspace, + targetWorkspaceDir: targetWorkspace, + bundledSkillsDir: path.join(sourceWorkspace, ".bundled"), + managedSkillsDir: path.join(sourceWorkspace, ".managed"), + }), + ); expect( await pathExists(path.join(targetWorkspace, "skills", "safe-traversal-skill", "SKILL.md")), @@ -125,12 +135,14 @@ describe("buildWorkspaceSkillsPrompt", () => { expect(await pathExists(absoluteDest)).toBe(false); - await syncSkillsToWorkspace({ - sourceWorkspaceDir: sourceWorkspace, - targetWorkspaceDir: targetWorkspace, - bundledSkillsDir: path.join(sourceWorkspace, ".bundled"), - managedSkillsDir: path.join(sourceWorkspace, ".managed"), - }); + await withEnv({ HOME: sourceWorkspace, PATH: "" }, () => + syncSkillsToWorkspace({ + sourceWorkspaceDir: sourceWorkspace, + targetWorkspaceDir: targetWorkspace, + bundledSkillsDir: path.join(sourceWorkspace, ".bundled"), + managedSkillsDir: path.join(sourceWorkspace, ".managed"), + }), + ); expect( await pathExists(path.join(targetWorkspace, "skills", "safe-absolute-skill", "SKILL.md")), @@ -150,13 +162,13 @@ describe("buildWorkspaceSkillsPrompt", () => { }); withEnv({ GEMINI_API_KEY: undefined }, () => { - const missingPrompt = buildWorkspaceSkillsPrompt(workspaceDir, { + const missingPrompt = buildPrompt(workspaceDir, { managedSkillsDir: path.join(workspaceDir, ".managed"), config: { skills: { entries: { "nano-banana-pro": { apiKey: "" } } } }, }); expect(missingPrompt).not.toContain("nano-banana-pro"); - const enabledPrompt = buildWorkspaceSkillsPrompt(workspaceDir, { + const enabledPrompt = buildPrompt(workspaceDir, { managedSkillsDir: path.join(workspaceDir, ".managed"), config: { skills: { entries: { "nano-banana-pro": { apiKey: "test-key" } } }, @@ -178,14 +190,14 @@ describe("buildWorkspaceSkillsPrompt", () => { description: "Beta skill", }); - const filteredPrompt = buildWorkspaceSkillsPrompt(workspaceDir, { + const filteredPrompt = buildPrompt(workspaceDir, { managedSkillsDir: path.join(workspaceDir, ".managed"), skillFilter: ["alpha"], }); expect(filteredPrompt).toContain("alpha"); expect(filteredPrompt).not.toContain("beta"); - const emptyPrompt = buildWorkspaceSkillsPrompt(workspaceDir, { + const emptyPrompt = buildPrompt(workspaceDir, { managedSkillsDir: path.join(workspaceDir, ".managed"), skillFilter: [], }); diff --git a/src/agents/skills.buildworkspaceskillsnapshot.test.ts b/src/agents/skills.buildworkspaceskillsnapshot.test.ts index 5e24e31b0..9fec26d16 100644 --- a/src/agents/skills.buildworkspaceskillsnapshot.test.ts +++ b/src/agents/skills.buildworkspaceskillsnapshot.test.ts @@ -1,6 +1,7 @@ import fs from "node:fs/promises"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; +import { withEnv } from "../test-utils/env.js"; import { createTrackedTempDirs } from "../test-utils/tracked-temp-dirs.js"; import { writeSkill } from "./skills.e2e-test-helpers.js"; import { buildWorkspaceSkillSnapshot, buildWorkspaceSkillsPrompt } from "./skills.js"; @@ -11,14 +12,20 @@ afterEach(async () => { await tempDirs.cleanup(); }); +function withWorkspaceHome(workspaceDir: string, cb: () => T): T { + return withEnv({ HOME: workspaceDir, PATH: "" }, cb); +} + describe("buildWorkspaceSkillSnapshot", () => { it("returns an empty snapshot when skills dirs are missing", async () => { const workspaceDir = await tempDirs.make("openclaw-"); - const snapshot = buildWorkspaceSkillSnapshot(workspaceDir, { - managedSkillsDir: path.join(workspaceDir, ".managed"), - bundledSkillsDir: path.join(workspaceDir, ".bundled"), - }); + const snapshot = withWorkspaceHome(workspaceDir, () => + buildWorkspaceSkillSnapshot(workspaceDir, { + managedSkillsDir: path.join(workspaceDir, ".managed"), + bundledSkillsDir: path.join(workspaceDir, ".bundled"), + }), + ); expect(snapshot.prompt).toBe(""); expect(snapshot.skills).toEqual([]); @@ -38,10 +45,12 @@ describe("buildWorkspaceSkillSnapshot", () => { frontmatterExtra: "disable-model-invocation: true", }); - const snapshot = buildWorkspaceSkillSnapshot(workspaceDir, { - managedSkillsDir: path.join(workspaceDir, ".managed"), - bundledSkillsDir: path.join(workspaceDir, ".bundled"), - }); + const snapshot = withWorkspaceHome(workspaceDir, () => + buildWorkspaceSkillSnapshot(workspaceDir, { + managedSkillsDir: path.join(workspaceDir, ".managed"), + bundledSkillsDir: path.join(workspaceDir, ".bundled"), + }), + ); expect(snapshot.prompt).toContain("visible-skill"); expect(snapshot.prompt).not.toContain("hidden-skill"); @@ -86,8 +95,12 @@ describe("buildWorkspaceSkillSnapshot", () => { }, }; - const snapshot = buildWorkspaceSkillSnapshot(workspaceDir, opts); - const prompt = buildWorkspaceSkillsPrompt(workspaceDir, opts); + const snapshot = withWorkspaceHome(workspaceDir, () => + buildWorkspaceSkillSnapshot(workspaceDir, opts), + ); + const prompt = withWorkspaceHome(workspaceDir, () => + buildWorkspaceSkillsPrompt(workspaceDir, opts), + ); expect(snapshot.prompt).toBe(prompt); }); @@ -95,38 +108,40 @@ describe("buildWorkspaceSkillSnapshot", () => { it("truncates the skills prompt when it exceeds the configured char budget", async () => { const workspaceDir = await tempDirs.make("openclaw-"); - // Make a bunch of skills with very long descriptions. - for (let i = 0; i < 25; i += 1) { + // Keep fixture size modest while still forcing truncation logic. + for (let i = 0; i < 8; i += 1) { const name = `skill-${String(i).padStart(2, "0")}`; await writeSkill({ dir: path.join(workspaceDir, "skills", name), name, - description: "x".repeat(5000), + description: "x".repeat(800), }); } - const snapshot = buildWorkspaceSkillSnapshot(workspaceDir, { - config: { - skills: { - limits: { - maxSkillsInPrompt: 100, - maxSkillsPromptChars: 1500, + const snapshot = withWorkspaceHome(workspaceDir, () => + buildWorkspaceSkillSnapshot(workspaceDir, { + config: { + skills: { + limits: { + maxSkillsInPrompt: 100, + maxSkillsPromptChars: 500, + }, }, }, - }, - managedSkillsDir: path.join(workspaceDir, ".managed"), - bundledSkillsDir: path.join(workspaceDir, ".bundled"), - }); + managedSkillsDir: path.join(workspaceDir, ".managed"), + bundledSkillsDir: path.join(workspaceDir, ".bundled"), + }), + ); expect(snapshot.prompt).toContain("⚠️ Skills truncated"); - expect(snapshot.prompt.length).toBeLessThan(5000); + expect(snapshot.prompt.length).toBeLessThan(2000); }); it("limits discovery for nested repo-style skills roots (dir/skills/*)", async () => { const workspaceDir = await tempDirs.make("openclaw-"); const repoDir = await tempDirs.make("openclaw-skills-repo-"); - for (let i = 0; i < 20; i += 1) { + for (let i = 0; i < 8; i += 1) { const name = `repo-skill-${String(i).padStart(2, "0")}`; await writeSkill({ dir: path.join(repoDir, "skills", name), @@ -135,26 +150,28 @@ describe("buildWorkspaceSkillSnapshot", () => { }); } - const snapshot = buildWorkspaceSkillSnapshot(workspaceDir, { - config: { - skills: { - load: { - extraDirs: [repoDir], - }, - limits: { - maxCandidatesPerRoot: 5, - maxSkillsLoadedPerSource: 5, + const snapshot = withWorkspaceHome(workspaceDir, () => + buildWorkspaceSkillSnapshot(workspaceDir, { + config: { + skills: { + load: { + extraDirs: [repoDir], + }, + limits: { + maxCandidatesPerRoot: 5, + maxSkillsLoadedPerSource: 5, + }, }, }, - }, - managedSkillsDir: path.join(workspaceDir, ".managed"), - bundledSkillsDir: path.join(workspaceDir, ".bundled"), - }); + managedSkillsDir: path.join(workspaceDir, ".managed"), + bundledSkillsDir: path.join(workspaceDir, ".bundled"), + }), + ); // We should only have loaded a small subset. expect(snapshot.skills.length).toBeLessThanOrEqual(5); expect(snapshot.prompt).toContain("repo-skill-00"); - expect(snapshot.prompt).not.toContain("repo-skill-19"); + expect(snapshot.prompt).not.toContain("repo-skill-07"); }); it("skips skills whose SKILL.md exceeds maxSkillFileBytes", async () => { @@ -170,20 +187,22 @@ describe("buildWorkspaceSkillSnapshot", () => { dir: path.join(workspaceDir, "skills", "big-skill"), name: "big-skill", description: "Big", - body: "x".repeat(50_000), + body: "x".repeat(5_000), }); - const snapshot = buildWorkspaceSkillSnapshot(workspaceDir, { - config: { - skills: { - limits: { - maxSkillFileBytes: 1000, + const snapshot = withWorkspaceHome(workspaceDir, () => + buildWorkspaceSkillSnapshot(workspaceDir, { + config: { + skills: { + limits: { + maxSkillFileBytes: 1000, + }, }, }, - }, - managedSkillsDir: path.join(workspaceDir, ".managed"), - bundledSkillsDir: path.join(workspaceDir, ".bundled"), - }); + managedSkillsDir: path.join(workspaceDir, ".managed"), + bundledSkillsDir: path.join(workspaceDir, ".bundled"), + }), + ); expect(snapshot.skills.map((s) => s.name)).toContain("small-skill"); expect(snapshot.skills.map((s) => s.name)).not.toContain("big-skill"); @@ -208,21 +227,23 @@ describe("buildWorkspaceSkillSnapshot", () => { description: "Nested skill discovered late", }); - const snapshot = buildWorkspaceSkillSnapshot(workspaceDir, { - config: { - skills: { - load: { - extraDirs: [repoDir], - }, - limits: { - maxCandidatesPerRoot: 30, - maxSkillsLoadedPerSource: 30, + const snapshot = withWorkspaceHome(workspaceDir, () => + buildWorkspaceSkillSnapshot(workspaceDir, { + config: { + skills: { + load: { + extraDirs: [repoDir], + }, + limits: { + maxCandidatesPerRoot: 30, + maxSkillsLoadedPerSource: 30, + }, }, }, - }, - managedSkillsDir: path.join(workspaceDir, ".managed"), - bundledSkillsDir: path.join(workspaceDir, ".bundled"), - }); + managedSkillsDir: path.join(workspaceDir, ".managed"), + bundledSkillsDir: path.join(workspaceDir, ".bundled"), + }), + ); expect(snapshot.skills.map((s) => s.name)).toContain("late-skill"); expect(snapshot.prompt).toContain("late-skill"); @@ -236,23 +257,25 @@ describe("buildWorkspaceSkillSnapshot", () => { dir: rootSkillDir, name: "root-big-skill", description: "Big", - body: "x".repeat(50_000), + body: "x".repeat(5_000), }); - const snapshot = buildWorkspaceSkillSnapshot(workspaceDir, { - config: { - skills: { - load: { - extraDirs: [rootSkillDir], - }, - limits: { - maxSkillFileBytes: 1000, + const snapshot = withWorkspaceHome(workspaceDir, () => + buildWorkspaceSkillSnapshot(workspaceDir, { + config: { + skills: { + load: { + extraDirs: [rootSkillDir], + }, + limits: { + maxSkillFileBytes: 1000, + }, }, }, - }, - managedSkillsDir: path.join(workspaceDir, ".managed"), - bundledSkillsDir: path.join(workspaceDir, ".bundled"), - }); + managedSkillsDir: path.join(workspaceDir, ".managed"), + bundledSkillsDir: path.join(workspaceDir, ".bundled"), + }), + ); expect(snapshot.skills.map((s) => s.name)).not.toContain("root-big-skill"); expect(snapshot.prompt).not.toContain("root-big-skill"); diff --git a/src/agents/skills.buildworkspaceskillstatus.test.ts b/src/agents/skills.buildworkspaceskillstatus.test.ts index 2a3b4cff4..c8b7c220e 100644 --- a/src/agents/skills.buildworkspaceskillstatus.test.ts +++ b/src/agents/skills.buildworkspaceskillstatus.test.ts @@ -1,28 +1,68 @@ -import fs from "node:fs/promises"; -import os from "node:os"; -import path from "node:path"; import { describe, expect, it } from "vitest"; import { withEnv } from "../test-utils/env.js"; import { buildWorkspaceSkillStatus } from "./skills-status.js"; -import { writeSkill } from "./skills.e2e-test-helpers.js"; +import type { SkillEntry } from "./skills/types.js"; + +function makeEntry(params: { + name: string; + source?: string; + os?: string[]; + requires?: { bins?: string[]; env?: string[]; config?: string[] }; + install?: Array<{ + id: string; + kind: "brew" | "download"; + bins?: string[]; + formula?: string; + os?: string[]; + url?: string; + label?: string; + }>; +}): SkillEntry { + return { + skill: { + name: params.name, + description: `desc:${params.name}`, + source: params.source ?? "openclaw-workspace", + filePath: `/tmp/${params.name}/SKILL.md`, + baseDir: `/tmp/${params.name}`, + disableModelInvocation: false, + }, + frontmatter: {}, + metadata: { + ...(params.os ? { os: params.os } : {}), + ...(params.requires ? { requires: params.requires } : {}), + ...(params.install ? { install: params.install } : {}), + ...(params.requires?.env?.[0] ? { primaryEnv: params.requires.env[0] } : {}), + }, + }; +} describe("buildWorkspaceSkillStatus", () => { it("reports missing requirements and install options", async () => { - const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-")); - const skillDir = path.join(workspaceDir, "skills", "status-skill"); - - await writeSkill({ - dir: skillDir, + const entry = makeEntry({ name: "status-skill", - description: "Needs setup", - metadata: - '{"openclaw":{"requires":{"bins":["fakebin"],"env":["ENV_KEY"],"config":["browser.enabled"]},"install":[{"id":"brew","kind":"brew","formula":"fakebin","bins":["fakebin"],"label":"Install fakebin"}]}}', + requires: { + bins: ["fakebin"], + env: ["ENV_KEY"], + config: ["browser.enabled"], + }, + install: [ + { + id: "brew", + kind: "brew", + formula: "fakebin", + bins: ["fakebin"], + label: "Install fakebin", + }, + ], }); - const report = buildWorkspaceSkillStatus(workspaceDir, { - managedSkillsDir: path.join(workspaceDir, ".managed"), - config: { browser: { enabled: false } }, - }); + const report = withEnv({ PATH: "" }, () => + buildWorkspaceSkillStatus("/tmp/ws", { + entries: [entry], + config: { browser: { enabled: false } }, + }), + ); const skill = report.skills.find((entry) => entry.name === "status-skill"); expect(skill).toBeDefined(); @@ -33,19 +73,12 @@ describe("buildWorkspaceSkillStatus", () => { expect(skill?.install[0]?.id).toBe("brew"); }); it("respects OS-gated skills", async () => { - const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-")); - const skillDir = path.join(workspaceDir, "skills", "os-skill"); - - await writeSkill({ - dir: skillDir, + const entry = makeEntry({ name: "os-skill", - description: "Darwin only", - metadata: '{"openclaw":{"os":["darwin"]}}', + os: ["darwin"], }); - const report = buildWorkspaceSkillStatus(workspaceDir, { - managedSkillsDir: path.join(workspaceDir, ".managed"), - }); + const report = buildWorkspaceSkillStatus("/tmp/ws", { entries: [entry] }); const skill = report.skills.find((entry) => entry.name === "os-skill"); expect(skill).toBeDefined(); @@ -58,46 +91,57 @@ describe("buildWorkspaceSkillStatus", () => { } }); it("marks bundled skills blocked by allowlist", async () => { - const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-")); - const bundledDir = path.join(workspaceDir, ".bundled"); - const bundledSkillDir = path.join(bundledDir, "peekaboo"); - - await writeSkill({ - dir: bundledSkillDir, + const entry = makeEntry({ name: "peekaboo", - description: "Capture UI", - body: "# Peekaboo\n", + source: "openclaw-bundled", }); - withEnv({ OPENCLAW_BUNDLED_SKILLS_DIR: bundledDir }, () => { - const report = buildWorkspaceSkillStatus(workspaceDir, { - managedSkillsDir: path.join(workspaceDir, ".managed"), - config: { skills: { allowBundled: ["other-skill"] } }, - }); - const skill = report.skills.find((entry) => entry.name === "peekaboo"); - - expect(skill).toBeDefined(); - expect(skill?.blockedByAllowlist).toBe(true); - expect(skill?.eligible).toBe(false); + const report = buildWorkspaceSkillStatus("/tmp/ws", { + entries: [entry], + config: { skills: { allowBundled: ["other-skill"] } }, }); + const skill = report.skills.find((reportEntry) => reportEntry.name === "peekaboo"); + + expect(skill).toBeDefined(); + expect(skill?.blockedByAllowlist).toBe(true); + expect(skill?.eligible).toBe(false); + expect(skill?.bundled).toBe(true); }); it("filters install options by OS", async () => { - const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-")); - const skillDir = path.join(workspaceDir, "skills", "install-skill"); - - await writeSkill({ - dir: skillDir, + const entry = makeEntry({ name: "install-skill", - description: "OS-specific installs", - metadata: - '{"openclaw":{"requires":{"bins":["missing-bin"]},"install":[{"id":"mac","kind":"download","os":["darwin"],"url":"https://example.com/mac.tar.bz2"},{"id":"linux","kind":"download","os":["linux"],"url":"https://example.com/linux.tar.bz2"},{"id":"win","kind":"download","os":["win32"],"url":"https://example.com/win.tar.bz2"}]}}', + requires: { + bins: ["missing-bin"], + }, + install: [ + { + id: "mac", + kind: "download", + os: ["darwin"], + url: "https://example.com/mac.tar.bz2", + }, + { + id: "linux", + kind: "download", + os: ["linux"], + url: "https://example.com/linux.tar.bz2", + }, + { + id: "win", + kind: "download", + os: ["win32"], + url: "https://example.com/win.tar.bz2", + }, + ], }); - const report = buildWorkspaceSkillStatus(workspaceDir, { - managedSkillsDir: path.join(workspaceDir, ".managed"), - }); - const skill = report.skills.find((entry) => entry.name === "install-skill"); + const report = withEnv({ PATH: "" }, () => + buildWorkspaceSkillStatus("/tmp/ws", { + entries: [entry], + }), + ); + const skill = report.skills.find((reportEntry) => reportEntry.name === "install-skill"); expect(skill).toBeDefined(); if (process.platform === "darwin") {