diff options
Diffstat (limited to 'packages/skills/src')
| -rw-r--r-- | packages/skills/src/extension.ts | 34 | ||||
| -rw-r--r-- | packages/skills/src/index.ts | 16 | ||||
| -rw-r--r-- | packages/skills/src/load-skill.ts | 268 | ||||
| -rw-r--r-- | packages/skills/src/pure.test.ts | 300 | ||||
| -rw-r--r-- | packages/skills/src/pure.ts | 106 | ||||
| -rw-r--r-- | packages/skills/src/skills.test.ts | 478 | ||||
| -rw-r--r-- | packages/skills/src/tools-filter.ts | 102 |
7 files changed, 652 insertions, 652 deletions
diff --git a/packages/skills/src/extension.ts b/packages/skills/src/extension.ts index e8000e4..1f16586 100644 --- a/packages/skills/src/extension.ts +++ b/packages/skills/src/extension.ts @@ -5,22 +5,22 @@ import { createLoadSkillTool } from "./load-skill.js"; import { makeSkillsToolFilter } from "./tools-filter.js"; export const extension: Extension = { - manifest: { - id: "skills", - name: "Skills", - version: "0.0.0", - apiVersion: "^0.1.0", - trust: "bundled", - activation: "eager", - dependsOn: ["session-orchestrator"], - capabilities: { fs: true }, - contributes: { tools: ["load_skill"] }, - }, - activate(host: HostAPI) { - const homeDir = homedir(); - const workdir = process.cwd(); + manifest: { + id: "skills", + name: "Skills", + version: "0.0.0", + apiVersion: "^0.1.0", + trust: "bundled", + activation: "eager", + dependsOn: ["session-orchestrator"], + capabilities: { fs: true }, + contributes: { tools: ["load_skill"] }, + }, + activate(host: HostAPI) { + const homeDir = homedir(); + const workdir = process.cwd(); - host.defineTool(createLoadSkillTool({ homeDir, workdir })); - host.addFilter(toolsFilter, makeSkillsToolFilter({ homeDir, workdir })); - }, + host.defineTool(createLoadSkillTool({ homeDir, workdir })); + host.addFilter(toolsFilter, makeSkillsToolFilter({ homeDir, workdir })); + }, }; diff --git a/packages/skills/src/index.ts b/packages/skills/src/index.ts index 12f9f19..ab6b574 100644 --- a/packages/skills/src/index.ts +++ b/packages/skills/src/index.ts @@ -1,13 +1,13 @@ export { extension } from "./extension.js"; export { createLoadSkillTool, type SkillsDeps, scanSkillsDir } from "./load-skill.js"; export { - isPathWithinDir, - isValidSkillName, - mergeCatalog, - parseSkillMeta, - renderDescription, - type SkillEntry, - type SkillMeta, - stripLoadedBody, + isPathWithinDir, + isValidSkillName, + mergeCatalog, + parseSkillMeta, + renderDescription, + type SkillEntry, + type SkillMeta, + stripLoadedBody, } from "./pure.js"; export { makeSkillsToolFilter } from "./tools-filter.js"; diff --git a/packages/skills/src/load-skill.ts b/packages/skills/src/load-skill.ts index ef7fc23..3a6173c 100644 --- a/packages/skills/src/load-skill.ts +++ b/packages/skills/src/load-skill.ts @@ -2,16 +2,16 @@ import { readdir, readFile } from "node:fs/promises"; import { join, resolve } from "node:path"; import type { ToolContract, ToolExecuteContext, ToolResult } from "@dispatch/kernel"; import { - isPathWithinDir, - isValidSkillName, - parseSkillMeta, - type SkillEntry, - stripLoadedBody, + isPathWithinDir, + isValidSkillName, + parseSkillMeta, + type SkillEntry, + stripLoadedBody, } from "./pure.js"; export interface SkillsDeps { - readonly homeDir: string; - readonly workdir: string; + readonly homeDir: string; + readonly workdir: string; } /** @@ -22,36 +22,36 @@ export interface SkillsDeps { * Returns an empty array on any error (fail-open). */ export async function scanSkillsDir(dir: string): Promise<readonly SkillEntry[]> { - async function scan(d: string): Promise<SkillEntry[]> { - try { - const entries = await readdir(d, { encoding: "utf8", withFileTypes: true }); - const skills: SkillEntry[] = []; - for (const entry of entries) { - if (entry.isDirectory()) { - const subSkills = await scan(join(d, entry.name)); - for (const sub of subSkills) { - if (!skills.some((s) => s.name === sub.name)) { - skills.push(sub); - } - } - } else if (entry.isFile() && entry.name.endsWith(".md")) { - const name = entry.name.slice(0, -3); - if (skills.some((s) => s.name === name)) continue; - try { - const content = await readFile(join(d, entry.name), "utf8"); - const meta = parseSkillMeta(content); - skills.push({ name, summary: meta.hasMeta ? meta.summary : undefined }); - } catch { - skills.push({ name }); - } - } - } - return skills; - } catch { - return []; - } - } - return scan(dir); + async function scan(d: string): Promise<SkillEntry[]> { + try { + const entries = await readdir(d, { encoding: "utf8", withFileTypes: true }); + const skills: SkillEntry[] = []; + for (const entry of entries) { + if (entry.isDirectory()) { + const subSkills = await scan(join(d, entry.name)); + for (const sub of subSkills) { + if (!skills.some((s) => s.name === sub.name)) { + skills.push(sub); + } + } + } else if (entry.isFile() && entry.name.endsWith(".md")) { + const name = entry.name.slice(0, -3); + if (skills.some((s) => s.name === name)) continue; + try { + const content = await readFile(join(d, entry.name), "utf8"); + const meta = parseSkillMeta(content); + skills.push({ name, summary: meta.hasMeta ? meta.summary : undefined }); + } catch { + skills.push({ name }); + } + } + } + return skills; + } catch { + return []; + } + } + return scan(dir); } /** @@ -60,26 +60,26 @@ export async function scanSkillsDir(dir: string): Promise<readonly SkillEntry[]> * Top-level files are found before nested ones (readdir order within each level). */ async function findSkillFile(dir: string, name: string): Promise<string | null> { - async function search(d: string): Promise<string | null> { - try { - const entries = await readdir(d, { encoding: "utf8", withFileTypes: true }); - for (const entry of entries) { - if (entry.isFile() && entry.name === `${name}.md`) { - return join(d, entry.name); - } - } - for (const entry of entries) { - if (entry.isDirectory()) { - const found = await search(join(d, entry.name)); - if (found !== null) return found; - } - } - return null; - } catch { - return null; - } - } - return search(dir); + async function search(d: string): Promise<string | null> { + try { + const entries = await readdir(d, { encoding: "utf8", withFileTypes: true }); + for (const entry of entries) { + if (entry.isFile() && entry.name === `${name}.md`) { + return join(d, entry.name); + } + } + for (const entry of entries) { + if (entry.isDirectory()) { + const found = await search(join(d, entry.name)); + if (found !== null) return found; + } + } + return null; + } catch { + return null; + } + } + return search(dir); } /** @@ -87,81 +87,81 @@ async function findSkillFile(dir: string, name: string): Promise<string | null> * The tool reads a skill file from disk on execute (uncached). */ export function createLoadSkillTool(deps: SkillsDeps): ToolContract { - const { homeDir, workdir } = deps; - - return { - name: "load_skill", - description: "Load a skill by name. No skills are currently available.", - parameters: { - type: "object", - properties: { - name: { - type: "string", - description: "The name of the skill to load.", - }, - }, - required: ["name"], - }, - concurrencySafe: true, - async execute(args: unknown, ctx: ToolExecuteContext): Promise<ToolResult> { - const obj = args as Record<string, unknown>; - const rawName = obj?.name; - - if (typeof rawName !== "string") { - return { content: 'Error: Missing or invalid "name" parameter.', isError: true }; - } - - if (!isValidSkillName(rawName)) { - return { - content: `Error: Invalid skill name "${rawName}". Name must not contain path separators or "..".`, - isError: true, - }; - } - - const effectiveBase = ctx.cwd ? resolve(ctx.cwd) : resolve(workdir); - const cwdSkillsDir = join(effectiveBase, ".skills"); - const homeSkillsDir = join(resolve(homeDir), ".skills"); - - let filePath: string | null = null; - - try { - filePath = await findSkillFile(cwdSkillsDir, rawName); - } catch { - // Cwd miss — try home - } - - if (filePath === null) { - try { - filePath = await findSkillFile(homeSkillsDir, rawName); - } catch { - // Both miss - } - } - - if (filePath === null) { - return { content: `Error: unknown skill: ${rawName}`, isError: true }; - } - - const resolvedPath = resolve(filePath); - if ( - !isPathWithinDir(resolvedPath, cwdSkillsDir) && - !isPathWithinDir(resolvedPath, homeSkillsDir) - ) { - return { content: "Error: Invalid skill path.", isError: true }; - } - - let content: string; - - try { - content = await readFile(resolvedPath, "utf8"); - } catch { - return { content: `Error: unknown skill: ${rawName}`, isError: true }; - } - - const meta = parseSkillMeta(content); - const body = stripLoadedBody(content, meta.hasMeta); - - return { content: body }; - }, - }; + const { homeDir, workdir } = deps; + + return { + name: "load_skill", + description: "Load a skill by name. No skills are currently available.", + parameters: { + type: "object", + properties: { + name: { + type: "string", + description: "The name of the skill to load.", + }, + }, + required: ["name"], + }, + concurrencySafe: true, + async execute(args: unknown, ctx: ToolExecuteContext): Promise<ToolResult> { + const obj = args as Record<string, unknown>; + const rawName = obj?.name; + + if (typeof rawName !== "string") { + return { content: 'Error: Missing or invalid "name" parameter.', isError: true }; + } + + if (!isValidSkillName(rawName)) { + return { + content: `Error: Invalid skill name "${rawName}". Name must not contain path separators or "..".`, + isError: true, + }; + } + + const effectiveBase = ctx.cwd ? resolve(ctx.cwd) : resolve(workdir); + const cwdSkillsDir = join(effectiveBase, ".skills"); + const homeSkillsDir = join(resolve(homeDir), ".skills"); + + let filePath: string | null = null; + + try { + filePath = await findSkillFile(cwdSkillsDir, rawName); + } catch { + // Cwd miss — try home + } + + if (filePath === null) { + try { + filePath = await findSkillFile(homeSkillsDir, rawName); + } catch { + // Both miss + } + } + + if (filePath === null) { + return { content: `Error: unknown skill: ${rawName}`, isError: true }; + } + + const resolvedPath = resolve(filePath); + if ( + !isPathWithinDir(resolvedPath, cwdSkillsDir) && + !isPathWithinDir(resolvedPath, homeSkillsDir) + ) { + return { content: "Error: Invalid skill path.", isError: true }; + } + + let content: string; + + try { + content = await readFile(resolvedPath, "utf8"); + } catch { + return { content: `Error: unknown skill: ${rawName}`, isError: true }; + } + + const meta = parseSkillMeta(content); + const body = stripLoadedBody(content, meta.hasMeta); + + return { content: body }; + }, + }; } diff --git a/packages/skills/src/pure.test.ts b/packages/skills/src/pure.test.ts index a8c6af5..1fdadfd 100644 --- a/packages/skills/src/pure.test.ts +++ b/packages/skills/src/pure.test.ts @@ -1,174 +1,174 @@ import { describe, expect, it } from "vitest"; import { - isPathWithinDir, - isValidSkillName, - mergeCatalog, - parseSkillMeta, - renderDescription, - stripLoadedBody, + isPathWithinDir, + isValidSkillName, + mergeCatalog, + parseSkillMeta, + renderDescription, + stripLoadedBody, } from "./pure.js"; describe("parseSkillMeta", () => { - it("extracts summary when line 2 is ---", () => { - const content = "Use this for web searches\n---\nBody content here"; - const result = parseSkillMeta(content); - expect(result.hasMeta).toBe(true); - expect(result.summary).toBe("Use this for web searches"); - }); - - it("extracts summary with trailing whitespace on separator", () => { - const content = "Summary text\n--- \nBody"; - const result = parseSkillMeta(content); - expect(result.hasMeta).toBe(true); - expect(result.summary).toBe("Summary text"); - }); - - it("reports no metadata when line 2 is not ---", () => { - const content = "Some content\nNot a separator\nBody"; - const result = parseSkillMeta(content); - expect(result.hasMeta).toBe(false); - expect(result.summary).toBeUndefined(); - }); - - it("reports no metadata for single-line content", () => { - const content = "Only one line"; - const result = parseSkillMeta(content); - expect(result.hasMeta).toBe(false); - }); - - it("reports no metadata for empty content", () => { - const result = parseSkillMeta(""); - expect(result.hasMeta).toBe(false); - }); - - it("treats empty summary as undefined", () => { - const content = "\n---\nBody"; - const result = parseSkillMeta(content); - expect(result.hasMeta).toBe(true); - expect(result.summary).toBeUndefined(); - }); + it("extracts summary when line 2 is ---", () => { + const content = "Use this for web searches\n---\nBody content here"; + const result = parseSkillMeta(content); + expect(result.hasMeta).toBe(true); + expect(result.summary).toBe("Use this for web searches"); + }); + + it("extracts summary with trailing whitespace on separator", () => { + const content = "Summary text\n--- \nBody"; + const result = parseSkillMeta(content); + expect(result.hasMeta).toBe(true); + expect(result.summary).toBe("Summary text"); + }); + + it("reports no metadata when line 2 is not ---", () => { + const content = "Some content\nNot a separator\nBody"; + const result = parseSkillMeta(content); + expect(result.hasMeta).toBe(false); + expect(result.summary).toBeUndefined(); + }); + + it("reports no metadata for single-line content", () => { + const content = "Only one line"; + const result = parseSkillMeta(content); + expect(result.hasMeta).toBe(false); + }); + + it("reports no metadata for empty content", () => { + const result = parseSkillMeta(""); + expect(result.hasMeta).toBe(false); + }); + + it("treats empty summary as undefined", () => { + const content = "\n---\nBody"; + const result = parseSkillMeta(content); + expect(result.hasMeta).toBe(true); + expect(result.summary).toBeUndefined(); + }); }); describe("stripLoadedBody", () => { - it("removes the first two lines when metadata is present", () => { - const content = "Summary\n---\nLine 3\nLine 4"; - const result = stripLoadedBody(content, true); - expect(result).toBe("Line 3\nLine 4"); - }); - - it("returns the whole file when malformed", () => { - const content = "No metadata here\nJust content"; - const result = stripLoadedBody(content, false); - expect(result).toBe("No metadata here\nJust content"); - }); - - it("handles file with only metadata and no body", () => { - const content = "Summary\n---\n"; - const result = stripLoadedBody(content, true); - expect(result).toBe(""); - }); + it("removes the first two lines when metadata is present", () => { + const content = "Summary\n---\nLine 3\nLine 4"; + const result = stripLoadedBody(content, true); + expect(result).toBe("Line 3\nLine 4"); + }); + + it("returns the whole file when malformed", () => { + const content = "No metadata here\nJust content"; + const result = stripLoadedBody(content, false); + expect(result).toBe("No metadata here\nJust content"); + }); + + it("handles file with only metadata and no body", () => { + const content = "Summary\n---\n"; + const result = stripLoadedBody(content, true); + expect(result).toBe(""); + }); }); describe("mergeCatalog", () => { - it("merges disjoint entries", () => { - const home = [{ name: "a" }, { name: "b" }]; - const cwd = [{ name: "c" }]; - const result = mergeCatalog(home, cwd); - expect(result.map((e) => e.name)).toEqual(["a", "b", "c"]); - }); - - it("cwd skill shadows home skill of the same name", () => { - const home = [{ name: "shared", summary: "home summary" }]; - const cwd = [{ name: "shared", summary: "cwd summary" }]; - const result = mergeCatalog(home, cwd); - expect(result).toHaveLength(1); - expect(result[0]?.summary).toBe("cwd summary"); - }); - - it("sorts by name", () => { - const home = [{ name: "z" }, { name: "a" }]; - const cwd = [{ name: "m" }]; - const result = mergeCatalog(home, cwd); - expect(result.map((e) => e.name)).toEqual(["a", "m", "z"]); - }); - - it("handles empty inputs", () => { - expect(mergeCatalog([], [])).toEqual([]); - }); + it("merges disjoint entries", () => { + const home = [{ name: "a" }, { name: "b" }]; + const cwd = [{ name: "c" }]; + const result = mergeCatalog(home, cwd); + expect(result.map((e) => e.name)).toEqual(["a", "b", "c"]); + }); + + it("cwd skill shadows home skill of the same name", () => { + const home = [{ name: "shared", summary: "home summary" }]; + const cwd = [{ name: "shared", summary: "cwd summary" }]; + const result = mergeCatalog(home, cwd); + expect(result).toHaveLength(1); + expect(result[0]?.summary).toBe("cwd summary"); + }); + + it("sorts by name", () => { + const home = [{ name: "z" }, { name: "a" }]; + const cwd = [{ name: "m" }]; + const result = mergeCatalog(home, cwd); + expect(result.map((e) => e.name)).toEqual(["a", "m", "z"]); + }); + + it("handles empty inputs", () => { + expect(mergeCatalog([], [])).toEqual([]); + }); }); describe("renderDescription", () => { - it("lists all skills by name and appends summaries only for valid ones", () => { - const catalog = [ - { name: "web-search", summary: "Use for web searches" }, - { name: "malformed-skill" }, - ]; - const result = renderDescription(catalog); - expect(result).toBe( - "Load a skill by name. Available skills:\n- web-search: Use for web searches\n- malformed-skill", - ); - }); - - it("returns a plain message when no skills are available", () => { - const result = renderDescription([]); - expect(result).toBe("Load a skill by name. No skills are currently available."); - }); - - it("lists skills with summaries and without", () => { - const catalog = [ - { name: "alpha", summary: "First skill" }, - { name: "beta" }, - { name: "gamma", summary: "Third skill" }, - ]; - const result = renderDescription(catalog); - expect(result).toContain("- alpha: First skill"); - expect(result).toContain("- beta"); - expect(result).toContain("- gamma: Third skill"); - }); + it("lists all skills by name and appends summaries only for valid ones", () => { + const catalog = [ + { name: "web-search", summary: "Use for web searches" }, + { name: "malformed-skill" }, + ]; + const result = renderDescription(catalog); + expect(result).toBe( + "Load a skill by name. Available skills:\n- web-search: Use for web searches\n- malformed-skill", + ); + }); + + it("returns a plain message when no skills are available", () => { + const result = renderDescription([]); + expect(result).toBe("Load a skill by name. No skills are currently available."); + }); + + it("lists skills with summaries and without", () => { + const catalog = [ + { name: "alpha", summary: "First skill" }, + { name: "beta" }, + { name: "gamma", summary: "Third skill" }, + ]; + const result = renderDescription(catalog); + expect(result).toContain("- alpha: First skill"); + expect(result).toContain("- beta"); + expect(result).toContain("- gamma: Third skill"); + }); }); describe("isValidSkillName", () => { - it("accepts a bare skill name", () => { - expect(isValidSkillName("web-search")).toBe(true); - }); - - it("rejects a name containing /", () => { - expect(isValidSkillName("../escape")).toBe(false); - }); - - it("rejects a name containing \\", () => { - expect(isValidSkillName("path\\name")).toBe(false); - }); - - it("rejects a name containing ..", () => { - expect(isValidSkillName("skill..evil")).toBe(false); - }); - - it("rejects an empty string", () => { - expect(isValidSkillName("")).toBe(false); - }); - - it("rejects a non-string value", () => { - expect(isValidSkillName(123)).toBe(false); - expect(isValidSkillName(null)).toBe(false); - expect(isValidSkillName(undefined)).toBe(false); - }); + it("accepts a bare skill name", () => { + expect(isValidSkillName("web-search")).toBe(true); + }); + + it("rejects a name containing /", () => { + expect(isValidSkillName("../escape")).toBe(false); + }); + + it("rejects a name containing \\", () => { + expect(isValidSkillName("path\\name")).toBe(false); + }); + + it("rejects a name containing ..", () => { + expect(isValidSkillName("skill..evil")).toBe(false); + }); + + it("rejects an empty string", () => { + expect(isValidSkillName("")).toBe(false); + }); + + it("rejects a non-string value", () => { + expect(isValidSkillName(123)).toBe(false); + expect(isValidSkillName(null)).toBe(false); + expect(isValidSkillName(undefined)).toBe(false); + }); }); describe("isPathWithinDir", () => { - it("accepts a path within the directory", () => { - expect(isPathWithinDir("/tmp/base/file.txt", "/tmp/base")).toBe(true); - }); + it("accepts a path within the directory", () => { + expect(isPathWithinDir("/tmp/base/file.txt", "/tmp/base")).toBe(true); + }); - it("accepts the directory itself", () => { - expect(isPathWithinDir("/tmp/base", "/tmp/base")).toBe(true); - }); + it("accepts the directory itself", () => { + expect(isPathWithinDir("/tmp/base", "/tmp/base")).toBe(true); + }); - it("rejects a path outside the directory", () => { - expect(isPathWithinDir("/tmp/other/file.txt", "/tmp/base")).toBe(false); - }); + it("rejects a path outside the directory", () => { + expect(isPathWithinDir("/tmp/other/file.txt", "/tmp/base")).toBe(false); + }); - it("rejects a prefix attack", () => { - expect(isPathWithinDir("/tmp/base-evil/file.txt", "/tmp/base")).toBe(false); - }); + it("rejects a prefix attack", () => { + expect(isPathWithinDir("/tmp/base-evil/file.txt", "/tmp/base")).toBe(false); + }); }); diff --git a/packages/skills/src/pure.ts b/packages/skills/src/pure.ts index 2800967..4cc8e5c 100644 --- a/packages/skills/src/pure.ts +++ b/packages/skills/src/pure.ts @@ -5,14 +5,14 @@ /** A discovered skill entry (name + optional summary from metadata). */ export interface SkillEntry { - readonly name: string; - readonly summary?: string | undefined; + readonly name: string; + readonly summary?: string | undefined; } /** Result of parsing a skill file's metadata. */ export interface SkillMeta { - readonly summary?: string | undefined; - readonly hasMeta: boolean; + readonly summary?: string | undefined; + readonly hasMeta: boolean; } /** @@ -22,16 +22,16 @@ export interface SkillMeta { * Returns `{ hasMeta: false }` when line 2 is not `---` (malformed). */ export function parseSkillMeta(content: string): SkillMeta { - const lines = content.split("\n"); - if (lines.length < 2) { - return { hasMeta: false }; - } - const line2 = lines[1]; - if (line2 === undefined || line2.trim() !== "---") { - return { hasMeta: false }; - } - const summary = lines[0]; - return { hasMeta: true, summary: summary?.trim() === "" ? undefined : summary }; + const lines = content.split("\n"); + if (lines.length < 2) { + return { hasMeta: false }; + } + const line2 = lines[1]; + if (line2 === undefined || line2.trim() !== "---") { + return { hasMeta: false }; + } + const summary = lines[0]; + return { hasMeta: true, summary: summary?.trim() === "" ? undefined : summary }; } /** @@ -40,11 +40,11 @@ export function parseSkillMeta(content: string): SkillMeta { * When hasMeta is false, returns the whole file unchanged. */ export function stripLoadedBody(content: string, hasMeta: boolean): string { - if (!hasMeta) { - return content; - } - const lines = content.split("\n"); - return lines.slice(2).join("\n"); + if (!hasMeta) { + return content; + } + const lines = content.split("\n"); + return lines.slice(2).join("\n"); } /** @@ -52,17 +52,17 @@ export function stripLoadedBody(content: string, hasMeta: boolean): string { * Returns a deduplicated array sorted by name. */ export function mergeCatalog( - homeEntries: readonly SkillEntry[], - cwdEntries: readonly SkillEntry[], + homeEntries: readonly SkillEntry[], + cwdEntries: readonly SkillEntry[], ): readonly SkillEntry[] { - const map = new Map<string, SkillEntry>(); - for (const entry of homeEntries) { - map.set(entry.name, entry); - } - for (const entry of cwdEntries) { - map.set(entry.name, entry); - } - return [...map.values()].sort((a, b) => a.name.localeCompare(b.name)); + const map = new Map<string, SkillEntry>(); + for (const entry of homeEntries) { + map.set(entry.name, entry); + } + for (const entry of cwdEntries) { + map.set(entry.name, entry); + } + return [...map.values()].sort((a, b) => a.name.localeCompare(b.name)); } /** @@ -70,18 +70,18 @@ export function mergeCatalog( * Lists all skills by name; appends summary only for skills with valid metadata. */ export function renderDescription(catalog: readonly SkillEntry[]): string { - if (catalog.length === 0) { - return "Load a skill by name. No skills are currently available."; - } - const lines = ["Load a skill by name. Available skills:"]; - for (const entry of catalog) { - if (entry.summary !== undefined) { - lines.push(`- ${entry.name}: ${entry.summary}`); - } else { - lines.push(`- ${entry.name}`); - } - } - return lines.join("\n"); + if (catalog.length === 0) { + return "Load a skill by name. No skills are currently available."; + } + const lines = ["Load a skill by name. Available skills:"]; + for (const entry of catalog) { + if (entry.summary !== undefined) { + lines.push(`- ${entry.name}: ${entry.summary}`); + } else { + lines.push(`- ${entry.name}`); + } + } + return lines.join("\n"); } /** @@ -89,16 +89,16 @@ export function renderDescription(catalog: readonly SkillEntry[]): string { * Returns true if the name is safe, false if it contains `/`, `\`, `..`, or is empty. */ export function isValidSkillName(name: unknown): name is string { - if (typeof name !== "string" || name.length === 0) { - return false; - } - if (name.includes("/") || name.includes("\\")) { - return false; - } - if (name.includes("..")) { - return false; - } - return true; + if (typeof name !== "string" || name.length === 0) { + return false; + } + if (name.includes("/") || name.includes("\\")) { + return false; + } + if (name.includes("..")) { + return false; + } + return true; } /** @@ -106,6 +106,6 @@ export function isValidSkillName(name: unknown): name is string { * Prefix check — catches `..` traversal and absolute paths outside base. */ export function isPathWithinDir(resolvedPath: string, base: string): boolean { - const normalizedBase = base.endsWith("/") ? base : `${base}/`; - return resolvedPath === base || resolvedPath.startsWith(normalizedBase); + const normalizedBase = base.endsWith("/") ? base : `${base}/`; + return resolvedPath === base || resolvedPath.startsWith(normalizedBase); } diff --git a/packages/skills/src/skills.test.ts b/packages/skills/src/skills.test.ts index fe0b437..57e5d63 100644 --- a/packages/skills/src/skills.test.ts +++ b/packages/skills/src/skills.test.ts @@ -8,261 +8,261 @@ import { createLoadSkillTool, scanSkillsDir } from "./load-skill.js"; import { makeSkillsToolFilter } from "./tools-filter.js"; function stubCtx(overrides?: Partial<ToolExecuteContext>): ToolExecuteContext { - return { - toolCallId: "test-call-1", - onOutput: () => {}, - signal: AbortSignal.timeout(5000), - log: createLogger( - { extensionId: "test" }, - { emit: () => {} }, - { now: () => 0, newId: () => "id" }, - ), - ...overrides, - }; + return { + toolCallId: "test-call-1", + onOutput: () => {}, + signal: AbortSignal.timeout(5000), + log: createLogger( + { extensionId: "test" }, + { emit: () => {} }, + { now: () => 0, newId: () => "id" }, + ), + ...overrides, + }; } let homeDir: string; let workdir: string; beforeEach(async () => { - homeDir = await mkdtemp(join(tmpdir(), "skills-home-test-")); - workdir = await mkdtemp(join(tmpdir(), "skills-workdir-test-")); + homeDir = await mkdtemp(join(tmpdir(), "skills-home-test-")); + workdir = await mkdtemp(join(tmpdir(), "skills-workdir-test-")); }); afterEach(async () => { - await rm(homeDir, { recursive: true, force: true }); - await rm(workdir, { recursive: true, force: true }); + await rm(homeDir, { recursive: true, force: true }); + await rm(workdir, { recursive: true, force: true }); }); describe("load_skill tool", () => { - it("loads a skill body (strips first two lines) from cwd .skills", async () => { - const skillsDir = join(workdir, ".skills"); - await mkdir(skillsDir); - await writeFile( - join(skillsDir, "web-search.md"), - "Use for web searches\n---\n# Web Search Skill\nDo a web search.", - "utf8", - ); - - const tool = createLoadSkillTool({ homeDir, workdir }); - const result = await tool.execute({ name: "web-search" }, stubCtx()); - - expect(result.isError).toBeUndefined(); - expect(result.content).toBe("# Web Search Skill\nDo a web search."); - }); - - it("falls back to home .skills when not in cwd", async () => { - const homeSkillsDir = join(homeDir, ".skills"); - await mkdir(homeSkillsDir); - await writeFile( - join(homeSkillsDir, "global-skill.md"), - "Global skill summary\n---\nGlobal body content", - "utf8", - ); - - const tool = createLoadSkillTool({ homeDir, workdir }); - const result = await tool.execute({ name: "global-skill" }, stubCtx()); - - expect(result.isError).toBeUndefined(); - expect(result.content).toBe("Global body content"); - }); - - it("returns isError for an unknown skill", async () => { - const tool = createLoadSkillTool({ homeDir, workdir }); - const result = await tool.execute({ name: "nonexistent" }, stubCtx()); - - expect(result.isError).toBe(true); - expect(result.content).toContain("unknown skill"); - }); - - it("rejects a name containing a path separator", async () => { - const tool = createLoadSkillTool({ homeDir, workdir }); - const result = await tool.execute({ name: "../escape" }, stubCtx()); - - expect(result.isError).toBe(true); - expect(result.content).toContain("Invalid skill name"); - }); - - it("rejects a name containing ..", async () => { - const tool = createLoadSkillTool({ homeDir, workdir }); - const result = await tool.execute({ name: "skill..evil" }, stubCtx()); - - expect(result.isError).toBe(true); - expect(result.content).toContain("Invalid skill name"); - }); - - it("rejects a name containing backslash", async () => { - const tool = createLoadSkillTool({ homeDir, workdir }); - const result = await tool.execute({ name: "path\\name" }, stubCtx()); - - expect(result.isError).toBe(true); - expect(result.content).toContain("Invalid skill name"); - }); - - it("returns the whole file when malformed (no --- on line 2)", async () => { - const skillsDir = join(workdir, ".skills"); - await mkdir(skillsDir); - await writeFile( - join(skillsDir, "malformed.md"), - "Just some content\nNo separator here\nMore content", - "utf8", - ); - - const tool = createLoadSkillTool({ homeDir, workdir }); - const result = await tool.execute({ name: "malformed" }, stubCtx()); - - expect(result.isError).toBeUndefined(); - expect(result.content).toBe("Just some content\nNo separator here\nMore content"); - }); - - it("cwd skill shadows home skill of the same name", async () => { - const homeSkillsDir = join(homeDir, ".skills"); - await mkdir(homeSkillsDir); - await writeFile(join(homeSkillsDir, "shared.md"), "Home summary\n---\nHome body", "utf8"); - - const cwdSkillsDir = join(workdir, ".skills"); - await mkdir(cwdSkillsDir); - await writeFile(join(cwdSkillsDir, "shared.md"), "Cwd summary\n---\nCwd body", "utf8"); - - const tool = createLoadSkillTool({ homeDir, workdir }); - const result = await tool.execute({ name: "shared" }, stubCtx()); - - expect(result.isError).toBeUndefined(); - expect(result.content).toBe("Cwd body"); - }); - - it("reads from ctx.cwd when set", async () => { - const ctxDir = await mkdtemp(join(tmpdir(), "skills-ctx-test-")); - try { - const ctxSkillsDir = join(ctxDir, ".skills"); - await mkdir(ctxSkillsDir); - await writeFile(join(ctxSkillsDir, "ctx-skill.md"), "Ctx summary\n---\nFrom ctx cwd", "utf8"); - - const tool = createLoadSkillTool({ homeDir, workdir }); - const result = await tool.execute({ name: "ctx-skill" }, stubCtx({ cwd: ctxDir })); - - expect(result.isError).toBeUndefined(); - expect(result.content).toBe("From ctx cwd"); - } finally { - await rm(ctxDir, { recursive: true, force: true }); - } - }); - - it("concurrencySafe is true", () => { - const tool = createLoadSkillTool({ homeDir, workdir }); - expect(tool.concurrencySafe).toBe(true); - }); + it("loads a skill body (strips first two lines) from cwd .skills", async () => { + const skillsDir = join(workdir, ".skills"); + await mkdir(skillsDir); + await writeFile( + join(skillsDir, "web-search.md"), + "Use for web searches\n---\n# Web Search Skill\nDo a web search.", + "utf8", + ); + + const tool = createLoadSkillTool({ homeDir, workdir }); + const result = await tool.execute({ name: "web-search" }, stubCtx()); + + expect(result.isError).toBeUndefined(); + expect(result.content).toBe("# Web Search Skill\nDo a web search."); + }); + + it("falls back to home .skills when not in cwd", async () => { + const homeSkillsDir = join(homeDir, ".skills"); + await mkdir(homeSkillsDir); + await writeFile( + join(homeSkillsDir, "global-skill.md"), + "Global skill summary\n---\nGlobal body content", + "utf8", + ); + + const tool = createLoadSkillTool({ homeDir, workdir }); + const result = await tool.execute({ name: "global-skill" }, stubCtx()); + + expect(result.isError).toBeUndefined(); + expect(result.content).toBe("Global body content"); + }); + + it("returns isError for an unknown skill", async () => { + const tool = createLoadSkillTool({ homeDir, workdir }); + const result = await tool.execute({ name: "nonexistent" }, stubCtx()); + + expect(result.isError).toBe(true); + expect(result.content).toContain("unknown skill"); + }); + + it("rejects a name containing a path separator", async () => { + const tool = createLoadSkillTool({ homeDir, workdir }); + const result = await tool.execute({ name: "../escape" }, stubCtx()); + + expect(result.isError).toBe(true); + expect(result.content).toContain("Invalid skill name"); + }); + + it("rejects a name containing ..", async () => { + const tool = createLoadSkillTool({ homeDir, workdir }); + const result = await tool.execute({ name: "skill..evil" }, stubCtx()); + + expect(result.isError).toBe(true); + expect(result.content).toContain("Invalid skill name"); + }); + + it("rejects a name containing backslash", async () => { + const tool = createLoadSkillTool({ homeDir, workdir }); + const result = await tool.execute({ name: "path\\name" }, stubCtx()); + + expect(result.isError).toBe(true); + expect(result.content).toContain("Invalid skill name"); + }); + + it("returns the whole file when malformed (no --- on line 2)", async () => { + const skillsDir = join(workdir, ".skills"); + await mkdir(skillsDir); + await writeFile( + join(skillsDir, "malformed.md"), + "Just some content\nNo separator here\nMore content", + "utf8", + ); + + const tool = createLoadSkillTool({ homeDir, workdir }); + const result = await tool.execute({ name: "malformed" }, stubCtx()); + + expect(result.isError).toBeUndefined(); + expect(result.content).toBe("Just some content\nNo separator here\nMore content"); + }); + + it("cwd skill shadows home skill of the same name", async () => { + const homeSkillsDir = join(homeDir, ".skills"); + await mkdir(homeSkillsDir); + await writeFile(join(homeSkillsDir, "shared.md"), "Home summary\n---\nHome body", "utf8"); + + const cwdSkillsDir = join(workdir, ".skills"); + await mkdir(cwdSkillsDir); + await writeFile(join(cwdSkillsDir, "shared.md"), "Cwd summary\n---\nCwd body", "utf8"); + + const tool = createLoadSkillTool({ homeDir, workdir }); + const result = await tool.execute({ name: "shared" }, stubCtx()); + + expect(result.isError).toBeUndefined(); + expect(result.content).toBe("Cwd body"); + }); + + it("reads from ctx.cwd when set", async () => { + const ctxDir = await mkdtemp(join(tmpdir(), "skills-ctx-test-")); + try { + const ctxSkillsDir = join(ctxDir, ".skills"); + await mkdir(ctxSkillsDir); + await writeFile(join(ctxSkillsDir, "ctx-skill.md"), "Ctx summary\n---\nFrom ctx cwd", "utf8"); + + const tool = createLoadSkillTool({ homeDir, workdir }); + const result = await tool.execute({ name: "ctx-skill" }, stubCtx({ cwd: ctxDir })); + + expect(result.isError).toBeUndefined(); + expect(result.content).toBe("From ctx cwd"); + } finally { + await rm(ctxDir, { recursive: true, force: true }); + } + }); + + it("concurrencySafe is true", () => { + const tool = createLoadSkillTool({ homeDir, workdir }); + expect(tool.concurrencySafe).toBe(true); + }); }); describe("scanSkillsDir", () => { - it("scans .md files and parses metadata", async () => { - const skillsDir = join(workdir, ".skills"); - await mkdir(skillsDir); - await writeFile(join(skillsDir, "valid.md"), "Summary\n---\nBody", "utf8"); - await writeFile(join(skillsDir, "malformed.md"), "No separator\nBody", "utf8"); - await writeFile(join(skillsDir, "other.txt"), "Not a skill", "utf8"); - - const result = await scanSkillsDir(skillsDir); - - expect(result).toHaveLength(2); - const valid = result.find((e) => e.name === "valid"); - expect(valid?.summary).toBe("Summary"); - const malformed = result.find((e) => e.name === "malformed"); - expect(malformed?.summary).toBeUndefined(); - }); - - it("returns empty array for nonexistent directory", async () => { - const result = await scanSkillsDir(join(workdir, "nonexistent")); - expect(result).toEqual([]); - }); + it("scans .md files and parses metadata", async () => { + const skillsDir = join(workdir, ".skills"); + await mkdir(skillsDir); + await writeFile(join(skillsDir, "valid.md"), "Summary\n---\nBody", "utf8"); + await writeFile(join(skillsDir, "malformed.md"), "No separator\nBody", "utf8"); + await writeFile(join(skillsDir, "other.txt"), "Not a skill", "utf8"); + + const result = await scanSkillsDir(skillsDir); + + expect(result).toHaveLength(2); + const valid = result.find((e) => e.name === "valid"); + expect(valid?.summary).toBe("Summary"); + const malformed = result.find((e) => e.name === "malformed"); + expect(malformed?.summary).toBeUndefined(); + }); + + it("returns empty array for nonexistent directory", async () => { + const result = await scanSkillsDir(join(workdir, "nonexistent")); + expect(result).toEqual([]); + }); }); describe("tools filter", () => { - it("rewrites load_skill description with the current catalog (cwd-aware)", async () => { - const homeSkillsDir = join(homeDir, ".skills"); - await mkdir(homeSkillsDir); - await writeFile(join(homeSkillsDir, "global.md"), "Global summary\n---\nBody", "utf8"); - - const cwdSkillsDir = join(workdir, ".skills"); - await mkdir(cwdSkillsDir); - await writeFile(join(cwdSkillsDir, "local.md"), "Local summary\n---\nBody", "utf8"); - - const filter = makeSkillsToolFilter({ homeDir, workdir }); - - const tool = createLoadSkillTool({ homeDir, workdir }); - const asm: ToolAssembly = { - tools: [tool], - cwd: workdir, - conversationId: "test-conv", - }; - - const result = await filter(asm); - const loadSkill = result.tools.find( - (t: import("@dispatch/kernel").ToolContract) => t.name === "load_skill", - ); - expect(loadSkill).toBeDefined(); - expect(loadSkill?.description).toContain("global"); - expect(loadSkill?.description).toContain("Global summary"); - expect(loadSkill?.description).toContain("local"); - expect(loadSkill?.description).toContain("Local summary"); - }); - - it("updates the name parameter enum with available skills", async () => { - const cwdSkillsDir = join(workdir, ".skills"); - await mkdir(cwdSkillsDir); - await writeFile(join(cwdSkillsDir, "alpha.md"), "Alpha\n---\nBody", "utf8"); - await writeFile(join(cwdSkillsDir, "beta.md"), "Beta\n---\nBody", "utf8"); - - const filter = makeSkillsToolFilter({ homeDir, workdir }); - - const tool = createLoadSkillTool({ homeDir, workdir }); - const asm: ToolAssembly = { - tools: [tool], - cwd: workdir, - conversationId: "test-conv", - }; - - const result = await filter(asm); - const loadSkill = result.tools.find( - (t: import("@dispatch/kernel").ToolContract) => t.name === "load_skill", - ); - expect(loadSkill?.parameters.properties?.name?.enum).toEqual(["alpha", "beta"]); - }); - - it("handles empty skill directories gracefully", async () => { - const filter = makeSkillsToolFilter({ homeDir, workdir }); - - const tool = createLoadSkillTool({ homeDir, workdir }); - const asm: ToolAssembly = { - tools: [tool], - cwd: workdir, - conversationId: "test-conv", - }; - - const result = await filter(asm); - const loadSkill = result.tools.find( - (t: import("@dispatch/kernel").ToolContract) => t.name === "load_skill", - ); - expect(loadSkill?.description).toContain("No skills are currently available"); - }); - - it("passes through non-load_skill tools unchanged", async () => { - const filter = makeSkillsToolFilter({ homeDir, workdir }); - - const otherTool = { - name: "other_tool", - description: "Some other tool", - parameters: { type: "object" as const }, - execute: async () => ({ content: "ok" }), - }; - const asm: ToolAssembly = { - tools: [otherTool], - cwd: workdir, - conversationId: "test-conv", - }; - - const result = await filter(asm); - expect(result.tools[0]?.name).toBe("other_tool"); - expect(result.tools[0]?.description).toBe("Some other tool"); - }); + it("rewrites load_skill description with the current catalog (cwd-aware)", async () => { + const homeSkillsDir = join(homeDir, ".skills"); + await mkdir(homeSkillsDir); + await writeFile(join(homeSkillsDir, "global.md"), "Global summary\n---\nBody", "utf8"); + + const cwdSkillsDir = join(workdir, ".skills"); + await mkdir(cwdSkillsDir); + await writeFile(join(cwdSkillsDir, "local.md"), "Local summary\n---\nBody", "utf8"); + + const filter = makeSkillsToolFilter({ homeDir, workdir }); + + const tool = createLoadSkillTool({ homeDir, workdir }); + const asm: ToolAssembly = { + tools: [tool], + cwd: workdir, + conversationId: "test-conv", + }; + + const result = await filter(asm); + const loadSkill = result.tools.find( + (t: import("@dispatch/kernel").ToolContract) => t.name === "load_skill", + ); + expect(loadSkill).toBeDefined(); + expect(loadSkill?.description).toContain("global"); + expect(loadSkill?.description).toContain("Global summary"); + expect(loadSkill?.description).toContain("local"); + expect(loadSkill?.description).toContain("Local summary"); + }); + + it("updates the name parameter enum with available skills", async () => { + const cwdSkillsDir = join(workdir, ".skills"); + await mkdir(cwdSkillsDir); + await writeFile(join(cwdSkillsDir, "alpha.md"), "Alpha\n---\nBody", "utf8"); + await writeFile(join(cwdSkillsDir, "beta.md"), "Beta\n---\nBody", "utf8"); + + const filter = makeSkillsToolFilter({ homeDir, workdir }); + + const tool = createLoadSkillTool({ homeDir, workdir }); + const asm: ToolAssembly = { + tools: [tool], + cwd: workdir, + conversationId: "test-conv", + }; + + const result = await filter(asm); + const loadSkill = result.tools.find( + (t: import("@dispatch/kernel").ToolContract) => t.name === "load_skill", + ); + expect(loadSkill?.parameters.properties?.name?.enum).toEqual(["alpha", "beta"]); + }); + + it("handles empty skill directories gracefully", async () => { + const filter = makeSkillsToolFilter({ homeDir, workdir }); + + const tool = createLoadSkillTool({ homeDir, workdir }); + const asm: ToolAssembly = { + tools: [tool], + cwd: workdir, + conversationId: "test-conv", + }; + + const result = await filter(asm); + const loadSkill = result.tools.find( + (t: import("@dispatch/kernel").ToolContract) => t.name === "load_skill", + ); + expect(loadSkill?.description).toContain("No skills are currently available"); + }); + + it("passes through non-load_skill tools unchanged", async () => { + const filter = makeSkillsToolFilter({ homeDir, workdir }); + + const otherTool = { + name: "other_tool", + description: "Some other tool", + parameters: { type: "object" as const }, + execute: async () => ({ content: "ok" }), + }; + const asm: ToolAssembly = { + tools: [otherTool], + cwd: workdir, + conversationId: "test-conv", + }; + + const result = await filter(asm); + expect(result.tools[0]?.name).toBe("other_tool"); + expect(result.tools[0]?.description).toBe("Some other tool"); + }); }); diff --git a/packages/skills/src/tools-filter.ts b/packages/skills/src/tools-filter.ts index 058f971..3f25e95 100644 --- a/packages/skills/src/tools-filter.ts +++ b/packages/skills/src/tools-filter.ts @@ -9,55 +9,55 @@ import { mergeCatalog, renderDescription } from "./pure.js"; * and name parameter enum with the current skill catalog. */ export function makeSkillsToolFilter(deps: SkillsDeps) { - const { homeDir, workdir } = deps; - - return async (asm: ToolAssembly): Promise<ToolAssembly> => { - const effectiveBase = asm.cwd ? resolve(asm.cwd) : resolve(workdir); - const cwdSkillsDir = join(effectiveBase, ".skills"); - const homeSkillsDir = join(resolve(homeDir), ".skills"); - - let homeEntries: readonly import("./pure.js").SkillEntry[]; - let cwdEntries: readonly import("./pure.js").SkillEntry[]; - try { - [homeEntries, cwdEntries] = await Promise.all([ - scanSkillsDir(homeSkillsDir), - scanSkillsDir(cwdSkillsDir), - ]); - } catch { - return asm; - } - - const catalog = mergeCatalog(homeEntries, cwdEntries); - const names = catalog.map((e) => e.name); - const description = renderDescription(catalog); - - return { - ...asm, - tools: asm.tools.map((t: ToolContract) => { - if (t.name !== "load_skill") return t; - - const nameProp = t.parameters.properties?.name; - if (!nameProp) { - return { ...t, description }; - } - - const updatedNameProp: JsonSchemaProperty = - names.length > 0 ? { ...nameProp, enum: names } : { ...nameProp }; - - const updatedProperties: Record<string, JsonSchemaProperty> = { - ...t.parameters.properties, - name: updatedNameProp, - }; - - return { - ...t, - description, - parameters: { - ...t.parameters, - properties: updatedProperties, - }, - }; - }), - }; - }; + const { homeDir, workdir } = deps; + + return async (asm: ToolAssembly): Promise<ToolAssembly> => { + const effectiveBase = asm.cwd ? resolve(asm.cwd) : resolve(workdir); + const cwdSkillsDir = join(effectiveBase, ".skills"); + const homeSkillsDir = join(resolve(homeDir), ".skills"); + + let homeEntries: readonly import("./pure.js").SkillEntry[]; + let cwdEntries: readonly import("./pure.js").SkillEntry[]; + try { + [homeEntries, cwdEntries] = await Promise.all([ + scanSkillsDir(homeSkillsDir), + scanSkillsDir(cwdSkillsDir), + ]); + } catch { + return asm; + } + + const catalog = mergeCatalog(homeEntries, cwdEntries); + const names = catalog.map((e) => e.name); + const description = renderDescription(catalog); + + return { + ...asm, + tools: asm.tools.map((t: ToolContract) => { + if (t.name !== "load_skill") return t; + + const nameProp = t.parameters.properties?.name; + if (!nameProp) { + return { ...t, description }; + } + + const updatedNameProp: JsonSchemaProperty = + names.length > 0 ? { ...nameProp, enum: names } : { ...nameProp }; + + const updatedProperties: Record<string, JsonSchemaProperty> = { + ...t.parameters.properties, + name: updatedNameProp, + }; + + return { + ...t, + description, + parameters: { + ...t.parameters, + properties: updatedProperties, + }, + }; + }), + }; + }; } |
