diff --git a/packages/workspace-server/src/services/skills/skill-bundler.test.ts b/packages/workspace-server/src/services/skills/skill-bundler.test.ts new file mode 100644 index 0000000000..40e243004a --- /dev/null +++ b/packages/workspace-server/src/services/skills/skill-bundler.test.ts @@ -0,0 +1,81 @@ +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { unzipSync } from "fflate"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { bundleLocalSkill } from "./skill-bundler"; + +let root: string; +let skillPath: string; + +beforeEach(async () => { + root = await mkdtemp(path.join(tmpdir(), "skill-bundler-test-")); + skillPath = path.join(root, "alpha"); + await mkdir(skillPath, { recursive: true }); + await writeFile(path.join(skillPath, "SKILL.md"), "# alpha"); +}); + +afterEach(async () => { + await rm(root, { recursive: true, force: true }); +}); + +async function bundledFileNames(): Promise { + const bundle = await bundleLocalSkill({ + name: "alpha", + source: "user", + skillPath, + }); + const entries = unzipSync(Buffer.from(bundle.contentBase64, "base64")); + return Object.keys(entries).sort(); +} + +async function writeManyFiles(dir: string, count: number): Promise { + await mkdir(dir, { recursive: true }); + await Promise.all( + Array.from({ length: count }, (_, i) => + writeFile(path.join(dir, `file-${i}.txt`), "x"), + ), + ); +} + +describe("bundleLocalSkill", () => { + it("excludes hidden directories and junk but keeps hidden files and nested content", async () => { + await mkdir(path.join(skillPath, ".venv", "lib"), { recursive: true }); + await writeFile(path.join(skillPath, ".venv", "lib", "site.py"), "x"); + await mkdir(path.join(skillPath, "node_modules", "pkg"), { + recursive: true, + }); + await writeFile(path.join(skillPath, "node_modules", "pkg", "i.js"), "x"); + await mkdir(path.join(skillPath, "references", "deep"), { + recursive: true, + }); + await writeFile(path.join(skillPath, "references", "deep", "x.md"), "xx"); + await writeFile(path.join(skillPath, ".gitignore"), "*.log"); + + expect(await bundledFileNames()).toEqual([ + ".gitignore", + "SKILL.md", + "posthog-skill-bundle.json", + "references/deep/x.md", + ]); + }); + + it("does not count files inside hidden directories toward the file limit", async () => { + await writeManyFiles(path.join(skillPath, ".venv"), 1100); + + expect(await bundledFileNames()).toEqual([ + "SKILL.md", + "posthog-skill-bundle.json", + ]); + }); + + it("names the skill and path when the file limit is exceeded", async () => { + await writeManyFiles(path.join(skillPath, "data"), 1000); + + await expect( + bundleLocalSkill({ name: "alpha", source: "user", skillPath }), + ).rejects.toThrow( + /Skill "alpha" \(.*alpha\) contains more than 1000 files/, + ); + }); +}); diff --git a/packages/workspace-server/src/services/skills/skill-bundler.ts b/packages/workspace-server/src/services/skills/skill-bundler.ts index 7bfe2301a8..7b621b041c 100644 --- a/packages/workspace-server/src/services/skills/skill-bundler.ts +++ b/packages/workspace-server/src/services/skills/skill-bundler.ts @@ -3,15 +3,10 @@ import * as fs from "node:fs"; import * as path from "node:path"; import { strToU8, zipSync } from "fflate"; import type { BundleLocalSkillOutput, UploadableSkillSource } from "./schemas"; +import { isIgnoredSkillEntry } from "./skill-discovery"; const SKILL_BUNDLE_MAX_BYTES = 30 * 1024 * 1024; const SKILL_BUNDLE_MAX_FILES = 1000; -const IGNORED_ENTRIES = new Set([ - ".DS_Store", - ".git", - "node_modules", - "__pycache__", -]); function toZipPath(filePath: string): string { return filePath.split(path.sep).join("/"); @@ -42,6 +37,8 @@ function isInsideRoot(root: string, candidate: string): boolean { } interface SkillFileAccumulator { + skillName: string; + root: string; files: Record; totalBytes: number; } @@ -54,11 +51,15 @@ async function addSkillFile( ): Promise { if (Object.keys(acc.files).length >= SKILL_BUNDLE_MAX_FILES) { throw new Error( - `Local skill bundle contains more than ${SKILL_BUNDLE_MAX_FILES} files`, + `Skill "${acc.skillName}" (${acc.root}) contains more than ` + + `${SKILL_BUNDLE_MAX_FILES} files. Cloud runs upload every file in ` + + `the skill folder, so move data and build artifacts out of it.`, ); } if (acc.totalBytes + size > SKILL_BUNDLE_MAX_BYTES) { - throw new Error("Local skill bundle exceeds the 30MB cloud run limit"); + throw new Error( + `Skill "${acc.skillName}" (${acc.root}) exceeds the 30MB cloud run upload limit`, + ); } const content = await fs.promises.readFile(sourcePath); acc.files[toZipPath(relativePath)] = new Uint8Array(content); @@ -66,7 +67,6 @@ async function addSkillFile( } async function collectSkillFiles( - root: string, currentDir: string, acc: SkillFileAccumulator, ): Promise { @@ -75,12 +75,12 @@ async function collectSkillFiles( }); for (const entry of entries) { - if (IGNORED_ENTRIES.has(entry.name)) { + if (isIgnoredSkillEntry(entry)) { continue; } const absolutePath = path.join(currentDir, entry.name); - const relativePath = path.relative(root, absolutePath); + const relativePath = path.relative(acc.root, absolutePath); if ( !relativePath || relativePath.startsWith("..") || @@ -93,7 +93,7 @@ async function collectSkillFiles( const realPath = await fs.promises .realpath(absolutePath) .catch(() => null); - if (!realPath || !isInsideRoot(root, realPath)) { + if (!realPath || !isInsideRoot(acc.root, realPath)) { continue; } const stat = await fs.promises.stat(realPath); @@ -105,7 +105,7 @@ async function collectSkillFiles( } if (entry.isDirectory()) { - await collectSkillFiles(root, absolutePath, acc); + await collectSkillFiles(absolutePath, acc); continue; } @@ -128,8 +128,13 @@ export async function bundleLocalSkill({ skillPath: string; }): Promise { const root = await assertSkillRoot(skillPath); - const acc: SkillFileAccumulator = { files: {}, totalBytes: 0 }; - await collectSkillFiles(root, root, acc); + const acc: SkillFileAccumulator = { + skillName: name, + root, + files: {}, + totalBytes: 0, + }; + await collectSkillFiles(root, acc); const files = acc.files; const fileNames = Object.keys(files).sort(); @@ -152,7 +157,7 @@ export async function bundleLocalSkill({ const zipped = zipSync(zipInput, { level: 6 }); if (zipped.byteLength > SKILL_BUNDLE_MAX_BYTES) { throw new Error( - "Local skill bundle archive exceeds the 30MB cloud run limit", + `Skill "${name}" (${root}) zip archive exceeds the 30MB cloud run upload limit`, ); } diff --git a/packages/workspace-server/src/services/skills/skill-discovery.test.ts b/packages/workspace-server/src/services/skills/skill-discovery.test.ts index 944ae0959f..56553dd3b5 100644 --- a/packages/workspace-server/src/services/skills/skill-discovery.test.ts +++ b/packages/workspace-server/src/services/skills/skill-discovery.test.ts @@ -140,6 +140,23 @@ describe("listSkillFiles", () => { expect(files.map((f) => f.path)).toEqual(["SKILL.md"]); }); + it("skips hidden directories and junk but keeps hidden files", async () => { + const skillsDir = path.join(root, "skills"); + await createSkill(skillsDir, "alpha", "hello"); + const skillPath = path.join(skillsDir, "alpha"); + await mkdir(path.join(skillPath, ".venv", "lib"), { recursive: true }); + await writeFile(path.join(skillPath, ".venv", "lib", "site.py"), "x"); + await mkdir(path.join(skillPath, "node_modules", "pkg"), { + recursive: true, + }); + await writeFile(path.join(skillPath, "node_modules", "pkg", "i.js"), "x"); + await writeFile(path.join(skillPath, ".gitignore"), "*.log"); + + const files = await listSkillFiles(skillPath, 500); + + expect(files.map((f) => f.path)).toEqual(["SKILL.md", ".gitignore"]); + }); + it("stops at the file cap", async () => { const skillsDir = path.join(root, "skills"); await createSkill(skillsDir, "alpha"); diff --git a/packages/workspace-server/src/services/skills/skill-discovery.ts b/packages/workspace-server/src/services/skills/skill-discovery.ts index 7bb0f7c679..01c82af8ff 100644 --- a/packages/workspace-server/src/services/skills/skill-discovery.ts +++ b/packages/workspace-server/src/services/skills/skill-discovery.ts @@ -47,6 +47,19 @@ export function isProbablyText(bytes: Uint8Array): boolean { return !bytes.subarray(0, 4096).includes(0); } +const IGNORED_SKILL_ENTRIES = new Set([ + ".DS_Store", + ".git", + "node_modules", + "__pycache__", +]); + +/** Junk names and hidden directories are never skill content; hidden files count. */ +export function isIgnoredSkillEntry(entry: fs.Dirent): boolean { + if (IGNORED_SKILL_ENTRIES.has(entry.name)) return true; + return entry.isDirectory() && entry.name.startsWith("."); +} + export async function findSkillDirs( sourceSkillsDir: string, ): Promise { @@ -135,7 +148,8 @@ export async function getMarketplaceInstallPaths(): Promise { /** * Recursively lists regular files inside a skill directory. Symlinks are - * skipped so a crafted skill cannot expose files outside its directory. + * skipped so a crafted skill cannot expose files outside its directory; + * hidden directories and junk entries are skipped to match cloud bundles. */ export async function listSkillFiles( skillDir: string, @@ -147,6 +161,7 @@ export async function listSkillFiles( const entries = await fs.promises.readdir(dir, { withFileTypes: true }); for (const entry of entries) { if (files.length >= maxFiles) return; + if (isIgnoredSkillEntry(entry)) continue; const relPath = prefix ? `${prefix}/${entry.name}` : entry.name; const fullPath = path.join(dir, entry.name); if (entry.isDirectory()) {