diff --git a/CHANGELOG.md b/CHANGELOG.md index cdf29bf2e..7a4b023a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ ### Added - **`/write-skill`**: three modes. New writes a skill from a request; change adds a lesson by editing the line it belongs to rather than appending a paragraph; diet produces a free report first (size bill, drift findings, suggested level moves), then eval tasks written before any cut, then the cut and a before, after and no-skill eval. It works on your own skill folders, never edits an installed gstack skill in place (it points at the checkout's template or a copy you own), and asks for a `Skill-Size: → bytes` line in the commit. Not `/skillify`, which codifies a browser scrape. -- **`gstack-skill-bill [--json]`**: bytes and estimated tokens for a skill's description, template, render and on-demand files, the last `Skill-Diet:` commit and the growth since, `DUE` past 10%, and `baseline: unknown` on shallow or non-git folders. Errors carry stable codes and a fix line (`SKILL_NOT_FOUND` with the closest names, `SKILL_AMBIGUOUS`, `SKILL_MD_MISSING`, `RENDER_STALE`). +- **`gstack-skill-bill [--json]`**: bytes and estimated tokens for a skill's description, template, render and on-demand files, the last `Skill-Diet:` commit and the growth since, `DUE` past 10%, and `baseline: unknown` on shallow or non-git folders. In a gstack checkout it also names the skill's eval task file and checks it against the contract (`EVAL TASKS:`, `EVAL_TASKS_INVALID`). Errors carry stable codes and a fix line (`SKILL_NOT_FOUND` with the closest names, `SKILL_AMBIGUOUS`, `SKILL_MD_MISSING`, `RENDER_STALE`). - **Skill-to-bin drift ratchet** (`scripts/skill-bin-drift.ts`, `test/skill-bin-drift.test.ts`): every gstack bin call in templates, carved sections and the Claude-host render must name a bin that exists and flags its parser accepts, or the free suite fails with `DRIFT_UNKNOWN_BIN` or `DRIFT_UNKNOWN_FLAG` at file:line. Today: 1,751 calls to 105 bins, one allowlisted false positive, and an allowlist that can only shrink. It also lists bins with permissive parsers or no usage text as the help backlog. - **`docs/designs/SKILL_EVAL_CONTRACT.md`**: the interface the shared skill eval implements (arms, task format, verdict file, exit codes, the per-task verdict policy). Task files live at `test/fixtures/skill-evals//tasks.json`, and a free test holds them to the format. - **`gstack-learnings-search --stats`** replaces the stats program /learn used to carry inline, and both learnings bins answer `--help`. diff --git a/bin/gstack-skill-bill b/bin/gstack-skill-bill index 352d0b320..86aa1c657 100755 --- a/bin/gstack-skill-bill +++ b/bin/gstack-skill-bill @@ -10,6 +10,8 @@ session), SKILL.md and its template (every use), and sections/ or references/ fi Names the skill's kind: gstack-checkout (edit SKILL.md.tmpl, then bun run gen:skill-docs), gstack-install (never edit; gstack-upgrade overwrites it) or user. BASELINE is the newest commit with a "Skill-Diet: " trailer; DUE means the authored file grew over 10% since then. +In a gstack checkout, EVAL TASKS names test/fixtures/skill-evals//tasks.json (or none) and +EVAL_TASKS_INVALID flags a file that breaks docs/designs/SKILL_EVAL_CONTRACT.md's task format. A name must match exactly; pass a path when it names more than one skill. Exit 0 on a report, 2 on SKILL_NOT_FOUND, SKILL_AMBIGUOUS or SKILL_MD_MISSING (each with a fix line).`; diff --git a/docs/skills.md b/docs/skills.md index ea27a169a..ce0f91cf8 100644 --- a/docs/skills.md +++ b/docs/skills.md @@ -899,7 +899,8 @@ gstack's own templates and for any skill folder you own. add nothing a fresh session already does; edit the line that covers the idea instead of appending; put the fact on the lowest level that reaches the agent in time (description, SKILL.md, `references/`, the tool's `--help`, the tool's errors); write the rule and put the story in the commit message. -- **Size.** `gstack-skill-bill` prints bytes and estimated tokens per level. Commits carry +- **Size.** `gstack-skill-bill` prints bytes and estimated tokens per level, and in a gstack checkout the skill's eval + task file (`EVAL TASKS:`). Commits carry `Skill-Size: → bytes`; a skill more than 10% over its last `Skill-Diet:` commit is DUE, and a red context-budget ratchet means diet, not a higher ceiling. - **Drift.** `test/skill-bin-drift.test.ts` fails when a template or rendered skill calls a gstack bin or flag that diff --git a/lib/skill-bill.ts b/lib/skill-bill.ts index 8fd772b27..f72e7d560 100644 --- a/lib/skill-bill.ts +++ b/lib/skill-bill.ts @@ -27,9 +27,42 @@ export interface SkillBillReport { authored: string; rows: LevelRow[]; baseline: Baseline; + evalTasks?: EvalTasks | null; warnings: string[]; } +export interface EvalTasks { path: string; count: number; problems: string[] } + +const EVAL_TASK_KEYS = new Set(["id", "prompt", "setup", "must", "must_not", "check", "safety"]); + +/** Problems in a tasks.json against the task format in docs/designs/SKILL_EVAL_CONTRACT.md. */ +export function checkEvalTasks(file: string): EvalTasks { + let tasks: unknown; + try { tasks = JSON.parse(readFileSync(file, "utf-8")); } catch (err) { return { path: file, count: 0, problems: [`not valid JSON: ${(err as Error).message}`] }; } + if (!Array.isArray(tasks)) return { path: file, count: 0, problems: ["must be a JSON array of tasks"] }; + const problems: string[] = []; + const ids = new Set(); + for (const t of tasks as Record[]) { + const where = String(t?.id ?? "?"); + for (const k of Object.keys(t ?? {})) if (!EVAL_TASK_KEYS.has(k)) problems.push(`${where}: unknown key ${k}`); + if (typeof t?.id !== "string" || !/^[a-z0-9]+(-[a-z0-9]+)*$/.test(t.id)) problems.push(`${where}: id must be kebab-case`); + else if (ids.has(t.id)) problems.push(`${where}: duplicate id`); + else ids.add(t.id); + if (typeof t?.prompt !== "string" || !t.prompt.trim()) problems.push(`${where}: prompt must be a non-empty string`); + for (const k of ["setup", "check"]) if (t && k in t && typeof t[k] !== "string") problems.push(`${where}: ${k} must be a string`); + for (const k of ["must", "must_not"]) { + if (!t || !(k in t)) continue; + const v = t[k]; + if (!Array.isArray(v) || !v.every((x) => typeof x === "string")) { problems.push(`${where}: ${k} must be a string array`); continue; } + for (const re of v) { try { new RegExp(re); } catch { problems.push(`${where}: ${k} pattern /${re}/ does not compile`); } } + } + if (t && "safety" in t && typeof t.safety !== "boolean") problems.push(`${where}: safety must be a boolean`); + if (!t || (!("must" in t) && !("check" in t))) problems.push(`${where}: needs must or check, or nothing can fail it`); + if (typeof t?.setup === "string" && t.setup.includes("seed.sh") && !existsSync(join(dirname(file), "seed.sh"))) problems.push(`${where}: setup runs seed.sh but it is missing`); + } + return { path: file, count: tasks.length, problems }; +} + export class SkillBillError extends Error { constructor(readonly code: "SKILL_NOT_FOUND" | "SKILL_AMBIGUOUS" | "SKILL_MD_MISSING", message: string, readonly fix: string) { super(message); @@ -161,13 +194,18 @@ export function buildSkillBill(dir: string): SkillBillReport { if (existsSync(md)) rows.push(row("SKILL.md", dir, md)); for (const f of onDemandFiles(dir)) rows.push(row("on-demand", dir, f)); const warnings: string[] = []; + let evalTasks: EvalTasks | null | undefined; if (kind === "gstack-checkout") { + const root = gstackRootOf(dir)!; + const file = join(root, "test", "fixtures", "skill-evals", name, "tasks.json"); + evalTasks = existsSync(file) ? { ...checkEvalTasks(file), path: relative(root, file) } : null; + if (evalTasks?.problems.length) warnings.push(`EVAL_TASKS_INVALID: ${evalTasks.problems[0]}${evalTasks.problems.length > 1 ? ` (+${evalTasks.problems.length - 1} more)` : ""}. fix: the task format in docs/designs/SKILL_EVAL_CONTRACT.md`); const changed = git(dir, ["status", "--porcelain", "--", "SKILL.md.tmpl", "SKILL.md"]).out.split("\n").filter(Boolean); if (changed.some((l) => l.endsWith("SKILL.md.tmpl")) && !changed.some((l) => l.endsWith(" SKILL.md") || l.endsWith("/SKILL.md"))) warnings.push("RENDER_STALE: SKILL.md.tmpl changed but SKILL.md did not. fix: bun run gen:skill-docs"); } if (kind === "gstack-install") warnings.push("INSTALLED_GSTACK_SKILL: gstack-upgrade overwrites this folder. fix: edit /SKILL.md.tmpl in a gstack checkout, or copy the skill to a folder you own"); - return { name, dir, kind, authored, rows, baseline: findBaseline(dir, name, authored), warnings }; + return { name, dir, kind, authored, rows, baseline: findBaseline(dir, name, authored), evalTasks, warnings }; } export function renderSkillBill(r: SkillBillReport): string { @@ -176,6 +214,8 @@ export function renderSkillBill(r: SkillBillReport): string { const b = r.baseline; if (b.status === "found") lines.push(`BASELINE: ${b.sha!.slice(0, 7)} ${b.date} ${r.authored} ${b.bytes} B; now ${b.growthPct! >= 0 ? "+" : ""}${b.growthPct}%${b.due ? ` DUE: over ${DUE_PCT}% since the last diet, so diet before adding` : ""}`); else lines.push(`BASELINE: ${b.status} (${b.reason})`); + if (r.evalTasks) lines.push(`EVAL TASKS: ${r.evalTasks.path} (${r.evalTasks.count} tasks)`); + else if (r.evalTasks === null) lines.push(`EVAL TASKS: none; a diet writes test/fixtures/skill-evals/${r.name}/tasks.json before any cut`); lines.push(...r.warnings); lines.push(`Commit line: Skill-Size: → bytes (${r.authored})`); return lines.join("\n"); diff --git a/test/skill-eval-tasks.test.ts b/test/skill-eval-tasks.test.ts index 6aede913e..9e510c5bd 100644 --- a/test/skill-eval-tasks.test.ts +++ b/test/skill-eval-tasks.test.ts @@ -1,16 +1,19 @@ -import { describe, test, expect } from 'bun:test'; +import { describe, test, expect, afterAll } from 'bun:test'; import * as fs from 'fs'; import * as path from 'path'; +import * as os from 'os'; +import { buildSkillBill, checkEvalTasks, renderSkillBill } from '../lib/skill-bill'; /** * Task files for the shared skill eval (docs/designs/SKILL_EVAL_CONTRACT.md) live at - * test/fixtures/skill-evals//tasks.json. gstack ships no harness, so this free test - * is their only in-repo reader: it holds every file to the contract's task format. + * test/fixtures/skill-evals//tasks.json. gstack-skill-bill reports them for a checkout + * skill through checkEvalTasks; this test holds every committed file to the contract's format. */ const ROOT = path.resolve(import.meta.dir, '..'); const DIR = path.join(ROOT, 'test', 'fixtures', 'skill-evals'); -const KEYS = new Set(['id', 'prompt', 'setup', 'must', 'must_not', 'check', 'safety']); const skills = fs.readdirSync(DIR).filter(d => fs.statSync(path.join(DIR, d)).isDirectory()).sort(); +const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'skill-eval-tasks-')); +afterAll(() => fs.rmSync(tmp, { recursive: true, force: true })); describe('skill-evals task files follow the shared eval contract', () => { test('there is at least one task file', () => { @@ -19,28 +22,37 @@ describe('skill-evals task files follow the shared eval contract', () => { for (const skill of skills) { test(`${skill}/tasks.json`, () => { - const tasks = JSON.parse(fs.readFileSync(path.join(DIR, skill, 'tasks.json'), 'utf8')); - expect(Array.isArray(tasks)).toBe(true); - const problems: string[] = []; - const ids = new Set(); - for (const t of tasks) { - const where = `${skill}/${t?.id ?? '?'}`; - for (const k of Object.keys(t)) if (!KEYS.has(k)) problems.push(`${where}: unknown key ${k}`); - if (typeof t.id !== 'string' || !/^[a-z0-9]+(-[a-z0-9]+)*$/.test(t.id)) problems.push(`${where}: id must be kebab-case`); - if (ids.has(t.id)) problems.push(`${where}: duplicate id`); - ids.add(t.id); - if (typeof t.prompt !== 'string' || !t.prompt.trim()) problems.push(`${where}: prompt must be a non-empty string`); - for (const k of ['setup', 'check']) if (k in t && typeof t[k] !== 'string') problems.push(`${where}: ${k} must be a string`); - for (const k of ['must', 'must_not']) { - if (!(k in t)) continue; - if (!Array.isArray(t[k]) || !t[k].every((x: unknown) => typeof x === 'string')) { problems.push(`${where}: ${k} must be a string array`); continue; } - for (const re of t[k]) { try { new RegExp(re); } catch { problems.push(`${where}: ${k} pattern /${re}/ does not compile`); } } - } - if ('safety' in t && typeof t.safety !== 'boolean') problems.push(`${where}: safety must be a boolean`); - if (!('must' in t) && !('check' in t)) problems.push(`${where}: needs must or check, or nothing can fail it`); - if (typeof t.setup === 'string' && t.setup.includes('seed.sh') && !fs.existsSync(path.join(DIR, skill, 'seed.sh'))) problems.push(`${where}: setup runs seed.sh but ${skill}/seed.sh is missing`); - } - expect(problems).toEqual([]); + const r = checkEvalTasks(path.join(DIR, skill, 'tasks.json')); + expect(r.problems).toEqual([]); + expect(r.count).toBeGreaterThan(0); }); } + + test('a malformed task file is reported, one problem per defect', () => { + const file = path.join(tmp, 'tasks.json'); + fs.writeFileSync(file, JSON.stringify([ + { id: 'Bad Id', prompt: '' }, + { id: 'dup', prompt: 'x', must: ['('], extra: 1 }, + { id: 'dup', prompt: 'y', check: 'true', setup: 'sh "$SKILL_EVAL_FIXTURES/seed.sh"', safety: 'yes' }, + ])); + expect(checkEvalTasks(file).problems).toEqual([ + 'Bad Id: id must be kebab-case', + 'Bad Id: prompt must be a non-empty string', + 'Bad Id: needs must or check, or nothing can fail it', + 'dup: unknown key extra', + 'dup: must pattern /(/ does not compile', + 'dup: duplicate id', + 'dup: safety must be a boolean', + 'dup: setup runs seed.sh but it is missing', + ]); + fs.writeFileSync(file, '{'); + expect(checkEvalTasks(file).problems[0]).toStartWith('not valid JSON'); + }); + + test('gstack-skill-bill shows a checkout skill its eval tasks', () => { + const report = buildSkillBill(path.join(ROOT, 'learn')); + const count = JSON.parse(fs.readFileSync(path.join(DIR, 'learn', 'tasks.json'), 'utf8')).length; + expect(report.evalTasks).toEqual({ path: 'test/fixtures/skill-evals/learn/tasks.json', count, problems: [] }); + expect(renderSkillBill(report)).toContain(`EVAL TASKS: test/fixtures/skill-evals/learn/tasks.json (${count} tasks)`); + }); });