From e48d82b0ef4d24e30af7f485c98889a92aab3023 Mon Sep 17 00:00:00 2001 From: Eric Wang Date: Thu, 13 Aug 2026 11:35:34 -0700 Subject: [PATCH] =?UTF-8?q?M8:=20skills=20=E2=80=94=20SKILL.md=20discovery?= =?UTF-8?q?=20shared=20with=20dsh,=20index=20injection,=20on-demand=20load?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Discovery reads the same roots dsh reads (verified in its skill-filesystem package): /.dsc/skills, /.agents/skills, ~/.dsc/skills, ~/.agents/skills, precedence in that order on a name collision, project root = nearest .git — so one skills directory serves both harnesses with the same collision semantics. Skill identity is the frontmatter name, under dsh's own name rule. Progressive disclosure per DESIGN.md: the system prompt carries one index line per skill; the body enters context only when the model invokes the skill tool (static schema — no name enum, so adding a skill never rewrites the tool's prefix bytes). Zero skills = no tool registered = the gated default prompt, byte-identical, and the golden now proves that for an empty index explicitly. Frontmatter parsing is a zero-dep YAML subset; anything unparseable skips that one skill with a warning (the 126 real skills in this machine's ~/.agents/skills all parse, zero warnings). dsc skills lists what discovery found; /status gained a skills line. Measured (real API, flash, totals byte-stable across runs): the index costs ~149 tokens fixed + ~25 per skill — 1134 -> 1308 -> 1359 input tokens/turn at 0/1/3 skills. Verified live: flash invoked skill {"name":"release-notes"} and answered from the loaded body. MCP ships nothing here (Round 3 amendment stands; upstream deleted its plugin manifest 08-09 with no migration — holding is deliberate). Cache-impact: none by default — the default prompt is byte-unchanged (prompt-golden green). With skills discovered the prompt grows by the index only, frozen by a new golden; discovery runs once per process so the prefix is byte-stable across a session's runs. 133 tests. Co-Authored-By: Claude Fable 5 Co-authored-by: Cursor --- DESIGN.md | 22 ++- README.md | 23 ++- docs/devlog/2026-08-13-m8-skills.org | 111 +++++++++++++++ docs/devlog/README.md | 5 + package.json | 2 +- src/cli.ts | 46 +++++- src/engine/loop.ts | 7 +- src/engine/prompt.ts | 19 ++- src/skills/discover.ts | 178 +++++++++++++++++++++++ src/tools/skill.ts | 67 +++++++++ src/ui/repl.ts | 11 +- tests/prompt-golden.test.ts | 59 ++++++++ tests/repl-status.test.ts | 2 + tests/skills.test.ts | 204 +++++++++++++++++++++++++++ 14 files changed, 740 insertions(+), 16 deletions(-) create mode 100644 docs/devlog/2026-08-13-m8-skills.org create mode 100644 src/skills/discover.ts create mode 100644 src/tools/skill.ts create mode 100644 tests/skills.test.ts diff --git a/DESIGN.md b/DESIGN.md index cdb2714..1604e68 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -178,11 +178,21 @@ Small and stable — the schema is part of the cached prefix: ## Skills -- Discovery: `./.agents/skills/`, `~/.agents/skills/` (cross-client - convention), plus `./.dsc/skills/`, `~/.dsc/skills/`. +- Discovery (SHIPPED, devlog 2026-08-13-m8-skills.org): + `/.dsc/skills/`, `/.agents/skills/`, + `~/.dsc/skills/`, `~/.agents/skills/` — precedence in that order on a + name collision (project beats home, harness-native beats the shared + convention). That mirrors dsh's root ranks (its `.dsh/skills` at the + positions our `.dsc/skills` holds), so both harnesses resolve the + same collision the same way. Project root = nearest ancestor with + `.git`, else cwd — also the dsh rule. - SKILL.md with frontmatter; progressive disclosure (index line in prompt, body loaded on invoke) — skill bodies never sit in the stable prefix, only the one-line index does (prefix stays small AND stable). + Shipped as the `skill` tool: static schema (no name enum — names live + in the index, so adding a skill never rewrites the tool bytes), + registered only when discovery found something; zero skills = the + gated default prompt, byte-identical. - v2: skill versioning + per-skill eval records (the "skills with evidence" idea from the research docs). @@ -300,7 +310,13 @@ done right). appends a request event, only the daemon fires. The read preset carries a ported classifyBash (docs/research/deepseek-pi-sibling.md) so "read-only scheduled run" is enforced, not advisory prose. -7. Skills + MCP client. +7. Skills + MCP client. Skills SHIPPED (as M8 in the devlog numbering — + /seek took the M6 slot and the scheduler M7, so this item kept its + build-order number but not its milestone number; see + docs/devlog/2026-08-13-m8-skills.org). Discovery reads the same + roots dsh reads: one skills directory, two harnesses. MCP remains + OUT of core per the Round 3 amendment below; nothing shipped here + changes that. Eval harness (EVAL.md, separate task) develops in parallel from milestone 1 — it gates every milestone exit. diff --git a/README.md b/README.md index 53abc0f..ac0bfa8 100644 --- a/README.md +++ b/README.md @@ -13,8 +13,9 @@ instead of a few hundred thousand. **Status: work in progress.** The agent loop, tools, context engine, sessions/compaction, sub-agents, and interactive mode work and are -eval-gated; the job scheduler works and is test-gated (the default -prompt is byte-unchanged by it). Interfaces will move. +eval-gated; the job scheduler and SKILL.md skills work and are +test-gated (the default prompt is byte-unchanged by both). Interfaces +will move. ## dsc and dsh @@ -43,9 +44,9 @@ dsc's lane is narrower and stays that way: behavior in an afternoon and measure the difference. Interop note: dsh discovers SKILL.md skills in `~/.agents/skills` and -`/.agents/skills`; dsc's planned skills support (milestone 7 -in the build order) targets the same layout, so one skills directory -should serve both. Planned, not shipped. +`/.agents/skills`; dsc reads the same layout with the same +precedence rules (shipped — see Skills below), so one skills directory +serves both harnesses. Short version: use dsh for the official, full-featured harness. Use dsc when you want a minimal auditable agent, wire-level experiments, @@ -138,6 +139,18 @@ against `max_tokens`, which dsc's budgets already account for. preset is read-only, with bash held to an inspection allowlist; the documented safety model for write jobs is the decision-file pattern. See `docs/scheduler.md`. +- **Skills** (`src/skills/`, `src/tools/skill.ts`) — SKILL.md discovery + from the same directories dsh reads: `/.dsc/skills`, + `/.agents/skills`, `~/.dsc/skills`, `~/.agents/skills`, in + that precedence on a name collision (project beats home, + harness-native beats shared — dsh's rank order, so both harnesses + resolve a collision identically). One skills directory serves both + harnesses. Progressive disclosure: the system prompt carries one + index line per skill; the body enters context only when the model + invokes the `skill` tool. With nothing discovered, no tool registers + and the default prompt is byte-unchanged. Broken frontmatter skips + that skill with a warning, never a crash. `dsc skills` lists what + discovery found and from where. - **Eval harness** (`eval/`) — the part that decides what stays. ## Everything here is eval-gated diff --git a/docs/devlog/2026-08-13-m8-skills.org b/docs/devlog/2026-08-13-m8-skills.org new file mode 100644 index 0000000..ab8bd05 --- /dev/null +++ b/docs/devlog/2026-08-13-m8-skills.org @@ -0,0 +1,111 @@ +#+TITLE: M8 skills — SKILL.md discovery shared with dsh: one directory, two harnesses +#+DATE: 2026-08-13 + +* Tension + +Skills were already the next milestone (build-order #7; the devlog +numbering drifted to M8 because /seek took M6 and the scheduler M7). +Then DeepSeek shipped dsh on launch day, and dsh reads SKILL.md skills +from =~/.agents/skills= and =/.agents/skills=. That upgraded a +planned feature into an interop story: if dsc reads the same roots with +the same precedence, one skills directory serves both harnesses, on the +day the official harness launched. + +The constraint that shaped everything: the default system prompt is +byte-frozen (prompt-golden) and every one of its bytes went through the +eval gate. Skills had to be able to appear WITHOUT costing anything +when absent, and without ever putting a skill body into the stable +prefix. + +* What shipped + +- src/skills/discover.ts — discovery across four roots, in precedence + order: =/.dsc/skills=, =/.agents/skills=, + =~/.dsc/skills=, =~/.agents/skills=. First name wins a collision + (project beats home, harness-native beats shared); the loser gets a + "shadowed" warning, not silence. This mirrors dsh's root ranks (its + =.dsh/skills= sits where our =.dsc/skills= does — verified in dsh's + skill-filesystem package), so the two harnesses resolve the same + collision the same way. Project root = nearest ancestor with =.git=, + else cwd — also the dsh rule, so a repo subdirectory sees the same + skills in both harnesses. Skill identity is the frontmatter =name= + (dsh's rule, including its =^[a-z0-9]+(-[a-z0-9]+)*$= name check), + not the directory name. +- Frontmatter parsing is a deliberate YAML subset, zero deps: top-level + =key: value= scalars, quoted values, =|=/=>= blocks (folded to one + line — the index renders one line per skill), nested structures + tolerated and ignored. Anything unparseable skips THAT skill with a + warning; a broken SKILL.md must never take down the session. Run + against the 126 real skills in this machine's =~/.agents/skills=: + all 126 parsed, zero warnings. +- src/tools/skill.ts — progressive disclosure. The system prompt + carries one index line per skill (name + description); the body + enters context only when the model invokes =skill= with a name. The + tool schema is static (no name enum): names live in the index, so + adding a skill never rewrites the tool's prefix bytes. Zero skills = + no tool registered = the gated default prompt, byte-identical + (same rule as the task tool, and the golden test now proves it for + an empty index explicitly). +- Discovery runs ONCE per process, so the prompt is byte-stable across + every run of a session — the epoch rule (never re-render + mid-session) satisfied by construction rather than by a + re-render-and-diff mechanism we did not build. +- =dsc skills= — what discovery found, its source root, and every + warning. The listing is exactly what the session index will carry. + /status gained a =skills= line. + +* Measured: what the index costs (real API, flash) + +Same one-shot prompt, clean home, project-only skills, usage totals +from the provider (fresh + cache_read; totals were byte-stable across +repeat runs): + +| skills | input tokens/turn | delta | +|--------+-------------------+-------| +| 0 | 1,134 | — | +| 1 | 1,308 | +174 | +| 3 | 1,359 | +225 | + +So the surface costs ~149 tokens fixed (skill tool schema + index line ++ one guideline + section header) and ~25 tokens per skill at +realistic description lengths (~15–20 words). A 40-skill index would +add ~1,100 tokens to every turn's prefix — cheap while cached, but not +free; description length is the lever users control. + +* Verified live (real API) + +With three skills discovered, flash was asked to use one: it invoked +=skill {"name": "release-notes"}=, got the body, and answered from it +correctly. One honest wrinkle: the tool's output names the SKILL.md +path (so skills can reference sibling files), and the model then +re-read the same file with =read= before answering — a paid extra turn +to confirm what it already had. Not changed for now: the path is what +makes bundled-resource skills possible, and one wasted turn on a probe +is observation, not measurement. + +* Deviations from DESIGN.md and the milestone spec + +- Build order #7 said "Skills + MCP client". MCP shipped nothing and + will ship nothing here: the Round 3 amendment already moved it out + of core, and dsh's launch adds a second reason to hold (their plugin + manifest format was deleted upstream 08-09 with no migration). + Recorded in DESIGN.md against the build-order entry. +- dsh also accepts flat =/skills/.md= files (no directory). + dsc reads only the directory-bundle layout DESIGN specified — + =/SKILL.md= — because that is the shared convention worth + interop. If flat-file skills show up in the wild, three lines add + them. +- Scheduler jobs and sub-agent children do not get skills. Children + share one stable prefix per toolset by design, and job prompts are + their own gated surface; wiring skills into either is a decision for + a user who needs it, not a default. + +* Open + +- Skill bodies are capped by truncateHead (50 KB) rather than spilled; + a skill that long is probably wrong, but the spill path exists if + one ever earns it. +- Mid-session skill changes are invisible until the next process + start (deliberate: epoch rule). If it ever matters, the epoch + boundary is where the refresh belongs — never prompt mutation. +- Per-skill eval records ("skills with evidence") stay the v2 idea. diff --git a/docs/devlog/README.md b/docs/devlog/README.md index 9481fbd..d643195 100644 --- a/docs/devlog/README.md +++ b/docs/devlog/README.md @@ -28,6 +28,11 @@ Entries present here: add and fire, the two-file ledger, the read-only preset built on a classifier ported from the Go sibling (with the holes found porting it), and why serve ships without HTTP. +- `2026-08-13-m8-skills.org` — SKILL.md skills, shipped the day dsh + launched and reading the same directories it reads: one skills dir, + two harnesses. The measured cost of the index (~149 tokens fixed, + ~25 per skill), the byte-frozen-prompt constraint that shaped the + design, and why MCP still shipped nothing. Entries for M1-M3 are held back for now: they are written around a private comparison harness (a proprietary agent CLI retargeted to diff --git a/package.json b/package.json index f87fa6c..aa8b988 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "dsc", - "version": "0.6.0", + "version": "0.7.0", "private": false, "type": "module", "description": "DeepSeek-native coding agent for the v4 flash/pro series. TypeScript/Bun, zero runtime dependencies.", diff --git a/src/cli.ts b/src/cli.ts index a7bd1c7..9f0c19e 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -34,11 +34,13 @@ import type { RunResult } from "./engine/loop"; import { SubagentManager } from "./engine/subagent"; import { Session } from "./session/session"; import { SessionStore } from "./session/store"; +import { discoverSkills } from "./skills/discover"; import { readTool } from "./tools/read"; import { bashTool } from "./tools/bash"; import { editTool } from "./tools/edit"; import { writeTool } from "./tools/write"; import { makeTaskTool } from "./tools/task"; +import { makeSkillTool } from "./tools/skill"; import { runRepl } from "./ui/repl"; function argValue(name: string): string | undefined { @@ -53,6 +55,7 @@ const USAGE = `usage: dsc interactive session in the current directory dsc -p "prompt" one-shot, print the result and exit dsc job|ps|serve scheduled jobs (dsc job for details) + dsc skills discovered SKILL.md skills and their sources --model NAME ${Object.keys(MODELS).join(" | ")} --cwd DIR working directory for the run @@ -78,6 +81,26 @@ if (["job", "ps", "serve"].includes(process.argv[2] ?? "")) { process.exit(await schedulerCli(process.argv.slice(2))); } +// `dsc skills` — what discovery found and from where, no API key needed. +// The listing shows exactly what the session index will carry; a skipped +// or shadowed SKILL.md is reported here rather than silently absent. +if (process.argv[2] === "skills") { + const dir = resolve(argValue("cwd") ?? process.cwd()); + const found = discoverSkills(dir); + for (const w of found.warnings) console.error(`warning: ${w}`); + if (found.skills.length === 0) { + console.log( + "no skills (searched /.dsc/skills, /.agents/skills, ~/.dsc/skills, ~/.agents/skills)", + ); + } + for (const s of found.skills) { + console.log(`${s.name} [${s.source}]`); + console.log(` ${s.description}`); + console.log(` ${s.path}`); + } + process.exit(0); +} + const promptIdx = process.argv.indexOf("-p"); const prompt = promptIdx >= 0 ? process.argv[promptIdx + 1] : undefined; const interactive = prompt === undefined; @@ -116,6 +139,13 @@ if (!apiKey) { process.exit(2); } +// Skills are discovered ONCE per process, so the system prompt is +// byte-stable across every run of a session (the epoch rule: the index +// never re-renders mid-session). With nothing discovered, no skill tool +// is registered and the prompt stays byte-identical to the gated golden. +const skillset = discoverSkills(cwd); +for (const w of skillset.warnings) console.error(`dsc: skills: ${w}`); + const t0 = Date.now(); const baseUrl = process.env.DSC_BASE_URL ?? DEFAULT_BASE_URL; @@ -184,10 +214,16 @@ const makeManager = (): SubagentManager => { // The task TOOL stays opt-in (it changes the prompt); /seek drives the // same machinery from the frontend, so it needs no tool and no re-gate. -const makeTools = (mgr: SubagentManager) => - subagents - ? [readTool, bashTool, editTool, writeTool, makeTaskTool(mgr)] - : [readTool, bashTool, editTool, writeTool]; +// The skill tool registers only when discovery found something — same +// rule, same reason: an inert tool would be dead prefix weight. +const makeTools = (mgr: SubagentManager) => [ + readTool, + bashTool, + editTool, + writeTool, + ...(subagents ? [makeTaskTool(mgr)] : []), + ...(skillset.skills.length > 0 ? [makeSkillTool(skillset.skills)] : []), +]; if (interactive) { const code = await runRepl({ @@ -195,6 +231,7 @@ if (interactive) { session: session!, makeManager, makeTools, + skills: skillset.skills, model, cwd, apiKey, @@ -216,6 +253,7 @@ const runOpts = { apiKey, baseUrl, tools: makeTools(manager), + skills: skillset.skills, maxTurns, contextBudget, onEvent: verbose diff --git a/src/engine/loop.ts b/src/engine/loop.ts index b4fc68b..ab13666 100644 --- a/src/engine/loop.ts +++ b/src/engine/loop.ts @@ -20,6 +20,7 @@ import { streamMessage } from "../provider/client"; import type { ToolContext, ToolDefinition } from "../tools/index"; import { validateInput } from "../tools/index"; import { buildSystemPrompt, toWireTools } from "./prompt"; +import type { SkillIndexEntry } from "./prompt"; import { ContextMeter, estimateTokens } from "./context"; import { compactedView, @@ -47,6 +48,10 @@ export type RunOptions = { apiKey: string; baseUrl: string; tools: ToolDefinition[]; + /** Discovered skill index, rendered into the system prompt. Discovery + * happens once per process, so the prompt stays byte-stable across the + * runs of a session (epoch rule: never re-rendered mid-session). */ + skills?: SkillIndexEntry[]; maxTurns?: number; maxTokens?: number; signal?: AbortSignal; @@ -139,7 +144,7 @@ export class TurnSeam { export async function runLoop(opts: RunOptions): Promise { const emit: EventSink = opts.onEvent ?? (() => {}); const maxTurns = opts.maxTurns ?? 100; - const system = buildSystemPrompt(opts.tools, opts.cwd); + const system = buildSystemPrompt(opts.tools, opts.cwd, opts.skills); const wireTools = toWireTools(opts.tools); const toolCtx: ToolContext = { cwd: opts.cwd, diff --git a/src/engine/prompt.ts b/src/engine/prompt.ts index 151b6fa..20fc817 100644 --- a/src/engine/prompt.ts +++ b/src/engine/prompt.ts @@ -22,7 +22,15 @@ const GENERIC_GUIDELINES = [ "Your final text message is the deliverable. Keep it brief. If the task asks a question, end with exactly the answer requested.", ]; -export function buildSystemPrompt(tools: ToolDefinition[], cwd: string): string { +/** One index line per discovered skill (DESIGN.md "Skills": the index is + * the only skill surface in the stable prefix; bodies load on invoke). */ +export type SkillIndexEntry = { name: string; description: string }; + +export function buildSystemPrompt( + tools: ToolDefinition[], + cwd: string, + skills: SkillIndexEntry[] = [], +): string { const toolLines = tools.map( (t) => `- ${t.name}: ${t.promptSnippet ?? firstSentence(t.description)}`, ); @@ -38,6 +46,15 @@ export function buildSystemPrompt(tools: ToolDefinition[], cwd: string): string "", "Guidelines:", ...guidelines.map((g) => `- ${g}`), + // With no skills discovered this section is absent and the prompt is + // byte-identical to the gated golden — same rule as the task tool. + ...(skills.length > 0 + ? [ + "", + "Skills (reusable instructions; load the full text with the skill tool):", + ...skills.map((s) => `- ${s.name}: ${s.description}`), + ] + : []), "", `Current working directory: ${cwd}`, ].join("\n"); diff --git a/src/skills/discover.ts b/src/skills/discover.ts new file mode 100644 index 0000000..4ba9904 --- /dev/null +++ b/src/skills/discover.ts @@ -0,0 +1,178 @@ +// SKILL.md discovery (DESIGN.md "Skills", milestone 8). A skill is a +// directory holding a SKILL.md: YAML frontmatter with `name` and +// `description`, body = instructions. Discovery reads the SAME roots dsh +// reads (verified against dsh's skill-filesystem package at launch), so +// one skills directory serves both harnesses: +// +// /.dsc/skills (dsh reads .dsh/skills at the same rank) +// /.agents/skills (cross-harness convention) +// ~/.dsc/skills +// ~/.agents/skills +// +// Precedence on a name collision is that order — project beats home, +// harness-native beats shared — matching dsh's root ranks, so the two +// harnesses resolve a collision the same way. The project root is found +// by walking up from cwd to the nearest .git (else cwd itself), also the +// dsh rule; a skills dir in a repo works from any subdirectory. +// +// Only name + description are read at discovery (they go into the system +// prompt index); the body is loaded on demand by the skill tool. A file +// that cannot be parsed — malformed frontmatter, missing fields, a name +// dsh would also reject — is skipped with a warning, never a crash: one +// broken skill must not take down every session in the directory. + +import { existsSync, readdirSync, readFileSync, statSync } from "node:fs"; +import { homedir } from "node:os"; +import { dirname, join, resolve } from "node:path"; + +export type SkillSource = "project-dsc" | "project-agents" | "user-dsc" | "user-agents"; + +export type Skill = { + /** Frontmatter `name` — the identity the model invokes, per dsh. */ + name: string; + description: string; + /** Path to the SKILL.md file (its directory is the resource base). */ + path: string; + source: SkillSource; +}; + +export type SkillDiscovery = { skills: Skill[]; warnings: string[] }; + +/** Same rule dsh enforces; a name failing it is invisible to both harnesses. */ +const SKILL_NAME = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; + +export function discoverSkills(cwd: string, home = homedir()): SkillDiscovery { + const project = findProjectRoot(resolve(cwd)); + const roots: { dir: string; source: SkillSource }[] = [ + { dir: join(project, ".dsc", "skills"), source: "project-dsc" }, + { dir: join(project, ".agents", "skills"), source: "project-agents" }, + { dir: join(home, ".dsc", "skills"), source: "user-dsc" }, + { dir: join(home, ".agents", "skills"), source: "user-agents" }, + ]; + const seen = new Map(); + const warnings: string[] = []; + for (const root of roots) { + for (const entry of listSkillDirs(root.dir)) { + const path = join(root.dir, entry, "SKILL.md"); + if (!existsSync(path)) continue; // a dir without SKILL.md is not a skill + const parsed = parseSkillFile(path); + if ("problem" in parsed) { + warnings.push(`${path} skipped: ${parsed.problem}`); + continue; + } + const prior = seen.get(parsed.name); + if (prior !== undefined) { + warnings.push(`${path} shadowed: "${parsed.name}" already provided by ${prior.path}`); + continue; + } + seen.set(parsed.name, { ...parsed, path, source: root.source }); + } + } + return { skills: [...seen.values()], warnings }; +} + +/** Directory entries of a skills root, sorted for deterministic prompts. + * A missing root is the normal case, not an error. */ +function listSkillDirs(dir: string): string[] { + let entries: string[]; + try { + entries = readdirSync(dir); + } catch { + return []; + } + return entries + .filter((e) => { + try { + return statSync(join(dir, e)).isDirectory(); + } catch { + return false; + } + }) + .sort(); +} + +/** Nearest ancestor holding a .git, else cwd — the dsh project-root rule. */ +function findProjectRoot(cwd: string): string { + let dir = cwd; + for (;;) { + if (existsSync(join(dir, ".git"))) return dir; + const parent = dirname(dir); + if (parent === dir) return cwd; + dir = parent; + } +} + +function parseSkillFile( + path: string, +): { name: string; description: string } | { problem: string } { + let raw: string; + try { + raw = readFileSync(path, "utf8"); + } catch (err) { + return { problem: `unreadable (${String(err)})` }; + } + const fm = parseFrontmatter(raw); + if ("problem" in fm) return fm; + const name = fm.fields.name; + const description = fm.fields.description; + if (name === undefined || name === "" || description === undefined || description === "") { + return { problem: "frontmatter requires name and description" }; + } + if (!SKILL_NAME.test(name)) { + return { problem: `invalid skill name "${name}" (lowercase alphanumerics and hyphens)` }; + } + return { name, description }; +} + +/** Minimal YAML frontmatter reader — deliberately a subset, zero deps. + * Top-level `key: value` scalars (plain, quoted, or |/> block); indented + * lines under a structured key (e.g. metadata maps) are tolerated and + * ignored. Anything else is malformed and skips the skill with a warning. */ +export function parseFrontmatter( + raw: string, +): { fields: Record } | { problem: string } { + const lines = raw.split("\n").map((l) => l.replace(/\r$/, "")); + if (lines[0] !== "---") return { problem: "missing YAML frontmatter" }; + const fields: Record = {}; + for (let i = 1; i < lines.length; i++) { + const line = lines[i]; + if (line === "---") return { fields }; + if (line.trim() === "" || line.trim().startsWith("#")) continue; + // Indented continuation: belongs to a nested structure or block we + // already consumed — never to the flat fields we need. + if (/^\s/.test(line)) continue; + const m = /^([A-Za-z0-9_-]+):(.*)$/.exec(line); + if (m === null) return { problem: `malformed YAML frontmatter (line ${i + 1}: "${line}")` }; + const key = m[1]; + let value = m[2].trim(); + if (value === "|" || value === ">" || value === "|-" || value === ">-") { + // The index renders one line per skill, so both literal and folded + // blocks collapse to a single space-joined line here. + const block: string[] = []; + while (i + 1 < lines.length && (/^\s/.test(lines[i + 1]) || lines[i + 1] === "")) { + const piece = lines[i + 1].trim(); + if (piece !== "") block.push(piece); + i++; + } + value = block.join(" "); + } else if ( + (value.startsWith('"') && value.endsWith('"') && value.length >= 2) || + (value.startsWith("'") && value.endsWith("'") && value.length >= 2) + ) { + value = value.slice(1, -1); + } + fields[key] = value; + } + return { problem: "unterminated YAML frontmatter (no closing ---)" }; +} + +/** Body after the frontmatter — what the skill tool returns. A file that + * lost its frontmatter since discovery degrades to its full text. */ +export function stripFrontmatter(raw: string): string { + const lines = raw.split("\n"); + if (lines[0]?.replace(/\r$/, "") !== "---") return raw; + for (let i = 1; i < lines.length; i++) { + if (lines[i].replace(/\r$/, "") === "---") return lines.slice(i + 1).join("\n"); + } + return raw; +} diff --git a/src/tools/skill.ts b/src/tools/skill.ts new file mode 100644 index 0000000..fb0044c --- /dev/null +++ b/src/tools/skill.ts @@ -0,0 +1,67 @@ +// The skill tool: on-demand loading for discovered SKILL.md skills. +// Progressive disclosure (DESIGN.md "Skills"): the system prompt carries +// one index line per skill, the body enters context only when the model +// asks for it — skill bodies never sit in the stable prefix. Built by +// factory because it closes over the discovery result; the schema itself +// is static (no name enum), so the tool's prefix bytes do not change when +// skills are added or removed. + +import type { ToolDefinition } from "./index"; +import { normalizeAliases } from "./index"; +import type { Skill } from "../skills/discover"; +import { stripFrontmatter } from "../skills/discover"; +import { truncateHead } from "./truncate"; + +export function makeSkillTool(skills: Skill[]): ToolDefinition { + return { + name: "skill", + description: + "Load a skill: reusable instructions for a specific kind of task. " + + "The available skills are indexed in the system prompt; pass a name " + + "from that index to get the skill's full instructions.", + promptSnippet: "Load a skill's full instructions by name.", + promptGuidelines: [ + "When the task matches a skill's description in the Skills index, load that skill first and follow its instructions instead of improvising the workflow it covers.", + ], + inputSchema: { + type: "object", + properties: { + name: { + type: "string", + description: "Skill name, exactly as it appears in the Skills index.", + }, + }, + required: ["name"], + }, + coerce: (input) => normalizeAliases(input, { name: ["skill", "skill_name"] }), + async execute(input) { + const name = String(input.name); + const skill = skills.find((s) => s.name === name); + if (skill === undefined) { + return { + output: `Unknown skill "${name}". Available skills: ${skills.map((s) => s.name).join(", ")}.`, + isError: true, + }; + } + // Read fresh on every invoke: the file can change between discovery + // and use, and stale instructions are worse than a re-read. + const file = Bun.file(skill.path); + if (!(await file.exists())) { + return { output: `Skill file no longer exists: ${skill.path}`, isError: true }; + } + const body = stripFrontmatter(await file.text()).trim(); + if (body === "") { + return { output: `Skill "${name}" has no instructions beyond its description.` }; + } + const r = truncateHead(body); + // The path lets the model read files the skill references relative + // to its own directory (the resource-base convention). + return { + output: + `Skill "${name}" (from ${skill.path}):\n\n` + + r.text + + (r.truncated ? `\n${r.notice}` : ""), + }; + }, + }; +} diff --git a/src/ui/repl.ts b/src/ui/repl.ts index cd71ad7..6cc1f01 100644 --- a/src/ui/repl.ts +++ b/src/ui/repl.ts @@ -28,6 +28,7 @@ import { addUsage, zeroUsage, type Message, type Usage } from "../provider/types import { compactThreshold } from "../engine/compact"; import { MODELS } from "../provider/catalog"; import { renderReport, type SubagentManager } from "../engine/subagent"; +import type { SkillIndexEntry } from "../engine/prompt"; import type { ToolDefinition } from "../tools/index"; import { Session } from "../session/session"; import type { SessionStore } from "../session/store"; @@ -53,6 +54,8 @@ export type ReplOptions = { * differencing cumulative totals. */ makeManager: () => SubagentManager; makeTools: (mgr: SubagentManager) => ToolDefinition[]; + /** Discovered skill index; fixed for the life of the process. */ + skills: SkillIndexEntry[]; model: string; cwd: string; apiKey: string; @@ -232,6 +235,7 @@ export class Repl { apiKey: this.opts.apiKey, baseUrl: this.opts.baseUrl, tools: this.opts.makeTools(manager), + skills: this.opts.skills, maxTurns: this.opts.maxTurns, contextBudget: this.opts.contextBudget, signal: ac.signal, @@ -404,7 +408,12 @@ export class Repl { ` ${dim("context ")} ${formatCount(used)} / ${formatCount(threshold)} tokens (${pct}%)\n` + ` ${dim("turns ")} ${turns}\n` + ` ${dim("cost ")} ${this.totalsLine()}\n` + - ` ${dim("tools ")} ${this.toolNames.join(", ")}\n\n`, + ` ${dim("tools ")} ${this.toolNames.join(", ")}\n` + + ` ${dim("skills ")} ${ + this.opts.skills.length > 0 + ? this.opts.skills.map((s) => s.name).join(", ") + : "(none discovered)" + }\n\n`, ); return false; } diff --git a/tests/prompt-golden.test.ts b/tests/prompt-golden.test.ts index 81689f0..e865c45 100644 --- a/tests/prompt-golden.test.ts +++ b/tests/prompt-golden.test.ts @@ -10,6 +10,7 @@ import { bashTool } from "../src/tools/bash"; import { editTool } from "../src/tools/edit"; import { writeTool } from "../src/tools/write"; import { makeTaskTool } from "../src/tools/task"; +import { makeSkillTool } from "../src/tools/skill"; const GOLDEN = `You are dsc, a coding agent operating non-interactively inside a workspace directory. Work directly toward the user's task: investigate, act, verify. There is no user available to answer questions mid-run, so make reasonable assumptions and proceed. @@ -52,6 +53,50 @@ Guidelines: Current working directory: /work`; +// Skills prompt (M8): with skills discovered, the skill tool adds one +// index line + one guideline, and a Skills section carries one line per +// skill — the ONLY skill bytes in the prefix; bodies load on invoke. +// With zero skills the tool is not registered and the prompt stays the +// 4-tool GOLDEN above, byte for byte. +const GOLDEN_SKILLS = `You are dsc, a coding agent operating non-interactively inside a workspace directory. Work directly toward the user's task: investigate, act, verify. There is no user available to answer questions mid-run, so make reasonable assumptions and proceed. + +Available tools: +- read: Read a file. +- bash: Run a bash command in the working directory. +- edit: Replace text in a file. +- write: Write a file, creating parent directories as needed and overwriting any existing content. +- skill: Load a skill's full instructions by name. + +Guidelines: +- bash covers search and inspection: grep -rn, find, ls, cat, head. There are no separate search tools. +- Read a file before editing it; edit requires oldText to match the file content exactly. +- When the task matches a skill's description in the Skills index, load that skill first and follow its instructions instead of improvising the workflow it covers. +- Fix bugs at the root cause: correct the faulty code itself, never add compensating code that masks it. If a regex or condition is wrong, fix that regex or condition rather than patching its output afterwards. Keep changes minimal and scoped to the task; do not refactor beyond it. +- Never modify files the task tells you not to touch. +- Verify your work by running the relevant command (tests, build, the script the task names) before finishing. +- Your final text message is the deliverable. Keep it brief. If the task asks a question, end with exactly the answer requested. + +Skills (reusable instructions; load the full text with the skill tool): +- release-notes: Draft release notes from merged PRs. +- sql-migrations: Write and review SQL migration files safely. + +Current working directory: /work`; + +const SKILLS = [ + { + name: "release-notes", + description: "Draft release notes from merged PRs.", + path: "/skills/release-notes/SKILL.md", + source: "project-agents" as const, + }, + { + name: "sql-migrations", + description: "Write and review SQL migration files safely.", + path: "/skills/sql-migrations/SKILL.md", + source: "user-agents" as const, + }, +]; + describe("system prompt golden", () => { test("child/solo prompt is byte-identical to the gated version", () => { const p = buildSystemPrompt([readTool, bashTool, editTool, writeTool], "/work"); @@ -66,4 +111,18 @@ describe("system prompt golden", () => { ); expect(p).toBe(GOLDEN_PARENT); }); + + test("skills prompt is byte-identical to the gated version", () => { + const p = buildSystemPrompt( + [readTool, bashTool, editTool, writeTool, makeSkillTool(SKILLS)], + "/work", + SKILLS, + ); + expect(p).toBe(GOLDEN_SKILLS); + }); + + test("an empty skill index changes nothing: still the gated GOLDEN bytes", () => { + const p = buildSystemPrompt([readTool, bashTool, editTool, writeTool], "/work", []); + expect(p).toBe(GOLDEN); + }); }); diff --git a/tests/repl-status.test.ts b/tests/repl-status.test.ts index 23629e1..a33fabb 100644 --- a/tests/repl-status.test.ts +++ b/tests/repl-status.test.ts @@ -19,6 +19,7 @@ function makeRepl(): { repl: Repl; output: () => string; store: SessionStore } { session: Session.create(store, "deepseek-v4-flash", "/work"), makeManager: () => new SubagentManager({ apiKey: "k", baseUrl: "http://x", cwd: "/work" }), makeTools: () => [readTool, bashTool], + skills: [{ name: "release-notes", description: "Draft release notes." }], model: "deepseek-v4-flash", cwd: "/work", apiKey: "k", @@ -52,5 +53,6 @@ describe("/status", () => { expect(out).toContain("read, bash"); expect(out).toContain("deepseek-v4-flash"); expect(out).toContain("context"); + expect(out).toContain("release-notes"); }); }); diff --git a/tests/skills.test.ts b/tests/skills.test.ts new file mode 100644 index 0000000..6f5533f --- /dev/null +++ b/tests/skills.test.ts @@ -0,0 +1,204 @@ +// Skills (M8): discovery, precedence, frontmatter hardening, and the +// on-demand loading contract of the skill tool. The prompt-injection +// bytes are frozen in prompt-golden.test.ts; this file covers behavior. +// The layout under test is the one dsh reads (/skills// +// SKILL.md across .dsc/.agents, project and home), so these tests are +// also the interop claim: a fixture either harness discovers, both do. + +import { afterEach, describe, expect, test } from "bun:test"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { discoverSkills, parseFrontmatter, stripFrontmatter } from "../src/skills/discover"; +import { makeSkillTool } from "../src/tools/skill"; + +let dirs: string[] = []; +afterEach(() => { + for (const d of dirs) rmSync(d, { recursive: true, force: true }); + dirs = []; +}); + +function tmp(): string { + const d = mkdtempSync(join(tmpdir(), "dsc-skills-")); + dirs.push(d); + return d; +} + +function addSkill(root: string, sub: string, name: string, frontmatter: string, body = "Do the thing."): string { + const dir = join(root, sub, name); + mkdirSync(dir, { recursive: true }); + const path = join(dir, "SKILL.md"); + writeFileSync(path, `---\n${frontmatter}\n---\n\n${body}\n`); + return path; +} + +describe("skill discovery", () => { + test("empty and missing roots discover nothing, without warnings", () => { + const project = tmp(); + const home = tmp(); + mkdirSync(join(project, ".agents", "skills"), { recursive: true }); + const r = discoverSkills(project, home); + expect(r.skills).toEqual([]); + expect(r.warnings).toEqual([]); + }); + + test("finds skills in all four roots with name and description from frontmatter", () => { + const project = tmp(); + const home = tmp(); + addSkill(project, ".dsc/skills", "alpha", 'name: alpha\ndescription: "First skill."'); + addSkill(project, ".agents/skills", "beta", "name: beta\ndescription: Second skill."); + addSkill(home, ".dsc/skills", "gamma", "name: gamma\ndescription: Third skill."); + addSkill(home, ".agents/skills", "delta", "name: delta\ndescription: Fourth skill."); + const r = discoverSkills(project, home); + expect(r.skills.map((s) => [s.name, s.source])).toEqual([ + ["alpha", "project-dsc"], + ["beta", "project-agents"], + ["gamma", "user-dsc"], + ["delta", "user-agents"], + ]); + expect(r.skills[0].description).toBe("First skill."); + expect(r.warnings).toEqual([]); + }); + + test("project wins a name collision against home, with a shadow warning", () => { + const project = tmp(); + const home = tmp(); + const winner = addSkill(project, ".agents/skills", "dupe", "name: dupe\ndescription: Project copy."); + addSkill(home, ".agents/skills", "dupe", "name: dupe\ndescription: Home copy."); + const r = discoverSkills(project, home); + expect(r.skills).toHaveLength(1); + expect(r.skills[0].description).toBe("Project copy."); + expect(r.skills[0].path).toBe(winner); + expect(r.warnings).toHaveLength(1); + expect(r.warnings[0]).toContain("shadowed"); + }); + + test(".dsc beats .agents within the same scope (harness-native first, the dsh rank order)", () => { + const project = tmp(); + const home = tmp(); + addSkill(project, ".dsc/skills", "dupe", "name: dupe\ndescription: Native copy."); + addSkill(project, ".agents/skills", "dupe", "name: dupe\ndescription: Shared copy."); + const r = discoverSkills(project, home); + expect(r.skills).toHaveLength(1); + expect(r.skills[0].source).toBe("project-dsc"); + }); + + test("walks up to the .git project root, so subdirectories see project skills", () => { + const project = tmp(); + const home = tmp(); + mkdirSync(join(project, ".git"), { recursive: true }); + addSkill(project, ".agents/skills", "rooted", "name: rooted\ndescription: At the repo root."); + const sub = join(project, "src", "deep"); + mkdirSync(sub, { recursive: true }); + const r = discoverSkills(sub, home); + expect(r.skills.map((s) => s.name)).toEqual(["rooted"]); + }); + + test("skips with a warning instead of crashing: missing fields, bad name, malformed YAML", () => { + const project = tmp(); + const home = tmp(); + addSkill(project, ".agents/skills", "no-desc", "name: no-desc"); + addSkill(project, ".agents/skills", "bad-name", "name: Bad_Name\ndescription: Casing dsh rejects too."); + addSkill(project, ".agents/skills", "mangled", "name mangled\nnot yaml at all"); + const unterminated = join(project, ".agents/skills", "unterminated"); + mkdirSync(unterminated, { recursive: true }); + writeFileSync( + join(unterminated, "SKILL.md"), + "---\nname: unterminated\ndescription: x\nbody without a closing fence\n", + ); + addSkill(project, ".agents/skills", "ok", "name: ok\ndescription: Survives its broken neighbors."); + const r = discoverSkills(project, home); + expect(r.skills.map((s) => s.name)).toEqual(["ok"]); + expect(r.warnings).toHaveLength(4); + expect(r.warnings.join("\n")).toContain("requires name and description"); + expect(r.warnings.join("\n")).toContain('invalid skill name "Bad_Name"'); + expect(r.warnings.join("\n")).toContain("malformed YAML"); + expect(r.warnings.join("\n")).toContain("unterminated"); + }); + + test("a directory without SKILL.md and a file without frontmatter", () => { + const project = tmp(); + const home = tmp(); + mkdirSync(join(project, ".agents", "skills", "not-a-skill"), { recursive: true }); + const bare = join(project, ".agents", "skills", "bare"); + mkdirSync(bare, { recursive: true }); + writeFileSync(join(bare, "SKILL.md"), "Just prose, no frontmatter.\n"); + const r = discoverSkills(project, home); + expect(r.skills).toEqual([]); + // No SKILL.md = not a skill, silently (matches dsh); no frontmatter + // in a SKILL.md = broken skill, warned. + expect(r.warnings).toHaveLength(1); + expect(r.warnings[0]).toContain("missing YAML frontmatter"); + }); +}); + +describe("frontmatter parsing", () => { + test("plain, quoted, and folded-block scalars", () => { + const r = parseFrontmatter( + "---\nname: fold\ndescription: >-\n Spans two\n source lines.\nlicense: 'MIT'\n---\nbody", + ); + if ("problem" in r) throw new Error(r.problem); + expect(r.fields.name).toBe("fold"); + expect(r.fields.description).toBe("Spans two source lines."); + expect(r.fields.license).toBe("MIT"); + }); + + test("tolerates nested structures it does not need", () => { + const r = parseFrontmatter("---\nname: meta\ndescription: Has metadata.\nmetadata:\n author: someone\n---\n"); + if ("problem" in r) throw new Error(r.problem); + expect(r.fields.name).toBe("meta"); + }); + + test("stripFrontmatter returns the body, or the whole text when no frontmatter", () => { + expect(stripFrontmatter("---\nname: x\n---\nThe body.\n")).toBe("The body.\n"); + expect(stripFrontmatter("No fences here.\n")).toBe("No fences here.\n"); + }); +}); + +describe("skill tool", () => { + const ctx = { cwd: "/work" }; + + test("loads the body on demand, without the frontmatter, naming the source path", async () => { + const project = tmp(); + const home = tmp(); + const path = addSkill(project, ".agents/skills", "loader", "name: loader\ndescription: Loads.", "Step 1: read.\nStep 2: act."); + const { skills } = discoverSkills(project, home); + const tool = makeSkillTool(skills); + const r = await tool.execute({ name: "loader" }, ctx); + expect(r.isError ?? false).toBe(false); + expect(r.output).toContain('Skill "loader"'); + expect(r.output).toContain(path); + expect(r.output).toContain("Step 1: read."); + expect(r.output).not.toContain("description:"); + }); + + test("unknown name errors and lists what exists", async () => { + const project = tmp(); + const home = tmp(); + addSkill(project, ".agents/skills", "real", "name: real\ndescription: Exists."); + const tool = makeSkillTool(discoverSkills(project, home).skills); + const r = await tool.execute({ name: "imaginary" }, ctx); + expect(r.isError).toBe(true); + expect(r.output).toContain("real"); + }); + + test("a file deleted after discovery errors instead of crashing", async () => { + const project = tmp(); + const home = tmp(); + const path = addSkill(project, ".agents/skills", "gone", "name: gone\ndescription: Doomed."); + const tool = makeSkillTool(discoverSkills(project, home).skills); + rmSync(path); + const r = await tool.execute({ name: "gone" }, ctx); + expect(r.isError).toBe(true); + expect(r.output).toContain("no longer exists"); + }); + + test("coerces the aliases models actually emit", async () => { + const project = tmp(); + const home = tmp(); + addSkill(project, ".agents/skills", "aliased", "name: aliased\ndescription: Aliases."); + const tool = makeSkillTool(discoverSkills(project, home).skills); + expect(tool.coerce!({ skill_name: "aliased" })).toEqual({ name: "aliased" }); + expect(tool.coerce!({ skill: "aliased" })).toEqual({ name: "aliased" }); + }); +});