From de0b5ff1ae679c0d633ddc8a79947fbd04834bab Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Fri, 21 Aug 2026 12:47:37 +0200 Subject: [PATCH 1/5] =?UTF-8?q?feat(appkit):=20agent=20skills=20v1=20?= =?UTF-8?q?=E2=80=94=20SKILL.md=20progressive=20disclosure=20(1/2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SDK half of #532 (split 1/2). Skills engine (parse/load/resolve/render/read), agent-definition skills: wiring, agents-plugin integration (load_skill + read_skill_file tools, catalog resolution, clientConfig), the appkit-ui useAgentChat /skill surface, and docs. Playground/template fixtures follow in 2/2. Signed-off-by: MarioCadenas --- .../api/appkit/Interface.AgentDefinition.md | 13 + .../appkit/Interface.AgentsPluginConfig.md | 42 ++ .../api/appkit/Interface.RegisteredAgent.md | 12 + .../api/appkit/TypeAlias.ResolvedToolEntry.md | 44 +++ docs/docs/plugins/agents.md | 67 +++- .../hooks/__tests__/use-agent-chat.test.ts | 45 +++ .../src/react/hooks/use-agent-chat.ts | 26 +- packages/appkit/src/core/agent/frontmatter.ts | 18 + packages/appkit/src/core/agent/load-agents.ts | 60 ++- .../appkit/src/core/agent/skills/index.ts | 6 + .../src/core/agent/skills/load-skills.ts | 94 +++++ .../src/core/agent/skills/parse-skill.ts | 130 +++++++ .../src/core/agent/skills/read-resource.ts | 52 +++ .../appkit/src/core/agent/skills/render.ts | 42 ++ .../src/core/agent/skills/resolve-catalog.ts | 138 +++++++ .../core/agent/skills/tests/skills.test.ts | 330 ++++++++++++++++ .../appkit/src/core/agent/skills/types.ts | 49 +++ .../src/core/agent/tests/load-agents.test.ts | 9 + packages/appkit/src/core/agent/types.ts | 48 +++ packages/appkit/src/plugins/agents/agents.ts | 364 +++++++++++++++++- .../appkit/src/plugins/agents/manifest.json | 18 + packages/appkit/src/plugins/agents/schemas.ts | 6 + .../agents/tests/dispatch-tool-call.test.ts | 157 +++++++- .../plugins/agents/tests/skill-client.test.ts | 148 +++++++ .../plugins/agents/tests/skill-volume.test.ts | 154 ++++++++ 25 files changed, 2056 insertions(+), 16 deletions(-) create mode 100644 packages/appkit/src/core/agent/frontmatter.ts create mode 100644 packages/appkit/src/core/agent/skills/index.ts create mode 100644 packages/appkit/src/core/agent/skills/load-skills.ts create mode 100644 packages/appkit/src/core/agent/skills/parse-skill.ts create mode 100644 packages/appkit/src/core/agent/skills/read-resource.ts create mode 100644 packages/appkit/src/core/agent/skills/render.ts create mode 100644 packages/appkit/src/core/agent/skills/resolve-catalog.ts create mode 100644 packages/appkit/src/core/agent/skills/tests/skills.test.ts create mode 100644 packages/appkit/src/core/agent/skills/types.ts create mode 100644 packages/appkit/src/plugins/agents/tests/skill-client.test.ts create mode 100644 packages/appkit/src/plugins/agents/tests/skill-volume.test.ts diff --git a/docs/docs/api/appkit/Interface.AgentDefinition.md b/docs/docs/api/appkit/Interface.AgentDefinition.md index 2c9b5bb27..3010adf52 100644 --- a/docs/docs/api/appkit/Interface.AgentDefinition.md +++ b/docs/docs/api/appkit/Interface.AgentDefinition.md @@ -131,6 +131,19 @@ entirely. *** +### skills? + +```ts +optional skills: string[]; +``` + +Names of global skills (shared `skills/` pool or catalog volume) to make +visible to this agent. Per-agent skills under `/skills/` are always +visible and need not be listed. Ignored when the plugin's +`autoInheritSkills` makes every global skill visible. + +*** + ### tools? ```ts diff --git a/docs/docs/api/appkit/Interface.AgentsPluginConfig.md b/docs/docs/api/appkit/Interface.AgentsPluginConfig.md index 59d7d1220..a2f2c963f 100644 --- a/docs/docs/api/appkit/Interface.AgentsPluginConfig.md +++ b/docs/docs/api/appkit/Interface.AgentsPluginConfig.md @@ -69,6 +69,22 @@ Milliseconds to wait before auto-denying. Default: 60_000. *** +### autoInheritSkills? + +```ts +optional autoInheritSkills: + | boolean + | AutoInheritToolsConfig; +``` + +Whether every global skill (shared `skills/` pool or catalog volume) is +visible to an agent without listing it in `skills:` frontmatter. Off by +default so each agent's always-on skill catalog stays lean; accepts a +boolean shorthand or a per-origin `{ file, code }` config, mirroring +[autoInheritTools](#autoinherittools). + +*** + ### autoInheritTools? ```ts @@ -218,6 +234,32 @@ optional name: string; *** +### skillCredentialMode? + +```ts +optional skillCredentialMode: "sp" | "obo"; +``` + +Identity used to read catalog (volume) skills. v1 supports `"sp"` (default — +a shared, service-principal-readable curated pool). `"obo"` is the reserved +switch point for per-user skill volumes and is not wired yet (falls back to +`"sp"` with a warning). + +*** + +### skillsVolume? + +```ts +optional skillsVolume: string; +``` + +Unity Catalog Volume path for catalog-sourced skills (e.g. +`/Volumes///`). Falls back to the +`DATABRICKS_VOLUME_AGENT_SKILLS` env var. Skills at `//SKILL.md` +are discovered at boot and on `reload()` and read as the service principal. + +*** + ### telemetry? ```ts diff --git a/docs/docs/api/appkit/Interface.RegisteredAgent.md b/docs/docs/api/appkit/Interface.RegisteredAgent.md index 4cbe04682..78a70f9a1 100644 --- a/docs/docs/api/appkit/Interface.RegisteredAgent.md +++ b/docs/docs/api/appkit/Interface.RegisteredAgent.md @@ -70,6 +70,18 @@ name: string; *** +### skills? + +```ts +optional skills: ResolvedSkillCatalog; +``` + +Resolved per-agent skill catalog (visibility + collision rules applied). +Present when any skill is visible to this agent; drives the always-on +prompt catalog and `load_skill` dispatch. + +*** + ### toolIndex ```ts diff --git a/docs/docs/api/appkit/TypeAlias.ResolvedToolEntry.md b/docs/docs/api/appkit/TypeAlias.ResolvedToolEntry.md index d03c0afda..08571cff5 100644 --- a/docs/docs/api/appkit/TypeAlias.ResolvedToolEntry.md +++ b/docs/docs/api/appkit/TypeAlias.ResolvedToolEntry.md @@ -27,6 +27,12 @@ type ResolvedToolEntry = def: AgentToolDefinition; source: "hosted-supervisor"; spec: SupervisorTool; +} + | { + builtin: "load_skill" | "read_skill_file"; + catalog: ResolvedSkillCatalog; + def: AgentToolDefinition; + source: "skill"; }; ``` @@ -179,3 +185,41 @@ is intentionally NOT included in the `tools` array passed to ```ts spec: SupervisorTool; ``` + +```ts +{ + builtin: "load_skill" | "read_skill_file"; + catalog: ResolvedSkillCatalog; + def: AgentToolDefinition; + source: "skill"; +} +``` + +### builtin + +```ts +builtin: "load_skill" | "read_skill_file"; +``` + +### catalog + +```ts +catalog: ResolvedSkillCatalog; +``` + +### def + +```ts +def: AgentToolDefinition; +``` + +### source + +```ts +source: "skill"; +``` + +Built-in skill tools (`load_skill`, `read_skill_file`) injected into +any agent that has a visible skill catalog. Executed in-process by the +agents plugin against the agent's resolved catalog; read-only, so they +bypass the approval gate. diff --git a/docs/docs/plugins/agents.md b/docs/docs/plugins/agents.md index 11bb32505..19b0f6da2 100644 --- a/docs/docs/plugins/agents.md +++ b/docs/docs/plugins/agents.md @@ -37,7 +37,7 @@ That alone gives you a live HTTP server with `POST /invocations` (and its alias ## Level 1: drop a markdown agent package -Each agent lives in its own folder under `server/agents/` with entry file `agent.md`. A folder is an agent only if it holds an entry file (`agent.md` or `agent.ts`); a folder without one is skipped, so per-agent asset folders like `skills/` sit beside the entry. +Each agent lives in its own folder under `server/agents/` with entry file `agent.md`. A folder is an agent only if it holds an entry file (`agent.md` or `agent.ts`); a folder without one is skipped, so per-agent asset folders sit beside the entry — notably a `skills/` folder holding [Skills](#skills) (on-demand instruction packs the agent loads by name). A shared `server/agents/skills/` folder holds skills available to any agent. ``` my-app/ @@ -206,6 +206,67 @@ await createApp({ Put `supervisor`, `researcher`, and `writer` in their own `server/agents//agent.ts` folders (default export each) — a markdown parent can also delegate to a code child in a sibling folder via `agents: [helper]` frontmatter. Each key in `agents: {...}` on an `AgentDefinition` becomes an `agent-` tool on the parent. When invoked, the agents plugin runs the child's adapter with a fresh message list (no shared thread state) and returns the aggregated text. Cycles in a code agent's inline `agents: {}` graph are rejected at load (`createAgent`); markdown `agents:` delegation rejects self-references at load and bounds deeper cycles at runtime via `limits.maxSubAgentDepth`. +## Skills + +Skills are on-demand instruction packs — the same `SKILL.md` format Claude Code and Cursor use. Only each skill's `name` + `description` sit in the system prompt (always-on, cheap); the full body loads on demand when the agent (or the user) invokes it. This works on any Databricks-served model — AppKit implements the disclosure itself, so it doesn't depend on a provider-native skills feature. + +A skill is a directory with a `SKILL.md` plus any bundled reference files: + +``` +config/agents/ + skills/ # shared pool — any agent can opt in + pdf-forms/ + SKILL.md + reference.md + planner/ + agent.md + skills/ # private to the `planner` agent + house-style/ + SKILL.md +``` + +```md +--- +name: pdf-forms +description: Fill and validate PDF form fields from a data record. +--- + +To fill a PDF form: + +1. Read `reference.md` for the field-name conventions. +2. ... +``` + +`name` and `description` are required; `license`, `allowed-tools`, and `metadata` are accepted for compatibility with skills authored elsewhere. Unknown keys warn and are ignored. + +### Visibility + +- **Per-agent skills** (`config/agents//skills/`) are always visible to that agent. +- **Global skills** (`config/agents/skills/`, and catalog-volume skills) are **opt-in**: list them in the agent's frontmatter, `skills: [pdf-forms]`. Set `autoInheritSkills: true` (or `{ file, code }`) on the plugin to make every global skill visible without listing — off by default so each agent's always-on catalog stays lean. + +### How the agent uses a skill + +Two read-only built-in tools are injected into any agent that has a visible catalog: + +- `load_skill(skill)` — returns the skill's full instructions plus a manifest of its bundled files. +- `read_skill_file(skill, path)` — returns the contents of one of those bundled files. + +The model calls `load_skill` on its own when a task matches a skill's description. A **user** can force a specific skill for a turn with the `/skill-name` prefix in chat (or the `send(message, { skill })` option on `useAgentChat`); the skill's instructions are injected into that turn deterministically, and `load_skill` remains available for auto-selection. The client reads the per-agent catalog from the plugin's `clientConfig()` payload to power a picker. + +### Catalog skills (Unity Catalog Volume) + +Point `skillsVolume` (or the `DATABRICKS_VOLUME_AGENT_SKILLS` env var) at a UC Volume laid out the same way — `//SKILL.md`. Catalog skills are discovered at boot and on `reload()`, merged into the shared global pool, and read as the **service principal** (`skillCredentialMode` defaults to `"sp"`). They're intended as a shared, curated pool; per-user (OBO) skill volumes are not wired yet. Declaring the optional `volume` resource in the manifest lets the scaffolder grant the SP read access. + +### Name collisions + +Skill names are addressed bare. If two sources provide the same name, each becomes a qualified `:name` (`agent:`, `bundle:`, `volume:`) and the bare name is rejected as ambiguous with the alternatives listed. Two skills with the same name from the *same* source is a boot-time error. + +### v1 caveats + +- **Scripts are not executed.** A skill may reference `scripts/foo.py`; v1 loads prose and reference docs only. +- **`allowed-tools` is advisory.** It's surfaced as a hint in the loaded skill, not enforced — loading a skill does not restrict the agent's callable tools. It is not a sandbox. +- **Skill bodies are not per-user access-controlled** (they read as the SP). Keep user-sensitive content out of skill bodies. + ## Level 5: standalone (no `createApp`) ```ts @@ -414,6 +475,9 @@ agents({ defaultModel?: AgentAdapter | Promise | string, tools?: Record, autoInheritTools?: boolean | { file?: boolean, code?: boolean }, + autoInheritSkills?: boolean | { file?: boolean, code?: boolean }, // default off + skillsVolume?: string, // UC Volume for catalog skills; falls back to DATABRICKS_VOLUME_AGENT_SKILLS + skillCredentialMode?: "sp" | "obo", // default "sp" (see Skills) threadStore?: ThreadStore, // default in-memory baseSystemPrompt?: false | string | (ctx: PromptContext) => string, mcp?: { @@ -603,6 +667,7 @@ appkit.agents.getThreads(userId); // list user's threads | `endpoint` | string | Model serving endpoint name. Shortcut for `model`. | | `model` | string | Same as `endpoint`; either works. | | `tools` | array | Unified tool list. Entries are `plugin:` / `plugin:: [t1, t2]` / `plugin:: { only, except, rename, prefix }` for plugin tools, or a bare `` resolved against `agents({ tools: {...} })` for ambient tools. See "Level 2: scope tools in frontmatter" above for examples. | +| `skills` | array | Names of global skills (shared `skills/` pool or catalog volume) to make visible to this agent. Per-agent skills under `/skills/` are always visible. See [Skills](#skills). | | `default` | boolean | First agent id (sorted order) with `default: true` becomes the default agent. | | `agents` | array | Sub-agent ids (sibling folders) to delegate to; each becomes an `agent-` tool. Resolves against other markdown and code agents. | | `maxSteps` | number | Adapter max-step hint. | diff --git a/packages/appkit-ui/src/react/hooks/__tests__/use-agent-chat.test.ts b/packages/appkit-ui/src/react/hooks/__tests__/use-agent-chat.test.ts index c066d35f8..4e6646913 100644 --- a/packages/appkit-ui/src/react/hooks/__tests__/use-agent-chat.test.ts +++ b/packages/appkit-ui/src/react/hooks/__tests__/use-agent-chat.test.ts @@ -77,6 +77,51 @@ describe("useAgentChat", () => { expect(capturedCallbacks.maxRetries).toBe(0); }); + test("send(message, { skill }) includes the skill in the payload", async () => { + const { result } = renderHook(() => useAgentChat({ agent: "helper" })); + + act(() => { + void result.current.send("summarize", { skill: "pdf" }); + }); + + await waitFor(() => expect(mockConnectSSE).toHaveBeenCalled()); + expect(capturedCallbacks.payload).toEqual({ + message: "summarize", + agent: "helper", + skill: "pdf", + }); + }); + + test("parses a leading /skill-name token off the message", async () => { + const { result } = renderHook(() => useAgentChat({ agent: "helper" })); + + act(() => { + void result.current.send("/pdf extract the tables"); + }); + + await waitFor(() => expect(mockConnectSSE).toHaveBeenCalled()); + expect(capturedCallbacks.payload).toEqual({ + message: "extract the tables", + agent: "helper", + skill: "pdf", + }); + }); + + test("/skill-name with no text falls back to a non-empty message", async () => { + const { result } = renderHook(() => useAgentChat({ agent: "helper" })); + + act(() => { + void result.current.send("/pdf"); + }); + + await waitFor(() => expect(mockConnectSSE).toHaveBeenCalled()); + expect(capturedCallbacks.payload).toEqual({ + message: "Use the pdf skill.", + agent: "helper", + skill: "pdf", + }); + }); + test("custom endpoint is forwarded to connectSSE", async () => { const { result } = renderHook(() => useAgentChat({ agent: "helper", endpoint: "/v2/chat" }), diff --git a/packages/appkit-ui/src/react/hooks/use-agent-chat.ts b/packages/appkit-ui/src/react/hooks/use-agent-chat.ts index c256adf07..7107cb362 100644 --- a/packages/appkit-ui/src/react/hooks/use-agent-chat.ts +++ b/packages/appkit-ui/src/react/hooks/use-agent-chat.ts @@ -85,8 +85,13 @@ export interface UseAgentChatResult { /** * Send a user turn and stream the response. Aborts any in-flight * stream. Resolves when the stream completes (success or error). + * + * Pass `opts.skill` to force-load a skill for this turn, or prefix the + * message with `/skill-name` as sugar (the leading token is parsed off and + * sent as the skill; the rest becomes the message). An explicit + * `opts.skill` wins over a `/`-prefix. */ - send: (message: string) => Promise; + send: (message: string, opts?: { skill?: string }) => Promise; /** * Discard accumulated content, events, and threadId. Aborts any * in-flight stream. Use when switching agents or starting a fresh @@ -158,7 +163,7 @@ export function useAgentChat({ }, []); const send = useCallback( - async (message: string) => { + async (message: string, opts?: { skill?: string }) => { // Abort any previous stream — only one chat turn in flight per hook. abortControllerRef.current?.abort(); const controller = new AbortController(); @@ -170,9 +175,24 @@ export function useAgentChat({ setError(null); setIsStreaming(true); + // Resolve the forced skill: explicit opts.skill wins; otherwise parse a + // leading `/skill-name` token off the message. When the message is only + // the token, fall back to a minimal instruction so the turn isn't empty. + let text = message; + let skill = opts?.skill; + if (!skill) { + const match = text.match(/^\/([A-Za-z0-9][\w.:-]*)\s*/); + if (match) { + skill = match[1]; + text = text.slice(match[0].length); + if (text.trim() === "") text = `Use the ${skill} skill.`; + } + } + const payload = { - message, + message: text, agent, + ...(skill ? { skill } : {}), ...(threadIdRef.current ? { threadId: threadIdRef.current } : {}), }; diff --git a/packages/appkit/src/core/agent/frontmatter.ts b/packages/appkit/src/core/agent/frontmatter.ts new file mode 100644 index 000000000..57fba1b7d --- /dev/null +++ b/packages/appkit/src/core/agent/frontmatter.ts @@ -0,0 +1,18 @@ +const FRONTMATTER_RE = /^---\r?\n([\s\S]*?)\r?\n---\r?\n?([\s\S]*)$/; + +/** + * Splits a `--- yaml ---\nbody` markdown string into its raw YAML block and + * trimmed body. Returns `yaml: null` when there is no leading frontmatter + * fence. Shared by the agent loader ({@link parseFrontmatter}) and the skill + * parser so the fence regex lives in one place. + */ +export function splitFrontmatter(raw: string): { + yaml: string | null; + body: string; +} { + const match = raw.match(FRONTMATTER_RE); + if (!match) { + return { yaml: null, body: raw.trim() }; + } + return { yaml: match[1], body: match[2].trim() }; +} diff --git a/packages/appkit/src/core/agent/load-agents.ts b/packages/appkit/src/core/agent/load-agents.ts index 54c77f157..2952ccaae 100644 --- a/packages/appkit/src/core/agent/load-agents.ts +++ b/packages/appkit/src/core/agent/load-agents.ts @@ -16,6 +16,7 @@ import type { import { isToolkitEntry } from "../../core/agent/types"; import { createLogger } from "../../logging/logger"; import { agentDirNames } from "./agent-dirs"; +import { splitFrontmatter } from "./frontmatter"; const logger = createLogger("agents:loader"); @@ -77,6 +78,13 @@ interface Frontmatter { * rejects non-empty values since there are no siblings to resolve against. */ agents?: string[]; + /** + * Names of global skills (from the shared `skills/` pool or a catalog + * volume) to make visible to this agent. Per-agent skills under + * `/skills/` are always visible and need not be listed here. Ignored + * when the plugin's `autoInheritSkills` makes every global skill visible. + */ + skills?: string[]; maxSteps?: number; maxTokens?: number; /** @@ -128,6 +136,7 @@ const ALLOWED_KEYS = new Set([ "model", "tools", "agents", + "skills", "maxSteps", "maxTokens", "generationParams", @@ -311,13 +320,13 @@ export function parseFrontmatter( raw: string, sourcePath?: string, ): { data: Frontmatter | null; content: string } { - const match = raw.match(/^---\r?\n([\s\S]*?)\r?\n---\r?\n?([\s\S]*)$/); - if (!match) { - return { data: null, content: raw.trim() }; + const { yaml: yamlBlock, body } = splitFrontmatter(raw); + if (yamlBlock === null) { + return { data: null, content: body }; } let parsed: unknown; try { - parsed = yaml.load(match[1]); + parsed = yaml.load(yamlBlock); } catch (err) { const src = sourcePath ? ` (${sourcePath})` : ""; throw new Error( @@ -325,7 +334,7 @@ export function parseFrontmatter( ); } if (parsed === null || parsed === undefined) { - return { data: {}, content: match[2].trim() }; + return { data: {}, content: body }; } if (typeof parsed !== "object" || Array.isArray(parsed)) { const src = sourcePath ? ` (${sourcePath})` : ""; @@ -341,7 +350,7 @@ export function parseFrontmatter( ); } } - return { data: data as Frontmatter, content: match[2].trim() }; + return { data: data as Frontmatter, content: body }; } const isNumber = (v: unknown): v is number => typeof v === "number"; @@ -411,6 +420,44 @@ function parseGenerationParams( return Object.keys(out).length > 0 ? (out as GenerationParams) : undefined; } +/** + * Defensively parses a frontmatter `skills:` list into deduped skill names. + * Non-array values and non-string/empty entries are dropped with a warning, + * so a malformed list is visible rather than silently applied. Returns + * `undefined` when nothing valid is present. + */ +function parseSkillsFrontmatter( + value: unknown, + sourcePath?: string, +): string[] | undefined { + if (value === undefined) return undefined; + const where = sourcePath ?? ""; + if (!Array.isArray(value)) { + logger.warn( + "Ignoring 'skills' in %s: expected an array of skill names", + where, + ); + return undefined; + } + const out: string[] = []; + const seen = new Set(); + for (const item of value) { + if (typeof item !== "string" || item.trim() === "") { + logger.warn( + "Ignoring invalid 'skills' entry in %s: %s", + where, + JSON.stringify(item), + ); + continue; + } + const name = item.trim(); + if (seen.has(name)) continue; + seen.add(name); + out.push(name); + } + return out.length > 0 ? out : undefined; +} + function buildDefinition( name: string, raw: string, @@ -433,6 +480,7 @@ function buildDefinition( instructions: content, model, tools: Object.keys(tools).length > 0 ? tools : undefined, + skills: parseSkillsFrontmatter(fm.skills, filePath), maxSteps: typeof fm.maxSteps === "number" ? fm.maxSteps : undefined, maxTokens: typeof fm.maxTokens === "number" ? fm.maxTokens : undefined, generationParams: parseGenerationParams(fm.generationParams, filePath), diff --git a/packages/appkit/src/core/agent/skills/index.ts b/packages/appkit/src/core/agent/skills/index.ts new file mode 100644 index 000000000..40d00e8f9 --- /dev/null +++ b/packages/appkit/src/core/agent/skills/index.ts @@ -0,0 +1,6 @@ +export { loadSkillsFromDir } from "./load-skills"; +export { parseSkill } from "./parse-skill"; +export { readSkillResource } from "./read-resource"; +export { renderLoadedSkill, renderSkillCatalog } from "./render"; +export { resolveSkill, resolveSkillCatalog } from "./resolve-catalog"; +export type { ResolvedSkillCatalog, SkillDefinition } from "./types"; diff --git a/packages/appkit/src/core/agent/skills/load-skills.ts b/packages/appkit/src/core/agent/skills/load-skills.ts new file mode 100644 index 000000000..60718ebc2 --- /dev/null +++ b/packages/appkit/src/core/agent/skills/load-skills.ts @@ -0,0 +1,94 @@ +import type { Dirent } from "node:fs"; +import fs from "node:fs/promises"; +import path from "node:path"; + +import { createLogger } from "../../../logging/logger"; +import { parseSkill } from "./parse-skill"; +import type { SkillDefinition, SkillSource } from "./types"; + +const logger = createLogger("agents:skills"); + +const SKILL_FILE = "SKILL.md"; + +/** + * Discovers skills under `dir` — one subfolder per skill, each containing a + * `SKILL.md`. Returns `[]` if the directory does not exist. Folders without a + * `SKILL.md` are skipped with a warning (they may be non-skill assets). + * + * Reads bodies eagerly at load time; the body is only *injected* into model + * context on demand, so reading a small markdown file at boot is cheap. + */ +export async function loadSkillsFromDir( + dir: string, + source: SkillSource, +): Promise { + let entries: Dirent[]; + try { + entries = await fs.readdir(dir, { withFileTypes: true }); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === "ENOENT") { + return []; + } + throw err; + } + + const skillDirs = entries + .filter((e) => e.isDirectory()) + .map((e) => e.name) + .sort(); + + const skills: SkillDefinition[] = []; + for (const name of skillDirs) { + const skillDir = path.join(dir, name); + const skillFile = path.join(skillDir, SKILL_FILE); + let raw: string; + try { + raw = await fs.readFile(skillFile, "utf-8"); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === "ENOENT") { + logger.warn("Skipping '%s': no %s found.", skillDir, SKILL_FILE); + continue; + } + throw err; + } + + const parsed = parseSkill(raw, skillFile); + const files = await listResourceFiles(skillDir); + skills.push({ + name: parsed.name, + description: parsed.description, + body: parsed.body, + source, + dir: skillDir, + files, + allowedTools: parsed.allowedTools, + }); + } + + return skills; +} + +/** + * Recursively lists resource files under a skill directory, returning relative + * posix paths and excluding the top-level `SKILL.md`. Used to build the file + * manifest `load_skill` returns so the model knows what else it can read. + */ +async function listResourceFiles(baseDir: string): Promise { + const out: string[] = []; + + async function walk(current: string, rel: string): Promise { + const entries = await fs.readdir(current, { withFileTypes: true }); + for (const entry of entries.sort((a, b) => a.name.localeCompare(b.name))) { + const childRel = rel ? `${rel}/${entry.name}` : entry.name; + if (entry.isDirectory()) { + await walk(path.join(current, entry.name), childRel); + } else if (entry.isFile()) { + if (rel === "" && entry.name === SKILL_FILE) continue; + out.push(childRel); + } + } + } + + await walk(baseDir, ""); + return out; +} diff --git a/packages/appkit/src/core/agent/skills/parse-skill.ts b/packages/appkit/src/core/agent/skills/parse-skill.ts new file mode 100644 index 000000000..bd622bc11 --- /dev/null +++ b/packages/appkit/src/core/agent/skills/parse-skill.ts @@ -0,0 +1,130 @@ +import yaml from "js-yaml"; + +import { createLogger } from "../../../logging/logger"; +import { splitFrontmatter } from "../frontmatter"; + +const logger = createLogger("agents:skills"); + +/** + * Frontmatter keys AppKit recognizes. Compatibility-first: this is the + * Anthropic `SKILL.md` surface (`name`, `description`, `license`, + * `allowed-tools`, `metadata`) so skills authored for Claude Code / Cursor + * load unmodified. Unknown keys warn rather than error. + */ +const KNOWN_SKILL_KEYS = new Set([ + "name", + "description", + "license", + "allowed-tools", + "metadata", +]); + +/** Addressable-name guard: no `:` (qualified-name separator), no `/`, no whitespace. */ +const NAME_RE = /^[a-zA-Z0-9][a-zA-Z0-9_.-]*$/; + +interface ParsedSkill { + name: string; + description: string; + body: string; + allowedTools?: string[]; +} + +/** + * Parses a `SKILL.md` string. Requires non-empty `name` + `description`; + * validates the name is addressable; warns on unknown frontmatter keys. + */ +export function parseSkill(raw: string, sourcePath: string): ParsedSkill { + const { yaml: yamlBlock, body } = splitFrontmatter(raw); + if (yamlBlock === null) { + throw new Error( + `Skill file ${sourcePath} has no YAML frontmatter (expected '--- name/description ---').`, + ); + } + + let parsed: unknown; + try { + parsed = yaml.load(yamlBlock); + } catch (err) { + throw new Error( + `Invalid YAML frontmatter in ${sourcePath}: ${err instanceof Error ? err.message : String(err)}`, + ); + } + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { + throw new Error( + `Skill frontmatter in ${sourcePath} must be a YAML object.`, + ); + } + + const data = parsed as Record; + const { name, description } = data; + + if (typeof name !== "string" || name.trim() === "") { + throw new Error( + `Skill ${sourcePath} is missing a non-empty 'name' in frontmatter.`, + ); + } + const trimmedName = name.trim(); + if (!NAME_RE.test(trimmedName)) { + throw new Error( + `Skill '${trimmedName}' (${sourcePath}) has an invalid name: use letters, digits, '.', '_', '-' only (no ':', '/', or spaces).`, + ); + } + if (typeof description !== "string" || description.trim() === "") { + throw new Error( + `Skill '${trimmedName}' (${sourcePath}) is missing a non-empty 'description' in frontmatter.`, + ); + } + + for (const key of Object.keys(data)) { + if (!KNOWN_SKILL_KEYS.has(key)) { + logger.warn( + "Ignoring unknown SKILL.md frontmatter key '%s' in %s", + key, + sourcePath, + ); + } + } + + return { + name: trimmedName, + description: description.trim(), + body, + allowedTools: parseAllowedTools( + data["allowed-tools"], + trimmedName, + sourcePath, + ), + }; +} + +/** + * Accepts `allowed-tools` as a string[] or a comma-separated string (both + * appear in the wild). Returns `undefined` when absent/empty/malformed. + */ +function parseAllowedTools( + value: unknown, + skillName: string, + sourcePath: string, +): string[] | undefined { + if (value === undefined) return undefined; + + let list: string[]; + if (typeof value === "string") { + list = value.split(","); + } else if ( + Array.isArray(value) && + value.every((v) => typeof v === "string") + ) { + list = value as string[]; + } else { + logger.warn( + "Ignoring 'allowed-tools' for skill '%s' in %s: expected string or string[]", + skillName, + sourcePath, + ); + return undefined; + } + + const cleaned = list.map((s) => s.trim()).filter((s) => s.length > 0); + return cleaned.length > 0 ? cleaned : undefined; +} diff --git a/packages/appkit/src/core/agent/skills/read-resource.ts b/packages/appkit/src/core/agent/skills/read-resource.ts new file mode 100644 index 000000000..8c14d9e59 --- /dev/null +++ b/packages/appkit/src/core/agent/skills/read-resource.ts @@ -0,0 +1,52 @@ +import fs from "node:fs/promises"; +import path from "node:path"; + +/** Read cap for a bundled skill resource file (bytes). */ +const MAX_SKILL_FILE_BYTES = 1_000_000; + +/** + * Reads a bundled skill resource file from local disk, constrained to the + * skill's own directory. The path must be relative; `..` traversal, null + * bytes, and absolute paths are rejected, and the resolved path is verified + * to stay within `baseDir` (a containment guard the agent markdown loader + * does not itself apply). Throws when the target is missing, not a file, or + * exceeds the size cap. + */ +export async function readSkillResource( + baseDir: string, + relPath: string, + maxSize = MAX_SKILL_FILE_BYTES, +): Promise { + if (relPath.includes("\0")) { + throw new Error("Path must not contain null bytes."); + } + if (relPath.length > 4096) { + throw new Error("Path exceeds the maximum length of 4096 characters."); + } + if (path.isAbsolute(relPath)) { + throw new Error( + "Skill resource path must be relative to the skill directory.", + ); + } + if (relPath.split(/[\\/]/).some((segment) => segment === "..")) { + throw new Error('Path traversal ("../") is not allowed.'); + } + + const root = path.resolve(baseDir); + const abs = path.resolve(root, relPath); + if (abs !== root && !abs.startsWith(root + path.sep)) { + throw new Error("Resolved path escapes the skill directory."); + } + + const stat = await fs.stat(abs); + if (!stat.isFile()) { + throw new Error(`Skill resource '${relPath}' is not a file.`); + } + if (stat.size > maxSize) { + throw new Error( + `Skill resource '${relPath}' exceeds the ${maxSize}-byte read limit.`, + ); + } + + return fs.readFile(abs, "utf-8"); +} diff --git a/packages/appkit/src/core/agent/skills/render.ts b/packages/appkit/src/core/agent/skills/render.ts new file mode 100644 index 000000000..1f87247da --- /dev/null +++ b/packages/appkit/src/core/agent/skills/render.ts @@ -0,0 +1,42 @@ +import type { SkillCatalogEntry, SkillDefinition } from "./types"; + +/** + * Renders the always-on skill catalog block appended to an agent's system + * prompt. Lists each visible skill's name + description and tells the model to + * call `load_skill` before acting on a matching task. + */ +export function renderSkillCatalog(entries: SkillCatalogEntry[]): string { + return [ + "## Available skills", + "When a task matches one of these skills, call the `load_skill` tool with the skill's exact name to load its full instructions before proceeding.", + "", + ...entries.map((e) => `- **${e.name}**: ${e.description}`), + ].join("\n"); +} + +/** + * Renders the tool-result payload returned by `load_skill`: the skill body + * plus a manifest of bundled files (readable via `read_skill_file`) and any + * advisory `allowed-tools` hint. + */ +export function renderLoadedSkill(skill: SkillDefinition): string { + const parts = [`# Skill: ${skill.name}`, "", skill.body]; + + if (skill.files.length > 0) { + parts.push( + "", + "## Bundled files", + "Read any of these with the `read_skill_file` tool (pass this skill's name and the file path):", + ...skill.files.map((f) => `- ${f}`), + ); + } + + if (skill.allowedTools && skill.allowedTools.length > 0) { + parts.push( + "", + `_Suggested tools for this skill: ${skill.allowedTools.join(", ")}._`, + ); + } + + return parts.join("\n"); +} diff --git a/packages/appkit/src/core/agent/skills/resolve-catalog.ts b/packages/appkit/src/core/agent/skills/resolve-catalog.ts new file mode 100644 index 000000000..410b18bf2 --- /dev/null +++ b/packages/appkit/src/core/agent/skills/resolve-catalog.ts @@ -0,0 +1,138 @@ +import { createLogger } from "../../../logging/logger"; +import type { + ResolvedSkillCatalog, + SkillCatalogEntry, + SkillDefinition, + SkillSource, +} from "./types"; + +const logger = createLogger("agents:skills"); + +/** Qualified-name scope prefix per source, used only on cross-source collision. */ +const SCOPE_BY_SOURCE: Record = { + "bundle-agent": "agent", + "bundle-global": "bundle", + volume: "volume", +}; + +interface ResolveCatalogInput { + agentName: string; + /** The agent's `skills:` frontmatter — opt-in selection from the global pool. */ + agentSkillNames?: string[]; + /** Skills private to this agent (`/skills/`), always visible. */ + perAgentSkills: SkillDefinition[]; + /** Shared pool (bundle-global + volume), visible only when opted in or inherited. */ + globalSkills: SkillDefinition[]; + /** When true, every global skill is visible without an explicit `skills:` list. */ + autoInherit: boolean; +} + +/** + * Applies visibility (per-agent auto; global opt-in or auto-inherit) then + * collision handling: a unique name is addressable bare; a name provided by + * multiple sources becomes `:name` per source and the bare name is + * marked ambiguous (addressing it errors with the alternatives). Two skills + * with the same name from the *same* source is a fatal config error. + */ +export function resolveSkillCatalog( + input: ResolveCatalogInput, +): ResolvedSkillCatalog { + const { + agentName, + agentSkillNames, + perAgentSkills, + globalSkills, + autoInherit, + } = input; + + const visible: SkillDefinition[] = [...perAgentSkills]; + if (autoInherit) { + visible.push(...globalSkills); + } else if (agentSkillNames && agentSkillNames.length > 0) { + const wanted = new Set(agentSkillNames); + for (const skill of globalSkills) { + if (wanted.has(skill.name)) visible.push(skill); + } + const localNames = new Set(perAgentSkills.map((s) => s.name)); + const globalNames = new Set(globalSkills.map((s) => s.name)); + for (const want of agentSkillNames) { + if (!globalNames.has(want) && !localNames.has(want)) { + logger.warn( + "Agent '%s' lists skill '%s' in 'skills:', but no global or per-agent skill with that name exists.", + agentName, + want, + ); + } + } + } + + const byName = new Map(); + for (const skill of visible) { + const group = byName.get(skill.name) ?? []; + group.push(skill); + byName.set(skill.name, group); + } + + const byAddress = new Map(); + const ambiguous = new Map(); + + for (const [name, group] of byName) { + if (group.length === 1) { + byAddress.set(name, group[0]); + continue; + } + + const alternatives: string[] = []; + for (const skill of group) { + const qualified = `${SCOPE_BY_SOURCE[skill.source]}:${name}`; + const existing = byAddress.get(qualified); + if (existing) { + throw new Error( + `Agent '${agentName}': two '${skill.source}' skills are both named '${name}' ` + + `(${existing.dir} and ${skill.dir}). Skill names must be unique within a source.`, + ); + } + byAddress.set(qualified, skill); + alternatives.push(qualified); + } + alternatives.sort(); + ambiguous.set(name, alternatives); + logger.warn( + "Agent '%s': skill name '%s' is provided by multiple sources; address it as %s.", + agentName, + name, + alternatives.join(" or "), + ); + } + + const catalog: SkillCatalogEntry[] = [...byAddress.entries()] + .map(([address, skill]) => ({ + name: address, + description: skill.description, + })) + .sort((a, b) => a.name.localeCompare(b.name)); + + return { byAddress, ambiguous, catalog }; +} + +/** + * Resolves a requested skill name (bare or qualified) against a catalog. + * Throws a helpful error on ambiguous or unknown names. + */ +export function resolveSkill( + catalog: ResolvedSkillCatalog, + requested: string, +): SkillDefinition { + const direct = catalog.byAddress.get(requested); + if (direct) return direct; + + const alternatives = catalog.ambiguous.get(requested); + if (alternatives) { + throw new Error( + `Skill '${requested}' is ambiguous; specify one of: ${alternatives.join(", ")}.`, + ); + } + + const available = [...catalog.byAddress.keys()].sort().join(", ") || ""; + throw new Error(`Unknown skill '${requested}'. Available: ${available}.`); +} diff --git a/packages/appkit/src/core/agent/skills/tests/skills.test.ts b/packages/appkit/src/core/agent/skills/tests/skills.test.ts new file mode 100644 index 000000000..3ef906eb6 --- /dev/null +++ b/packages/appkit/src/core/agent/skills/tests/skills.test.ts @@ -0,0 +1,330 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; + +import { loadSkillsFromDir } from "../load-skills"; +import { parseSkill } from "../parse-skill"; +import { readSkillResource } from "../read-resource"; +import { renderLoadedSkill, renderSkillCatalog } from "../render"; +import { resolveSkill, resolveSkillCatalog } from "../resolve-catalog"; +import type { SkillDefinition, SkillSource } from "../types"; + +let workDir: string; + +beforeEach(() => { + workDir = fs.mkdtempSync(path.join(os.tmpdir(), "skills-test-")); +}); + +afterEach(() => { + fs.rmSync(workDir, { recursive: true, force: true }); + vi.restoreAllMocks(); +}); + +/** Writes `//SKILL.md` plus optional sibling resource files. */ +function writeSkill( + name: string, + content: string, + files: Record = {}, +) { + const dir = path.join(workDir, name); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, "SKILL.md"), content, "utf-8"); + for (const [rel, body] of Object.entries(files)) { + const abs = path.join(dir, rel); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, body, "utf-8"); + } + return dir; +} + +/** Convenience for resolver tests that don't need real files. */ +function skill( + name: string, + source: SkillSource, + overrides: Partial = {}, +): SkillDefinition { + return { + name, + description: `${name} description`, + body: `${name} body`, + source, + dir: `/fake/${source}/${name}`, + files: [], + ...overrides, + }; +} + +describe("parseSkill", () => { + test("parses name, description, and body", () => { + const parsed = parseSkill( + "---\nname: pdf\ndescription: Work with PDFs\n---\nHow to work with PDFs.", + "SKILL.md", + ); + expect(parsed).toMatchObject({ + name: "pdf", + description: "Work with PDFs", + body: "How to work with PDFs.", + }); + expect(parsed.allowedTools).toBeUndefined(); + }); + + test("accepts allowed-tools as an array or comma string", () => { + const arr = parseSkill( + "---\nname: a\ndescription: d\nallowed-tools:\n - read\n - grep\n---\nbody", + "SKILL.md", + ); + expect(arr.allowedTools).toEqual(["read", "grep"]); + + const str = parseSkill( + "---\nname: b\ndescription: d\nallowed-tools: read, grep\n---\nbody", + "SKILL.md", + ); + expect(str.allowedTools).toEqual(["read", "grep"]); + }); + + test("throws when name or description is missing", () => { + expect(() => + parseSkill("---\ndescription: d\n---\nbody", "SKILL.md"), + ).toThrow(/missing a non-empty 'name'/); + expect(() => parseSkill("---\nname: a\n---\nbody", "SKILL.md")).toThrow( + /missing a non-empty 'description'/, + ); + }); + + test("rejects a name that breaks addressing", () => { + expect(() => + parseSkill("---\nname: bad:name\ndescription: d\n---\nbody", "SKILL.md"), + ).toThrow(/invalid name/); + }); + + test("throws when frontmatter is absent", () => { + expect(() => parseSkill("no frontmatter here", "SKILL.md")).toThrow( + /no YAML frontmatter/, + ); + }); + + test("warns on unknown frontmatter keys, keeps parsing", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const parsed = parseSkill( + "---\nname: a\ndescription: d\nbananas: 3\n---\nbody", + "SKILL.md", + ); + expect(parsed.name).toBe("a"); + expect(warn).toHaveBeenCalled(); + }); +}); + +describe("loadSkillsFromDir", () => { + test("returns [] for a missing directory", async () => { + const skills = await loadSkillsFromDir( + path.join(workDir, "nope"), + "bundle-global", + ); + expect(skills).toEqual([]); + }); + + test("discovers skills and enumerates resource files recursively", async () => { + writeSkill("pdf", "---\nname: pdf\ndescription: d\n---\nbody", { + "reference.md": "ref", + "scripts/extract.py": "print(1)", + }); + const skills = await loadSkillsFromDir(workDir, "bundle-global"); + expect(skills).toHaveLength(1); + expect(skills[0]).toMatchObject({ + name: "pdf", + description: "d", + body: "body", + source: "bundle-global", + }); + expect(skills[0].files).toEqual(["reference.md", "scripts/extract.py"]); + }); + + test("skips folders without a SKILL.md", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + fs.mkdirSync(path.join(workDir, "not-a-skill"), { recursive: true }); + writeSkill("real", "---\nname: real\ndescription: d\n---\nbody"); + const skills = await loadSkillsFromDir(workDir, "bundle-global"); + expect(skills.map((s) => s.name)).toEqual(["real"]); + expect(warn).toHaveBeenCalled(); + }); +}); + +describe("resolveSkillCatalog", () => { + test("per-agent skills are auto-visible; global skills are opt-in", () => { + const local = skill("local", "bundle-agent"); + const g1 = skill("wanted", "bundle-global"); + const g2 = skill("unwanted", "bundle-global"); + + const catalog = resolveSkillCatalog({ + agentName: "a", + agentSkillNames: ["wanted"], + perAgentSkills: [local], + globalSkills: [g1, g2], + autoInherit: false, + }); + + expect([...catalog.byAddress.keys()].sort()).toEqual(["local", "wanted"]); + expect(catalog.catalog.map((e) => e.name).sort()).toEqual([ + "local", + "wanted", + ]); + }); + + test("autoInherit exposes every global skill without an opt-in list", () => { + const catalog = resolveSkillCatalog({ + agentName: "a", + perAgentSkills: [], + globalSkills: [skill("x", "bundle-global"), skill("y", "bundle-global")], + autoInherit: true, + }); + expect([...catalog.byAddress.keys()].sort()).toEqual(["x", "y"]); + }); + + test("warns for opt-in names that match no skill", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + resolveSkillCatalog({ + agentName: "a", + agentSkillNames: ["ghost"], + perAgentSkills: [], + globalSkills: [skill("real", "bundle-global")], + autoInherit: false, + }); + expect(warn).toHaveBeenCalled(); + }); + + test("cross-source name collision produces qualified names + ambiguous bare", () => { + const catalog = resolveSkillCatalog({ + agentName: "a", + perAgentSkills: [skill("dup", "bundle-agent")], + globalSkills: [skill("dup", "bundle-global"), skill("dup", "volume")], + autoInherit: true, + }); + expect([...catalog.byAddress.keys()].sort()).toEqual([ + "agent:dup", + "bundle:dup", + "volume:dup", + ]); + expect(catalog.ambiguous.get("dup")).toEqual([ + "agent:dup", + "bundle:dup", + "volume:dup", + ]); + }); + + test("throws when two skills from the same source share a name", () => { + expect(() => + resolveSkillCatalog({ + agentName: "a", + perAgentSkills: [], + globalSkills: [ + skill("dup", "bundle-global", { dir: "/a" }), + skill("dup", "bundle-global", { dir: "/b" }), + ], + autoInherit: true, + }), + ).toThrow(/unique within a source/); + }); +}); + +describe("resolveSkill", () => { + const catalog = resolveSkillCatalog({ + agentName: "a", + perAgentSkills: [skill("solo", "bundle-agent")], + globalSkills: [skill("dup", "bundle-global"), skill("dup", "volume")], + autoInherit: true, + }); + + test("resolves a bare unique name", () => { + expect(resolveSkill(catalog, "solo").name).toBe("solo"); + }); + + test("resolves a qualified name", () => { + expect(resolveSkill(catalog, "volume:dup").source).toBe("volume"); + }); + + test("errors on an ambiguous bare name, listing alternatives", () => { + expect(() => resolveSkill(catalog, "dup")).toThrow( + /ambiguous.*bundle:dup.*volume:dup/, + ); + }); + + test("errors on an unknown name", () => { + expect(() => resolveSkill(catalog, "missing")).toThrow(/Unknown skill/); + }); +}); + +describe("renderSkillCatalog", () => { + test("lists each entry and points at load_skill", () => { + const out = renderSkillCatalog([ + { name: "pdf", description: "Work with PDFs" }, + { name: "sql", description: "Write SQL" }, + ]); + expect(out).toContain("load_skill"); + expect(out).toContain("**pdf**: Work with PDFs"); + expect(out).toContain("**sql**: Write SQL"); + }); +}); + +describe("renderLoadedSkill", () => { + test("includes the body and a file manifest when present", () => { + const out = renderLoadedSkill( + skill("pdf", "bundle-global", { + body: "Detailed PDF instructions.", + files: ["reference.md", "scripts/x.py"], + allowedTools: ["read", "grep"], + }), + ); + expect(out).toContain("Detailed PDF instructions."); + expect(out).toContain("read_skill_file"); + expect(out).toContain("- reference.md"); + expect(out).toContain("- scripts/x.py"); + expect(out).toContain("Suggested tools for this skill: read, grep"); + }); + + test("omits the manifest when there are no bundled files", () => { + const out = renderLoadedSkill( + skill("bare", "bundle-agent", { body: "Just prose.", files: [] }), + ); + expect(out).toContain("Just prose."); + expect(out).not.toContain("Bundled files"); + }); +}); + +describe("readSkillResource", () => { + test("reads a file inside the skill directory", async () => { + const dir = writeSkill("pdf", "---\nname: pdf\ndescription: d\n---\nbody", { + "reference.md": "the reference", + }); + await expect(readSkillResource(dir, "reference.md")).resolves.toBe( + "the reference", + ); + }); + + test("rejects traversal, absolute paths, and escapes", async () => { + const dir = writeSkill("pdf", "---\nname: pdf\ndescription: d\n---\nbody"); + // A real secret sitting next to the skill dir, reachable only via escape. + fs.writeFileSync(path.join(workDir, "secret.txt"), "top secret", "utf-8"); + await expect(readSkillResource(dir, "../secret.txt")).rejects.toThrow( + /traversal/, + ); + await expect( + readSkillResource(dir, path.join(workDir, "secret.txt")), + ).rejects.toThrow(/relative/); + }); + + test("throws when the file is missing", async () => { + const dir = writeSkill("pdf", "---\nname: pdf\ndescription: d\n---\nbody"); + await expect(readSkillResource(dir, "nope.md")).rejects.toThrow(); + }); + + test("enforces the size cap", async () => { + const dir = writeSkill("pdf", "---\nname: pdf\ndescription: d\n---\nbody", { + "big.txt": "x".repeat(50), + }); + await expect(readSkillResource(dir, "big.txt", 10)).rejects.toThrow( + /read limit/, + ); + }); +}); diff --git a/packages/appkit/src/core/agent/skills/types.ts b/packages/appkit/src/core/agent/skills/types.ts new file mode 100644 index 000000000..34717551c --- /dev/null +++ b/packages/appkit/src/core/agent/skills/types.ts @@ -0,0 +1,49 @@ +/** Where a skill was discovered. Drives the qualified name used on collision. */ +export type SkillSource = "bundle-agent" | "bundle-global" | "volume"; + +/** + * A single skill: a `SKILL.md` (frontmatter `name`+`description` + Markdown + * body) plus any bundled resource files in the same directory. The body is + * loaded into model context on demand (via the `load_skill` tool or a forced + * `/skill-name` invocation); only `name`+`description` are always-on in the + * prompt catalog. + */ +export interface SkillDefinition { + /** Frontmatter `name`. The addressable skill id. */ + name: string; + /** Frontmatter `description`. Injected into the always-on prompt catalog. */ + description: string; + /** Markdown body — the instructions loaded on demand. */ + body: string; + /** Where the skill came from. */ + source: SkillSource; + /** Absolute directory containing `SKILL.md` and any bundled resources. */ + dir: string; + /** Relative posix paths of bundled resource files (excludes `SKILL.md`). */ + files: string[]; + /** + * Optional advisory tool allowlist from frontmatter `allowed-tools`. Surfaced + * as a hint in v1 — NOT enforced (loading a skill does not restrict the + * agent's callable tools). + */ + allowedTools?: string[]; +} + +/** The always-on prompt entry for a skill (what the model sees before loading). */ +export interface SkillCatalogEntry { + /** Addressable name — bare when unique, `:name` when collided. */ + name: string; + description: string; +} + +/** + * Per-agent resolved skill catalog: visibility + collision rules applied. + * `byAddress` maps every addressable name (bare or qualified) to its skill; + * `ambiguous` maps a bare name shadowed by multiple sources to the qualified + * alternatives; `catalog` is the always-on prompt list (one entry per address). + */ +export interface ResolvedSkillCatalog { + byAddress: Map; + ambiguous: Map; + catalog: SkillCatalogEntry[]; +} diff --git a/packages/appkit/src/core/agent/tests/load-agents.test.ts b/packages/appkit/src/core/agent/tests/load-agents.test.ts index e17160168..cc927d9a8 100644 --- a/packages/appkit/src/core/agent/tests/load-agents.test.ts +++ b/packages/appkit/src/core/agent/tests/load-agents.test.ts @@ -194,6 +194,15 @@ describe("loadAgentsFromDir", () => { expect(Object.keys(res.defs)).toEqual(["solo"]); }); + test("parses the skills opt-in list from frontmatter", async () => { + writeAgent( + "picker", + "---\nendpoint: e\nskills:\n - pdf\n - pdf\n - sql\n---\nPrompt.", + ); + const res = await loadAgentsFromDir(workDir, {}); + expect(res.defs.picker.skills).toEqual(["pdf", "sql"]); + }); + test("picks up default: true from frontmatter (deterministic sorted ids)", async () => { writeAgent("one", "---\nendpoint: a\n---\nOne."); writeAgent("two", "---\nendpoint: b\ndefault: true\n---\nTwo."); diff --git a/packages/appkit/src/core/agent/types.ts b/packages/appkit/src/core/agent/types.ts index 234aa45c1..53dae3c87 100644 --- a/packages/appkit/src/core/agent/types.ts +++ b/packages/appkit/src/core/agent/types.ts @@ -8,6 +8,7 @@ import type { import type { GenerationParams } from "../../agents/databricks"; import type { McpHostPolicyConfig } from "../../connectors/mcp"; +import type { ResolvedSkillCatalog } from "./skills/types"; import type { FunctionTool } from "./tools/function-tool"; import type { HostedTool } from "./tools/hosted-tools"; @@ -173,6 +174,13 @@ export interface AgentDefinition { tools?: AgentTools | AgentToolsFn; /** Sub-agents, exposed as `agent-` tools on this agent. */ agents?: Record; + /** + * Names of global skills (shared `skills/` pool or catalog volume) to make + * visible to this agent. Per-agent skills under `/skills/` are always + * visible and need not be listed. Ignored when the plugin's + * `autoInheritSkills` makes every global skill visible. + */ + skills?: string[]; /** Override the plugin's baseSystemPrompt for this agent only. */ baseSystemPrompt?: BaseSystemPromptOption; maxSteps?: number; @@ -233,6 +241,28 @@ export interface AgentsPluginConfig extends BasePluginConfig { tools?: Record; /** Whether to auto-inherit every ToolProvider plugin's toolkit. Accepts a boolean shorthand. */ autoInheritTools?: boolean | AutoInheritToolsConfig; + /** + * Whether every global skill (shared `skills/` pool or catalog volume) is + * visible to an agent without listing it in `skills:` frontmatter. Off by + * default so each agent's always-on skill catalog stays lean; accepts a + * boolean shorthand or a per-origin `{ file, code }` config, mirroring + * {@link autoInheritTools}. + */ + autoInheritSkills?: boolean | AutoInheritToolsConfig; + /** + * Unity Catalog Volume path for catalog-sourced skills (e.g. + * `/Volumes///`). Falls back to the + * `DATABRICKS_VOLUME_AGENT_SKILLS` env var. Skills at `//SKILL.md` + * are discovered at boot and on `reload()` and read as the service principal. + */ + skillsVolume?: string; + /** + * Identity used to read catalog (volume) skills. v1 supports `"sp"` (default — + * a shared, service-principal-readable curated pool). `"obo"` is the reserved + * switch point for per-user skill volumes and is not wired yet (falls back to + * `"sp"` with a warning). + */ + skillCredentialMode?: "sp" | "obo"; /** Persistent thread store. Default: in-memory. */ threadStore?: ThreadStore; /** Customize or disable the AppKit base system prompt. */ @@ -343,6 +373,18 @@ export type ResolvedToolEntry = source: "hosted-supervisor"; spec: import("../../agents/supervisor-api").SupervisorTool; def: AgentToolDefinition; + } + | { + /** + * Built-in skill tools (`load_skill`, `read_skill_file`) injected into + * any agent that has a visible skill catalog. Executed in-process by the + * agents plugin against the agent's resolved catalog; read-only, so they + * bypass the approval gate. + */ + source: "skill"; + builtin: "load_skill" | "read_skill_file"; + catalog: ResolvedSkillCatalog; + def: AgentToolDefinition; }; export interface RegisteredAgent { @@ -357,6 +399,12 @@ export interface RegisteredAgent { generationParams?: GenerationParams; /** Mirrors `AgentDefinition.ephemeral` — skip thread persistence. */ ephemeral?: boolean; + /** + * Resolved per-agent skill catalog (visibility + collision rules applied). + * Present when any skill is visible to this agent; drives the always-on + * prompt catalog and `load_skill` dispatch. + */ + skills?: ResolvedSkillCatalog; } /** diff --git a/packages/appkit/src/plugins/agents/agents.ts b/packages/appkit/src/plugins/agents/agents.ts index 58c811aab..1a320528c 100644 --- a/packages/appkit/src/plugins/agents/agents.ts +++ b/packages/appkit/src/plugins/agents/agents.ts @@ -23,7 +23,9 @@ import { SUPERVISOR_EXTENSION_KEY, type SupervisorTool, } from "../../agents/supervisor-api"; +import { FilesConnector } from "../../connectors/files"; import { AppKitMcpClient, buildMcpHostPolicy } from "../../connectors/mcp"; +import { getWorkspaceClient } from "../../context"; import { consumeAdapterStream } from "../../core/agent/consume-adapter-stream"; import { loadAgentsFromDir } from "../../core/agent/load-agents"; import { @@ -33,6 +35,17 @@ import { } from "../../core/agent/load-code-agents"; import { normalizeToolResult } from "../../core/agent/normalize-result"; import { createPluginsProxy } from "../../core/agent/plugins-map"; +import { + loadSkillsFromDir, + parseSkill, + type ResolvedSkillCatalog, + readSkillResource, + renderLoadedSkill, + renderSkillCatalog, + resolveSkill, + resolveSkillCatalog, + type SkillDefinition, +} from "../../core/agent/skills"; import { buildBaseSystemPrompt, composeSystemPrompt, @@ -59,6 +72,7 @@ import { isToolkitEntry } from "../../core/agent/types"; import { createLogger } from "../../logging/logger"; import { Plugin, toPlugin } from "../../plugin"; import { defineManifest } from "../../registry"; +import type { WorkspaceClient } from "../../workspace-client"; import { agentStreamDefaults } from "./defaults"; import { EventChannel } from "./event-channel"; import { AgentEventTranslator } from "./event-translator"; @@ -206,6 +220,13 @@ export class AgentsPlugin extends Plugin implements ToolProvider { * negative, or `NaN`) can't degrade into immediate auto-denial of every * mutating tool call. */ + /** + * Shared global skill pool (bundle `skills/` + catalog volume), loaded once + * per registry build. Read by {@link buildRegisteredAgent} to resolve each + * agent's visible catalog and by the live `register` path. + */ + private globalSkills: SkillDefinition[] = []; + private cachedApprovalPolicy: { requireForDestructive: boolean; timeoutMs: number; @@ -344,6 +365,14 @@ export class AgentsPlugin extends Plugin implements ToolProvider { const discovered = await this.loadCodeAgents(); const deprecatedMapRaw = this.config.agents ?? {}; + // Global skills (bundle + configured UC volume) load in parallel; per-agent + // skills resolve later during agent build. + const [bundleSkills, volumeSkills] = await Promise.all([ + this.loadGlobalSkills(), + this.loadVolumeSkills(), + ]); + this.globalSkills = [...bundleSkills, ...volumeSkills]; + if (Object.keys(deprecatedMapRaw).length > 0) { this.warnAgentsMapDeprecated(); } @@ -622,7 +651,8 @@ export class AgentsPlugin extends Plugin implements ToolProvider { src: AgentSource, ): Promise { const adapter = await this.resolveAdapter(def, name); - const toolIndex = await this.buildToolIndex(name, def, src); + const skills = await this.resolveAgentSkills(name, def, src); + const toolIndex = await this.buildToolIndex(name, def, src, skills); warnOnCapabilityMismatch(name, adapter, toolIndex); @@ -636,9 +666,161 @@ export class AgentsPlugin extends Plugin implements ToolProvider { maxTokens: def.maxTokens, generationParams: def.generationParams, ephemeral: def.ephemeral, + skills, }; } + /** Loads the shared global skill pool from `/skills/`. */ + private async loadGlobalSkills(): Promise { + const dir = this.resolvedAgentsDir(); + if (!dir) return []; + return loadSkillsFromDir(path.join(dir, "skills"), "bundle-global"); + } + + /** Configured catalog-skills volume path, or null when none is set. */ + private resolveSkillsVolume(): string | null { + const configured = + this.config.skillsVolume ?? process.env.DATABRICKS_VOLUME_AGENT_SKILLS; + return configured && configured.trim() !== "" ? configured.trim() : null; + } + + /** + * Workspace client used to read catalog skills. v1 reads as the app service + * principal; `getWorkspaceClient()` resolves to SP outside a user scope + * (boot and skill-tool dispatch are both unscoped). This is the single + * switch point for a future OBO mode. + */ + private skillWorkspaceClient(): WorkspaceClient { + return getWorkspaceClient(); + } + + /** + * Discovers catalog skills from the configured UC Volume, read as the + * service principal at boot (and on `reload()`). Each `//` + * folder with a `SKILL.md` becomes a `source: "volume"` skill. Best-effort: + * a missing volume, unavailable workspace client, or a malformed individual + * skill is logged and skipped rather than failing the whole registry build. + */ + private async loadVolumeSkills(): Promise { + const volume = this.resolveSkillsVolume(); + if (!volume) return []; + + if ((this.config.skillCredentialMode ?? "sp") === "obo") { + logger.warn( + "skillCredentialMode 'obo' is not wired yet; reading catalog skills as the service principal.", + ); + } + + let client: WorkspaceClient; + try { + client = this.skillWorkspaceClient(); + } catch (err) { + logger.warn( + "Skipping catalog skills at '%s': no workspace client available (%s).", + volume, + err instanceof Error ? err.message : String(err), + ); + return []; + } + + const connector = new FilesConnector({ defaultVolume: volume }); + let entries: Awaited>; + try { + entries = await connector.list(client, volume); + } catch (err) { + logger.warn( + "Failed to list catalog skills volume '%s': %s", + volume, + err instanceof Error ? err.message : String(err), + ); + return []; + } + + const skills: SkillDefinition[] = []; + for (const entry of entries) { + if (!entry.is_directory || !entry.name || !entry.path) continue; + const skillDir = entry.path; + const skillFile = `${skillDir}/SKILL.md`; + try { + const raw = await connector.read(client, skillFile); + const parsed = parseSkill(raw, skillFile); + const files = await this.listVolumeSkillFiles( + connector, + client, + skillDir, + ); + skills.push({ + name: parsed.name, + description: parsed.description, + body: parsed.body, + source: "volume", + dir: skillDir, + files, + allowedTools: parsed.allowedTools, + }); + } catch (err) { + logger.warn( + "Skipping catalog skill '%s': %s", + skillDir, + err instanceof Error ? err.message : String(err), + ); + } + } + return skills; + } + + /** Lists a volume skill's resource files (one level, excluding SKILL.md). */ + private async listVolumeSkillFiles( + connector: FilesConnector, + client: WorkspaceClient, + skillDir: string, + ): Promise { + try { + const entries = await connector.list(client, skillDir); + return entries + .filter((e) => !e.is_directory && e.name && e.name !== "SKILL.md") + .map((e) => e.name as string) + .sort(); + } catch { + return []; + } + } + + /** + * Resolves the per-agent skill catalog: loads this agent's private skills + * (`//skills/`, file-origin only), then applies visibility + * (opt-in via `def.skills` or `autoInheritSkills`) and collision rules + * against the shared global pool. Returns `undefined` when nothing is + * visible so the prompt catalog and dispatch can cheaply skip skills. + */ + private async resolveAgentSkills( + name: string, + def: AgentDefinition, + src: AgentSource, + ): Promise { + const dir = this.resolvedAgentsDir(); + const perAgentSkills = + src.origin === "file" && dir + ? await loadSkillsFromDir( + path.join(dir, name, "skills"), + "bundle-agent", + ) + : []; + + const inherit = normalizeAutoInherit(this.config.autoInheritSkills); + const autoInherit = src.origin === "file" ? inherit.file : inherit.code; + + const catalog = resolveSkillCatalog({ + agentName: name, + agentSkillNames: def.skills, + perAgentSkills, + globalSkills: this.globalSkills, + autoInherit, + }); + + return catalog.byAddress.size > 0 ? catalog : undefined; + } + private async resolveAdapter( def: AgentDefinition, name: string, @@ -687,6 +869,7 @@ export class AgentsPlugin extends Plugin implements ToolProvider { agentName: string, def: AgentDefinition, src: AgentSource, + skills?: ResolvedSkillCatalog, ): Promise> { const index = new Map(); const hasDeclaredTools = def.tools !== undefined; @@ -781,6 +964,23 @@ export class AgentsPlugin extends Plugin implements ToolProvider { await this.connectHostedTools(hostedToCollect, index); } + // 3. Built-in skill tools, present only when this agent has a visible + // catalog. Injected last so they reliably shadow any same-named tool. + if (skills && skills.byAddress.size > 0) { + index.set("load_skill", { + source: "skill", + builtin: "load_skill", + catalog: skills, + def: LOAD_SKILL_TOOL_DEF, + }); + index.set("read_skill_file", { + source: "skill", + builtin: "read_skill_file", + catalog: skills, + def: READ_SKILL_FILE_TOOL_DEF, + }); + } + return index; } @@ -1041,9 +1241,17 @@ export class AgentsPlugin extends Plugin implements ToolProvider { } clientConfig(): Record { + // Per-agent skill catalog (name + description) so the client can offer a + // `/skill-name` autocomplete / picker. Only agents with a visible catalog + // appear here. + const skills: Record = {}; + for (const [name, agent] of this.agents) { + if (agent.skills) skills[name] = agent.skills.catalog; + } return { agents: Array.from(this.agents.keys()), defaultAgent: this.defaultAgentName, + skills, }; } @@ -1056,7 +1264,13 @@ export class AgentsPlugin extends Plugin implements ToolProvider { }); return; } - const { message, threadId, agent: agentName, mlflowRunId } = parsed.data; + const { + message, + threadId, + agent: agentName, + mlflowRunId, + skill, + } = parsed.data; const registered = this.resolveAgent(agentName); if (!registered) { @@ -1110,7 +1324,15 @@ export class AgentsPlugin extends Plugin implements ToolProvider { res.status(500).json({ error: "Thread operation failed" }); return; } - return this._streamAgent(req, res, registered, thread, userId, mlflowRunId); + return this._streamAgent( + req, + res, + registered, + thread, + userId, + mlflowRunId, + skill, + ); } /** @@ -1245,6 +1467,7 @@ export class AgentsPlugin extends Plugin implements ToolProvider { thread: Thread, userId: string, mlflowRunId?: string, + forcedSkill?: string, ): Promise { const abortController = new AbortController(); const signal = abortController.signal; @@ -1316,7 +1539,7 @@ export class AgentsPlugin extends Plugin implements ToolProvider { .getPluginNames() .filter((n) => n !== this.name && n !== "server") : []; - const fullPrompt = composePromptForAgent( + let fullPrompt = composePromptForAgent( registered, this.config.baseSystemPrompt, { @@ -1326,6 +1549,15 @@ export class AgentsPlugin extends Plugin implements ToolProvider { }, ); + // Deterministic `/skill-name` invocation: eagerly inject the + // requested skill's instructions into this turn rather than waiting + // for the model to call load_skill. The tool stays available for + // the model to auto-select others. + if (forcedSkill) { + const addendum = this.renderForcedSkill(registered, forcedSkill); + if (addendum) fullPrompt = `${fullPrompt}\n\n${addendum}`; + } + const messagesWithSystem: Message[] = [ { id: "system", @@ -1736,6 +1968,8 @@ export class AgentsPlugin extends Plugin implements ToolProvider { `Tool '${name}' is a hosted-supervisor tool and cannot be invoked from the Node process. ` + "It is executed server-side by the Databricks AI Gateway and is only reachable when the agent's model is a Supervisor API adapter.", ); + } else if (entry.source === "skill") { + result = await this.dispatchSkillTool(entry, args); } return result; @@ -1744,6 +1978,81 @@ export class AgentsPlugin extends Plugin implements ToolProvider { return normalizeToolResult(toolResult); } + /** + * Executes the built-in `load_skill` / `read_skill_file` tools against the + * agent's resolved skill catalog. `load_skill` returns a skill's body plus a + * manifest of bundled files; `read_skill_file` returns the contents of one + * of those files (bundle skills only in v1 — catalog-volume resource reads + * arrive with the volume source). + */ + private async dispatchSkillTool( + entry: Extract, + args: unknown, + ): Promise { + const obj = + typeof args === "object" && args !== null + ? (args as Record) + : {}; + const skillName = typeof obj.skill === "string" ? obj.skill.trim() : ""; + if (!skillName) { + throw new Error( + `'${entry.builtin}' requires a 'skill' argument naming the skill to use.`, + ); + } + + const skill = resolveSkill(entry.catalog, skillName); + + if (entry.builtin === "load_skill") { + return renderLoadedSkill(skill); + } + + // read_skill_file + const filePath = typeof obj.path === "string" ? obj.path.trim() : ""; + if (!filePath) { + throw new Error("'read_skill_file' requires a 'path' argument."); + } + if (!skill.files.includes(filePath)) { + throw new Error( + `Skill '${skill.name}' has no bundled file '${filePath}'. Available: ${ + skill.files.join(", ") || "" + }.`, + ); + } + + if (skill.source === "volume") { + const connector = new FilesConnector({ defaultVolume: skill.dir }); + return connector.read( + this.skillWorkspaceClient(), + `${skill.dir}/${filePath}`, + ); + } + return readSkillResource(skill.dir, filePath); + } + + /** + * Renders the prompt addendum for a force-loaded skill (`/skill-name`). + * Returns null when the agent has no catalog or the name doesn't resolve — + * an unusable request shouldn't fail the whole turn, so it's logged and the + * model proceeds with the catalog + `load_skill` still available. + */ + private renderForcedSkill( + registered: RegisteredAgent, + name: string, + ): string | null { + if (!registered.skills) return null; + try { + const skill = resolveSkill(registered.skills, name); + return `The user explicitly requested the "${skill.name}" skill for this turn. Its instructions:\n\n${renderLoadedSkill(skill)}`; + } catch (err) { + logger.warn( + "Ignoring forced skill '%s': %s", + name, + err instanceof Error ? err.message : String(err), + ); + return null; + } + } + /** * Runs a sub-agent in response to an `agent-` tool call. Returns the * concatenated text output to hand back to the parent adapter as the tool @@ -2024,6 +2333,45 @@ function normalizeAutoInherit(value: AgentsPluginConfig["autoInheritTools"]): { return { file: value.file ?? false, code: value.code ?? false }; } +/** Built-in tool the model calls to load a skill's full instructions on demand. */ +const LOAD_SKILL_TOOL_DEF: AgentToolDefinition = { + name: "load_skill", + description: + "Load the full instructions for one of the available skills by name. Call this before acting on a task that matches a skill's description. Returns the skill's instructions plus a list of any bundled files you can read with read_skill_file.", + parameters: { + type: "object", + properties: { + skill: { + type: "string", + description: + "The exact skill name to load, as shown in the available-skills list.", + }, + }, + required: ["skill"], + }, + annotations: { effect: "read" }, +}; + +/** Built-in tool for reading a resource file that a loaded skill references. */ +const READ_SKILL_FILE_TOOL_DEF: AgentToolDefinition = { + name: "read_skill_file", + description: + "Read a bundled resource file that a loaded skill references (e.g. a reference doc). Only files listed by load_skill for that skill are readable.", + parameters: { + type: "object", + properties: { + skill: { type: "string", description: "The skill that owns the file." }, + path: { + type: "string", + description: + "Relative path of the file within the skill, as listed by load_skill.", + }, + }, + required: ["skill", "path"], + }, + annotations: { effect: "read" }, +}; + function composePromptForAgent( registered: RegisteredAgent, pluginLevel: BaseSystemPromptOption | undefined, @@ -2043,7 +2391,13 @@ function composePromptForAgent( base = buildBaseSystemPrompt(ctx); } - return composeSystemPrompt(base, registered.instructions); + const composed = composeSystemPrompt(base, registered.instructions); + + // Append the always-on skill catalog (name + description only). Done here, + // after composeSystemPrompt, so it survives a custom/`false` base prompt. + const catalog = registered.skills?.catalog; + if (!catalog || catalog.length === 0) return composed; + return `${composed}\n\n${renderSkillCatalog(catalog)}`; } /** diff --git a/packages/appkit/src/plugins/agents/manifest.json b/packages/appkit/src/plugins/agents/manifest.json index 4d6f52485..529c640bc 100644 --- a/packages/appkit/src/plugins/agents/manifest.json +++ b/packages/appkit/src/plugins/agents/manifest.json @@ -32,6 +32,24 @@ "description": "MLflow experiment id traces are logged to" } } + }, + { + "type": "volume", + "alias": "Agent skills", + "resourceKey": "agents-skills", + "description": "Optional Unity Catalog Volume providing catalog-sourced agent skills (read-only)", + "permission": "READ_VOLUME", + "fields": { + "path": { + "env": "DATABRICKS_VOLUME_AGENT_SKILLS", + "description": "Volume path for agent skills (e.g. /Volumes/catalog/schema/skills)", + "discovery": { + "type": "kind", + "resourceKind": "volume", + "select": "full_name" + } + } + } } ] } diff --git a/packages/appkit/src/plugins/agents/schemas.ts b/packages/appkit/src/plugins/agents/schemas.ts index 6dc5040cb..24e38f1d5 100644 --- a/packages/appkit/src/plugins/agents/schemas.ts +++ b/packages/appkit/src/plugins/agents/schemas.ts @@ -40,6 +40,12 @@ export const chatRequestSchema = z.object({ * to a run-id-shaped length since it reaches trace metadata and logs. */ mlflowRunId: z.string().max(64).optional(), + /** + * Optional skill to force-load for this turn (deterministic `/skill-name` + * invocation). Its instructions are injected into the turn's context; the + * model can still auto-load others via the `load_skill` tool. + */ + skill: z.string().optional(), }); const messageItemSchema = z.object({ diff --git a/packages/appkit/src/plugins/agents/tests/dispatch-tool-call.test.ts b/packages/appkit/src/plugins/agents/tests/dispatch-tool-call.test.ts index f700379f4..d503fea6f 100644 --- a/packages/appkit/src/plugins/agents/tests/dispatch-tool-call.test.ts +++ b/packages/appkit/src/plugins/agents/tests/dispatch-tool-call.test.ts @@ -1,7 +1,13 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + import type express from "express"; -import { beforeEach, describe, expect, test, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { CacheManager } from "../../../cache"; +import { resolveSkillCatalog } from "../../../core/agent/skills/resolve-catalog"; +import type { SkillDefinition } from "../../../core/agent/skills/types"; import { createTestPluginContext } from "../../../testing"; import { AgentsPlugin } from "../agents"; @@ -458,3 +464,152 @@ describe("runSubAgent — sub-agent event forwarding", () => { expect(types).toContain("message_delta"); }); }); + +describe("dispatchToolCall — skill built-ins", () => { + let skillDir = ""; + + afterEach(() => { + if (skillDir) { + fs.rmSync(skillDir, { recursive: true, force: true }); + skillDir = ""; + } + }); + + function skillCatalog(skills: SkillDefinition[]) { + return resolveSkillCatalog({ + agentName: "a", + perAgentSkills: skills, + globalSkills: [], + autoInherit: false, + }); + } + + function skillToolIndex( + catalog: ReturnType, + ): Map { + const readOnly = { effect: "read" as const }; + return new Map([ + [ + "load_skill", + { + source: "skill", + builtin: "load_skill", + catalog, + def: { + name: "load_skill", + description: "load", + parameters: { type: "object" }, + annotations: readOnly, + }, + }, + ], + [ + "read_skill_file", + { + source: "skill", + builtin: "read_skill_file", + catalog, + def: { + name: "read_skill_file", + description: "read", + parameters: { type: "object" }, + annotations: readOnly, + }, + }, + ], + ]); + } + + test("load_skill returns the skill body + manifest and skips the gate", async () => { + const plugin = new AgentsPlugin({ dir: false }); + const { runState } = makeRunState(plugin); + const catalog = skillCatalog([ + { + name: "pdf", + description: "d", + body: "Detailed PDF steps.", + source: "bundle-agent", + dir: "/fake/pdf", + files: ["reference.md"], + }, + ]); + // biome-ignore lint/suspicious/noExplicitAny: stub gate to assert it never fires + (plugin as any).approvalGate.wait = vi.fn(); + + const result = await callDispatch(plugin, { + runState, + toolIndex: skillToolIndex(catalog), + name: "load_skill", + args: { skill: "pdf" }, + }); + + expect(String(result)).toContain("Detailed PDF steps."); + expect(String(result)).toContain("reference.md"); + // biome-ignore lint/suspicious/noExplicitAny: assertion on stub + expect((plugin as any).approvalGate.wait).not.toHaveBeenCalled(); + }); + + test("load_skill errors on an unknown skill name", async () => { + const plugin = new AgentsPlugin({ dir: false }); + const { runState } = makeRunState(plugin); + const catalog = skillCatalog([ + { + name: "pdf", + description: "d", + body: "b", + source: "bundle-agent", + dir: "/fake/pdf", + files: [], + }, + ]); + + await expect( + callDispatch(plugin, { + runState, + toolIndex: skillToolIndex(catalog), + name: "load_skill", + args: { skill: "ghost" }, + }), + ).rejects.toThrow(/Unknown skill/); + }); + + test("read_skill_file reads a listed file and rejects an unlisted path", async () => { + skillDir = fs.mkdtempSync(path.join(os.tmpdir(), "skill-dispatch-")); + fs.writeFileSync( + path.join(skillDir, "reference.md"), + "the reference", + "utf-8", + ); + const plugin = new AgentsPlugin({ dir: false }); + const { runState } = makeRunState(plugin); + const catalog = skillCatalog([ + { + name: "pdf", + description: "d", + body: "b", + source: "bundle-agent", + dir: skillDir, + files: ["reference.md"], + }, + ]); + const toolIndex = skillToolIndex(catalog); + + await expect( + callDispatch(plugin, { + runState, + toolIndex, + name: "read_skill_file", + args: { skill: "pdf", path: "reference.md" }, + }), + ).resolves.toContain("the reference"); + + await expect( + callDispatch(plugin, { + runState, + toolIndex, + name: "read_skill_file", + args: { skill: "pdf", path: "../secret" }, + }), + ).rejects.toThrow(); + }); +}); diff --git a/packages/appkit/src/plugins/agents/tests/skill-client.test.ts b/packages/appkit/src/plugins/agents/tests/skill-client.test.ts new file mode 100644 index 000000000..988e4f987 --- /dev/null +++ b/packages/appkit/src/plugins/agents/tests/skill-client.test.ts @@ -0,0 +1,148 @@ +import { describe, expect, test, vi } from "vitest"; + +import type { SkillDefinition } from "../../../core/agent/skills"; +import { resolveSkillCatalog } from "../../../core/agent/skills"; +import { AgentsPlugin } from "../agents"; + +/** + * Phase 4 — the server surfaces the skill catalog to the client via + * `clientConfig()`, and force-loads a skill for a turn via `renderForcedSkill` + * (the deterministic `/skill-name` path). Both are pure and don't touch a + * workspace, so no mocks are needed. + */ + +function skill( + name: string, + overrides: Partial = {}, +): SkillDefinition { + return { + name, + description: `${name} description`, + body: `${name} body`, + source: "bundle-agent", + dir: `/fake/${name}`, + files: [], + ...overrides, + }; +} + +function catalogOf(...skills: SkillDefinition[]) { + return resolveSkillCatalog({ + agentName: "a", + perAgentSkills: skills, + globalSkills: [], + autoInherit: false, + }); +} + +// biome-ignore lint/suspicious/noExplicitAny: minimal RegisteredAgent stub +function registeredWith(catalog?: ReturnType): any { + return { + name: "a", + instructions: "", + adapter: {}, + toolIndex: new Map(), + ...(catalog ? { skills: catalog } : {}), + }; +} + +describe("skills are wired uniformly for every registered agent", () => { + // Sub-agents are ordinary registered agents resolved through + // buildRegisteredAgent, so opting one into a skill gives it the same + // catalog + load_skill/read_skill_file tools as a top-level agent. + test("buildRegisteredAgent gives a code agent its catalog and skill tools", async () => { + const plugin = new AgentsPlugin({}); + // biome-ignore lint/suspicious/noExplicitAny: seed the shared pool + (plugin as any).globalSkills = [skill("x", { description: "X skill" })]; + + // biome-ignore lint/suspicious/noExplicitAny: call private with a stub adapter + const registered = await (plugin as any).buildRegisteredAgent( + "child", + { + instructions: "hi", + model: { + run: async function* () {}, + acceptsExtensions: [], + consumesInputTools: false, + }, + skills: ["x"], + }, + { origin: "code" }, + ); + + expect(registered.skills?.byAddress.has("x")).toBe(true); + expect(registered.toolIndex.has("load_skill")).toBe(true); + expect(registered.toolIndex.has("read_skill_file")).toBe(true); + }); + + test("an agent with no visible skills gets no skill tools", async () => { + const plugin = new AgentsPlugin({}); + // biome-ignore lint/suspicious/noExplicitAny: call private with a stub adapter + const registered = await (plugin as any).buildRegisteredAgent( + "bare", + { instructions: "hi", model: { run: async function* () {} } }, + { origin: "code" }, + ); + expect(registered.skills).toBeUndefined(); + expect(registered.toolIndex.has("load_skill")).toBe(false); + }); +}); + +describe("clientConfig — skills", () => { + test("exposes each agent's skill catalog keyed by agent name", () => { + const plugin = new AgentsPlugin({}); + // biome-ignore lint/suspicious/noExplicitAny: seed private registry + (plugin as any).agents = new Map([ + [ + "helper", + registeredWith(catalogOf(skill("pdf", { description: "PDFs" }))), + ], + ]); + // biome-ignore lint/suspicious/noExplicitAny: seed private field + (plugin as any).defaultAgentName = "helper"; + + const cfg = plugin.clientConfig(); + expect(cfg.agents).toEqual(["helper"]); + expect(cfg.skills).toEqual({ + helper: [{ name: "pdf", description: "PDFs" }], + }); + }); + + test("omits agents that have no visible catalog", () => { + const plugin = new AgentsPlugin({}); + // biome-ignore lint/suspicious/noExplicitAny: seed private registry + (plugin as any).agents = new Map([["bare", registeredWith()]]); + expect(plugin.clientConfig().skills).toEqual({}); + }); +}); + +describe("renderForcedSkill", () => { + test("renders the resolved skill body with a request note", () => { + const plugin = new AgentsPlugin({}); + const registered = registeredWith( + catalogOf(skill("pdf", { body: "PDF steps." })), + ); + // biome-ignore lint/suspicious/noExplicitAny: call private + const out = (plugin as any).renderForcedSkill(registered, "pdf"); + expect(out).toContain("PDF steps."); + expect(out).toContain("explicitly requested"); + }); + + test("returns null for an unknown forced skill", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const plugin = new AgentsPlugin({}); + const registered = registeredWith(catalogOf(skill("pdf"))); + // biome-ignore lint/suspicious/noExplicitAny: call private + expect((plugin as any).renderForcedSkill(registered, "ghost")).toBeNull(); + expect(warn).toHaveBeenCalled(); + warn.mockRestore(); + }); + + test("returns null when the agent has no catalog", () => { + const plugin = new AgentsPlugin({}); + // biome-ignore lint/suspicious/noExplicitAny: call private + expect( + (plugin as any).renderForcedSkill(registeredWith(), "pdf"), + ).toBeNull(); + }); +}); diff --git a/packages/appkit/src/plugins/agents/tests/skill-volume.test.ts b/packages/appkit/src/plugins/agents/tests/skill-volume.test.ts new file mode 100644 index 000000000..5e33bf168 --- /dev/null +++ b/packages/appkit/src/plugins/agents/tests/skill-volume.test.ts @@ -0,0 +1,154 @@ +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; + +import { resolveSkillCatalog } from "../../../core/agent/skills"; + +/** + * Phase 3 — catalog (UC Volume) skill source. The workspace client and the + * files connector are mocked so the test never touches Databricks: the mock + * connector serves a synthetic `/pdf/SKILL.md` + `reference.md`. + */ + +const h = vi.hoisted(() => ({ + list: vi.fn(), + read: vi.fn(), + client: { marker: "sp-client" } as unknown, +})); + +vi.mock("../../../context", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, getWorkspaceClient: () => h.client }; +}); + +vi.mock("../../../connectors/files", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + FilesConnector: class { + constructor(public config: { defaultVolume?: string }) {} + list(client: unknown, dir?: string) { + return h.list(client, dir); + } + read(client: unknown, filePath: string) { + return h.read(client, filePath); + } + }, + }; +}); + +// Imported after the mocks are registered. +const { AgentsPlugin } = await import("../agents"); + +const VOL = "/Volumes/cat/schema/skills"; + +beforeEach(() => { + process.env.DATABRICKS_VOLUME_AGENT_SKILLS = undefined; + delete process.env.DATABRICKS_VOLUME_AGENT_SKILLS; + + h.list.mockImplementation(async (_client: unknown, dir?: string) => { + if (dir === VOL) { + return [{ name: "pdf", is_directory: true, path: `${VOL}/pdf` }]; + } + if (dir === `${VOL}/pdf`) { + return [ + { name: "SKILL.md", is_directory: false, path: `${VOL}/pdf/SKILL.md` }, + { + name: "reference.md", + is_directory: false, + path: `${VOL}/pdf/reference.md`, + }, + ]; + } + return []; + }); + h.read.mockImplementation(async (_client: unknown, filePath: string) => { + if (filePath === `${VOL}/pdf/SKILL.md`) { + return "---\nname: pdf\ndescription: Work with PDFs\n---\nPDF body."; + } + if (filePath === `${VOL}/pdf/reference.md`) { + return "the reference"; + } + throw new Error(`unexpected read: ${filePath}`); + }); +}); + +afterEach(() => { + vi.clearAllMocks(); +}); + +describe("loadVolumeSkills", () => { + test("discovers, parses, and manifests a volume skill (SP identity)", async () => { + const plugin = new AgentsPlugin({ dir: false, skillsVolume: VOL }); + // biome-ignore lint/suspicious/noExplicitAny: call private + const skills = await (plugin as any).loadVolumeSkills(); + + expect(skills).toHaveLength(1); + expect(skills[0]).toMatchObject({ + name: "pdf", + description: "Work with PDFs", + body: "PDF body.", + source: "volume", + dir: `${VOL}/pdf`, + files: ["reference.md"], + }); + // Read as the SP client the mock hands back. + expect(h.read).toHaveBeenCalledWith(h.client, `${VOL}/pdf/SKILL.md`); + }); + + test("returns [] when no volume is configured", async () => { + const plugin = new AgentsPlugin({ dir: false }); + // biome-ignore lint/suspicious/noExplicitAny: call private + const skills = await (plugin as any).loadVolumeSkills(); + expect(skills).toEqual([]); + expect(h.list).not.toHaveBeenCalled(); + }); +}); + +describe("catalog resolution merges volume skills", () => { + test("a code agent opts into a volume skill via skills:", async () => { + const plugin = new AgentsPlugin({ dir: false, skillsVolume: VOL }); + // biome-ignore lint/suspicious/noExplicitAny: seed the global pool + call private + (plugin as any).globalSkills = await (plugin as any).loadVolumeSkills(); + // biome-ignore lint/suspicious/noExplicitAny: call private + const catalog = await (plugin as any).resolveAgentSkills( + "helper", + { instructions: "hi", skills: ["pdf"] }, + { origin: "code" }, + ); + expect(catalog?.byAddress.has("pdf")).toBe(true); + }); +}); + +describe("read_skill_file reads a volume resource", () => { + test("reads the file through the connector under SP identity", async () => { + const plugin = new AgentsPlugin({ dir: false, skillsVolume: VOL }); + // biome-ignore lint/suspicious/noExplicitAny: call private + const skills = await (plugin as any).loadVolumeSkills(); + const catalog = resolveSkillCatalog({ + agentName: "a", + perAgentSkills: [], + globalSkills: skills, + autoInherit: true, + }); + const entry = { + source: "skill" as const, + builtin: "read_skill_file" as const, + catalog, + def: { + name: "read_skill_file", + description: "read", + parameters: { type: "object" }, + annotations: { effect: "read" as const }, + }, + }; + + // biome-ignore lint/suspicious/noExplicitAny: call private + const result = await (plugin as any).dispatchSkillTool(entry, { + skill: "pdf", + path: "reference.md", + }); + + expect(result).toBe("the reference"); + expect(h.read).toHaveBeenCalledWith(h.client, `${VOL}/pdf/reference.md`); + }); +}); From fa045a00dbd90bb03f4217c7d60176730e9b07df Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Fri, 21 Aug 2026 13:02:22 +0200 Subject: [PATCH 2/5] chore(appkit): sync template manifest with agents skills volume resource template/appkit.plugins.json is generated from the plugin manifests; it must travel with the agents manifest.json change (skills volume resource) or CI's sync:template check fails. Was mis-bucketed into the 2/2 fixtures PR. Signed-off-by: MarioCadenas --- template/appkit.plugins.json | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/template/appkit.plugins.json b/template/appkit.plugins.json index 503ee7983..85fcbb8f2 100644 --- a/template/appkit.plugins.json +++ b/template/appkit.plugins.json @@ -37,6 +37,25 @@ "origin": "user" } } + }, + { + "type": "volume", + "alias": "Agent skills", + "resourceKey": "agents-skills", + "description": "Optional Unity Catalog Volume providing catalog-sourced agent skills (read-only)", + "permission": "READ_VOLUME", + "fields": { + "path": { + "env": "DATABRICKS_VOLUME_AGENT_SKILLS", + "description": "Volume path for agent skills (e.g. /Volumes/catalog/schema/skills)", + "discovery": { + "type": "kind", + "resourceKind": "volume", + "select": "full_name" + }, + "origin": "user" + } + } } ] }, From 1b9857a0953da8ac19282aa86693c3d70a9f1fd0 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Fri, 21 Aug 2026 15:47:57 +0200 Subject: [PATCH 3/5] refactor(appkit): tidy skill loading + drop dead biome-ignore comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on #543: - parallelize UC-volume skill reads (network-bound) via Promise.all; per-skill failures still skip individually and sorted order is preserved - extract the useAgentChat slash-command parse into a named resolveSkill helper (appkit-ui) - delete 18 stale biome-ignore comments — post-oxlint migration, and no-explicit-any is off in .oxlintrc.json, so they suppressed nothing Signed-off-by: MarioCadenas --- .../src/react/hooks/use-agent-chat.ts | 28 ++++---- packages/appkit/src/plugins/agents/agents.ts | 67 ++++++++++--------- .../agents/tests/dispatch-tool-call.test.ts | 2 - .../plugins/agents/tests/skill-client.test.ts | 10 --- .../plugins/agents/tests/skill-volume.test.ts | 6 -- 5 files changed, 51 insertions(+), 62 deletions(-) diff --git a/packages/appkit-ui/src/react/hooks/use-agent-chat.ts b/packages/appkit-ui/src/react/hooks/use-agent-chat.ts index 7107cb362..5817eb5a5 100644 --- a/packages/appkit-ui/src/react/hooks/use-agent-chat.ts +++ b/packages/appkit-ui/src/react/hooks/use-agent-chat.ts @@ -100,6 +100,20 @@ export interface UseAgentChatResult { reset: () => void; } +/** `forced` (opts.skill) wins; otherwise parse a leading `/skill-name` token off the message. */ +function resolveSkill( + message: string, + forced?: string, +): { skill?: string; text: string } { + if (forced) return { skill: forced, text: message }; + const match = message.match(/^\/([A-Za-z0-9][\w.:-]*)\s*/); + if (!match) return { text: message }; + const skill = match[1]; + const rest = message.slice(match[0].length); + // When the message is only the token, fall back to a minimal instruction. + return { skill, text: rest.trim() === "" ? `Use the ${skill} skill.` : rest }; +} + /** * React hook for chatting with an agent registered via the `agents()` * plugin. Wraps {@link connectSSE} (which owns the buffer cap, abort @@ -175,19 +189,7 @@ export function useAgentChat({ setError(null); setIsStreaming(true); - // Resolve the forced skill: explicit opts.skill wins; otherwise parse a - // leading `/skill-name` token off the message. When the message is only - // the token, fall back to a minimal instruction so the turn isn't empty. - let text = message; - let skill = opts?.skill; - if (!skill) { - const match = text.match(/^\/([A-Za-z0-9][\w.:-]*)\s*/); - if (match) { - skill = match[1]; - text = text.slice(match[0].length); - if (text.trim() === "") text = `Use the ${skill} skill.`; - } - } + const { skill, text } = resolveSkill(message, opts?.skill); const payload = { message: text, diff --git a/packages/appkit/src/plugins/agents/agents.ts b/packages/appkit/src/plugins/agents/agents.ts index 1a320528c..39acdaf2b 100644 --- a/packages/appkit/src/plugins/agents/agents.ts +++ b/packages/appkit/src/plugins/agents/agents.ts @@ -736,37 +736,42 @@ export class AgentsPlugin extends Plugin implements ToolProvider { return []; } - const skills: SkillDefinition[] = []; - for (const entry of entries) { - if (!entry.is_directory || !entry.name || !entry.path) continue; - const skillDir = entry.path; - const skillFile = `${skillDir}/SKILL.md`; - try { - const raw = await connector.read(client, skillFile); - const parsed = parseSkill(raw, skillFile); - const files = await this.listVolumeSkillFiles( - connector, - client, - skillDir, - ); - skills.push({ - name: parsed.name, - description: parsed.description, - body: parsed.body, - source: "volume", - dir: skillDir, - files, - allowedTools: parsed.allowedTools, - }); - } catch (err) { - logger.warn( - "Skipping catalog skill '%s': %s", - skillDir, - err instanceof Error ? err.message : String(err), - ); - } - } - return skills; + // Read every skill concurrently — each is a network round-trip to the + // volume, so serial reads would add up. A per-skill failure skips only + // that skill; Promise.all preserves the (sorted) entry order. + const loaded = await Promise.all( + entries.map(async (entry): Promise => { + if (!entry.is_directory || !entry.name || !entry.path) return null; + const skillDir = entry.path; + const skillFile = `${skillDir}/SKILL.md`; + try { + const raw = await connector.read(client, skillFile); + const parsed = parseSkill(raw, skillFile); + const files = await this.listVolumeSkillFiles( + connector, + client, + skillDir, + ); + return { + name: parsed.name, + description: parsed.description, + body: parsed.body, + source: "volume", + dir: skillDir, + files, + allowedTools: parsed.allowedTools, + }; + } catch (err) { + logger.warn( + "Skipping catalog skill '%s': %s", + skillDir, + err instanceof Error ? err.message : String(err), + ); + return null; + } + }), + ); + return loaded.filter((s): s is SkillDefinition => s !== null); } /** Lists a volume skill's resource files (one level, excluding SKILL.md). */ diff --git a/packages/appkit/src/plugins/agents/tests/dispatch-tool-call.test.ts b/packages/appkit/src/plugins/agents/tests/dispatch-tool-call.test.ts index d503fea6f..0cf596ef3 100644 --- a/packages/appkit/src/plugins/agents/tests/dispatch-tool-call.test.ts +++ b/packages/appkit/src/plugins/agents/tests/dispatch-tool-call.test.ts @@ -533,7 +533,6 @@ describe("dispatchToolCall — skill built-ins", () => { files: ["reference.md"], }, ]); - // biome-ignore lint/suspicious/noExplicitAny: stub gate to assert it never fires (plugin as any).approvalGate.wait = vi.fn(); const result = await callDispatch(plugin, { @@ -545,7 +544,6 @@ describe("dispatchToolCall — skill built-ins", () => { expect(String(result)).toContain("Detailed PDF steps."); expect(String(result)).toContain("reference.md"); - // biome-ignore lint/suspicious/noExplicitAny: assertion on stub expect((plugin as any).approvalGate.wait).not.toHaveBeenCalled(); }); diff --git a/packages/appkit/src/plugins/agents/tests/skill-client.test.ts b/packages/appkit/src/plugins/agents/tests/skill-client.test.ts index 988e4f987..5d3826e1d 100644 --- a/packages/appkit/src/plugins/agents/tests/skill-client.test.ts +++ b/packages/appkit/src/plugins/agents/tests/skill-client.test.ts @@ -35,7 +35,6 @@ function catalogOf(...skills: SkillDefinition[]) { }); } -// biome-ignore lint/suspicious/noExplicitAny: minimal RegisteredAgent stub function registeredWith(catalog?: ReturnType): any { return { name: "a", @@ -52,10 +51,8 @@ describe("skills are wired uniformly for every registered agent", () => { // catalog + load_skill/read_skill_file tools as a top-level agent. test("buildRegisteredAgent gives a code agent its catalog and skill tools", async () => { const plugin = new AgentsPlugin({}); - // biome-ignore lint/suspicious/noExplicitAny: seed the shared pool (plugin as any).globalSkills = [skill("x", { description: "X skill" })]; - // biome-ignore lint/suspicious/noExplicitAny: call private with a stub adapter const registered = await (plugin as any).buildRegisteredAgent( "child", { @@ -77,7 +74,6 @@ describe("skills are wired uniformly for every registered agent", () => { test("an agent with no visible skills gets no skill tools", async () => { const plugin = new AgentsPlugin({}); - // biome-ignore lint/suspicious/noExplicitAny: call private with a stub adapter const registered = await (plugin as any).buildRegisteredAgent( "bare", { instructions: "hi", model: { run: async function* () {} } }, @@ -91,14 +87,12 @@ describe("skills are wired uniformly for every registered agent", () => { describe("clientConfig — skills", () => { test("exposes each agent's skill catalog keyed by agent name", () => { const plugin = new AgentsPlugin({}); - // biome-ignore lint/suspicious/noExplicitAny: seed private registry (plugin as any).agents = new Map([ [ "helper", registeredWith(catalogOf(skill("pdf", { description: "PDFs" }))), ], ]); - // biome-ignore lint/suspicious/noExplicitAny: seed private field (plugin as any).defaultAgentName = "helper"; const cfg = plugin.clientConfig(); @@ -110,7 +104,6 @@ describe("clientConfig — skills", () => { test("omits agents that have no visible catalog", () => { const plugin = new AgentsPlugin({}); - // biome-ignore lint/suspicious/noExplicitAny: seed private registry (plugin as any).agents = new Map([["bare", registeredWith()]]); expect(plugin.clientConfig().skills).toEqual({}); }); @@ -122,7 +115,6 @@ describe("renderForcedSkill", () => { const registered = registeredWith( catalogOf(skill("pdf", { body: "PDF steps." })), ); - // biome-ignore lint/suspicious/noExplicitAny: call private const out = (plugin as any).renderForcedSkill(registered, "pdf"); expect(out).toContain("PDF steps."); expect(out).toContain("explicitly requested"); @@ -132,7 +124,6 @@ describe("renderForcedSkill", () => { const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); const plugin = new AgentsPlugin({}); const registered = registeredWith(catalogOf(skill("pdf"))); - // biome-ignore lint/suspicious/noExplicitAny: call private expect((plugin as any).renderForcedSkill(registered, "ghost")).toBeNull(); expect(warn).toHaveBeenCalled(); warn.mockRestore(); @@ -140,7 +131,6 @@ describe("renderForcedSkill", () => { test("returns null when the agent has no catalog", () => { const plugin = new AgentsPlugin({}); - // biome-ignore lint/suspicious/noExplicitAny: call private expect( (plugin as any).renderForcedSkill(registeredWith(), "pdf"), ).toBeNull(); diff --git a/packages/appkit/src/plugins/agents/tests/skill-volume.test.ts b/packages/appkit/src/plugins/agents/tests/skill-volume.test.ts index 5e33bf168..72f282806 100644 --- a/packages/appkit/src/plugins/agents/tests/skill-volume.test.ts +++ b/packages/appkit/src/plugins/agents/tests/skill-volume.test.ts @@ -79,7 +79,6 @@ afterEach(() => { describe("loadVolumeSkills", () => { test("discovers, parses, and manifests a volume skill (SP identity)", async () => { const plugin = new AgentsPlugin({ dir: false, skillsVolume: VOL }); - // biome-ignore lint/suspicious/noExplicitAny: call private const skills = await (plugin as any).loadVolumeSkills(); expect(skills).toHaveLength(1); @@ -97,7 +96,6 @@ describe("loadVolumeSkills", () => { test("returns [] when no volume is configured", async () => { const plugin = new AgentsPlugin({ dir: false }); - // biome-ignore lint/suspicious/noExplicitAny: call private const skills = await (plugin as any).loadVolumeSkills(); expect(skills).toEqual([]); expect(h.list).not.toHaveBeenCalled(); @@ -107,9 +105,7 @@ describe("loadVolumeSkills", () => { describe("catalog resolution merges volume skills", () => { test("a code agent opts into a volume skill via skills:", async () => { const plugin = new AgentsPlugin({ dir: false, skillsVolume: VOL }); - // biome-ignore lint/suspicious/noExplicitAny: seed the global pool + call private (plugin as any).globalSkills = await (plugin as any).loadVolumeSkills(); - // biome-ignore lint/suspicious/noExplicitAny: call private const catalog = await (plugin as any).resolveAgentSkills( "helper", { instructions: "hi", skills: ["pdf"] }, @@ -122,7 +118,6 @@ describe("catalog resolution merges volume skills", () => { describe("read_skill_file reads a volume resource", () => { test("reads the file through the connector under SP identity", async () => { const plugin = new AgentsPlugin({ dir: false, skillsVolume: VOL }); - // biome-ignore lint/suspicious/noExplicitAny: call private const skills = await (plugin as any).loadVolumeSkills(); const catalog = resolveSkillCatalog({ agentName: "a", @@ -142,7 +137,6 @@ describe("read_skill_file reads a volume resource", () => { }, }; - // biome-ignore lint/suspicious/noExplicitAny: call private const result = await (plugin as any).dispatchSkillTool(entry, { skill: "pdf", path: "reference.md", From 7f792098de6a61b17d6fd3f3cb17fc256bb02833 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Fri, 21 Aug 2026 12:53:07 +0200 Subject: [PATCH 4/5] test(playground): agent skills fixtures + template demo (2/2) Playground/template half of #532 (split 2/2), stacked on the SDK PR (1/2). Demo skills exercising every source/case: global bullet-brief (+ helper opt-in), per-agent query/skills/routing-brief, the agent:haiku/bundle:haiku collision, and the template tracer-bullets skill. Auto-retargets to main once 1/2 merges. Signed-off-by: MarioCadenas --- .../client/src/routes/agent.route.tsx | 102 +++++++++++++++++- .../server/agents/helper/agent.ts | 5 + .../server/agents/query/agent.md | 5 + .../server/agents/query/skills/haiku/SKILL.md | 10 ++ .../query/skills/routing-brief/SKILL.md | 17 +++ .../query/skills/routing-brief/reference.md | 8 ++ .../agents/skills/bullet-brief/SKILL.md | 12 +++ .../server/agents/skills/haiku/SKILL.md | 15 +++ .../server/agents/skills/haiku/reference.md | 15 +++ .../client/src/pages/agents/AgentChat.tsx | 28 ++++- template/server/agents/planner/agent.md | 6 +- .../agents/skills/tracer-bullets/SKILL.md | 29 +++++ .../agents/skills/tracer-bullets/reference.md | 28 +++++ 13 files changed, 275 insertions(+), 5 deletions(-) create mode 100644 apps/dev-playground/server/agents/query/skills/haiku/SKILL.md create mode 100644 apps/dev-playground/server/agents/query/skills/routing-brief/SKILL.md create mode 100644 apps/dev-playground/server/agents/query/skills/routing-brief/reference.md create mode 100644 apps/dev-playground/server/agents/skills/bullet-brief/SKILL.md create mode 100644 apps/dev-playground/server/agents/skills/haiku/SKILL.md create mode 100644 apps/dev-playground/server/agents/skills/haiku/reference.md create mode 100644 template/server/agents/skills/tracer-bullets/SKILL.md create mode 100644 template/server/agents/skills/tracer-bullets/reference.md diff --git a/apps/dev-playground/client/src/routes/agent.route.tsx b/apps/dev-playground/client/src/routes/agent.route.tsx index 2ae2a2557..95fb700d0 100644 --- a/apps/dev-playground/client/src/routes/agent.route.tsx +++ b/apps/dev-playground/client/src/routes/agent.route.tsx @@ -18,6 +18,7 @@ const AGENT_OPTIONS = [ { value: "helper", label: "Helper — general assistant" }, { value: "sql_analyst", label: "SQL Analyst — NYC taxi queries" }, { value: "supervisor", label: "Supervisor — Databricks-hosted tools" }, + { value: "query", label: "Query — skills dispatcher" }, ] as const; interface SSEEvent { @@ -151,7 +152,7 @@ function useAutocomplete(enabled: boolean) { function AgentRoute() { const [messages, setMessages] = useState([]); - const [events, setEvents] = useState([]); + const [events, setEvents] = useState([]); const [input, setInput] = useState(""); const [isLoading, setIsLoading] = useState(false); const [threadId, setThreadId] = useState(null); @@ -159,6 +160,8 @@ function AgentRoute() { const [pendingApprovals, setPendingApprovals] = useState( [], ); + // Highlighted row in the `/skill` menu. + const [skillIndex, setSkillIndex] = useState(0); const decideApproval = useCallback( async (approvalId: string, decision: "approve" | "deny") => { @@ -191,8 +194,11 @@ function AgentRoute() { const agentConfig = getPluginClientConfig<{ agents?: string[]; defaultAgent?: string; + skills?: Record; }>("agents"); const hasAutocomplete = (agentConfig.agents ?? []).includes("autocomplete"); + // Skills visible to the selected agent, from the boot config. + const activeSkills = agentConfig.skills?.[agent] ?? []; const { suggestion, @@ -201,6 +207,29 @@ function AgentRoute() { clear: clearSuggestion, } = useAutocomplete(hasAutocomplete); + // Slash-command menu: when the input is a leading `/token` (no space yet), + // surface matching skills for the active agent. + const slashQuery = input.match(/^\/([^\s]*)$/)?.[1] ?? null; + const skillMatches = + slashQuery !== null && activeSkills.length > 0 + ? activeSkills.filter((s) => + s.name.toLowerCase().includes(slashQuery.toLowerCase()), + ) + : []; + const skillMenuOpen = skillMatches.length > 0; + + const pickSkill = (name: string) => { + setInput(`/${name} `); + clearSuggestion(); + inputRef.current?.focus(); + }; + + // biome-ignore lint/correctness/useExhaustiveDependencies: reset highlight as the query changes + useEffect(() => { + setSkillIndex(0); + }, [input, agent]); + + // biome-ignore lint/correctness/useExhaustiveDependencies: scroll on new messages useEffect(() => { messagesEndRef.current?.scrollIntoView({ behavior: "smooth" }); }, [messages]); @@ -218,13 +247,25 @@ function AgentRoute() { setEvents([]); setIsLoading(true); + // `/skill-name …` forces a skill for this turn (the agents plugin injects + // its instructions); the model can still auto-load others via load_skill. + let messageBody = userMessage; + let skill: string | undefined; + const skillMatch = messageBody.match(/^\/([A-Za-z0-9][\w.:-]*)\s*/); + if (skillMatch) { + skill = skillMatch[1]; + messageBody = messageBody.slice(skillMatch[0].length); + if (messageBody.trim() === "") messageBody = `Use the ${skill} skill.`; + } + try { const response = await fetch("/api/agents/chat", { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ - message: userMessage, + message: messageBody, agent, + ...(skill && { skill }), ...(threadId && { threadId }), }), }); @@ -506,6 +547,34 @@ function AgentRoute() { value={input} onChange={(e) => handleInputChange(e.target.value)} onKeyDown={(e) => { + if (skillMenuOpen) { + if (e.key === "ArrowDown") { + e.preventDefault(); + setSkillIndex((i) => (i + 1) % skillMatches.length); + return; + } + if (e.key === "ArrowUp") { + e.preventDefault(); + setSkillIndex( + (i) => + (i - 1 + skillMatches.length) % + skillMatches.length, + ); + return; + } + if (e.key === "Enter" || e.key === "Tab") { + e.preventDefault(); + pickSkill( + (skillMatches[skillIndex] ?? skillMatches[0]).name, + ); + return; + } + if (e.key === "Escape") { + e.preventDefault(); + setInput(""); + return; + } + } if (e.key === "Tab" && suggestion) { e.preventDefault(); acceptSuggestion(); @@ -518,11 +587,38 @@ function AgentRoute() { sendMessage(); } }} - placeholder="Ask a question..." + placeholder={ + activeSkills.length > 0 + ? "Ask a question… (type / for skills)" + : "Ask a question..." + } disabled={isLoading} rows={1} className="w-full rounded-md border border-input bg-transparent px-3 py-2 text-sm ring-offset-background placeholder:text-muted-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring disabled:opacity-50 resize-none" /> + {skillMenuOpen && ( +
    + {skillMatches.map((s, i) => ( +
  • + +
  • + ))} +
+ )}