diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 89efaff..393f256 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -42,7 +42,7 @@ jobs: - name: Set version in package.json env: VERSION: ${{ inputs.version }} - run: npm version "$VERSION" --no-git-tag-version + run: npm version "$VERSION" --no-git-tag-version --allow-same-version working-directory: cli-tool - name: Install dependencies diff --git a/AGENTS.md b/AGENTS.md index 5fda736..d57784a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -8,11 +8,13 @@ This is a Node.js/TypeScript project providing an open-source collection of agen - `templates/agents/` -- 108 agent markdown definitions organized by category - `templates/skills/` -- 15 skill definitions (each in `/SKILL.md`) - `templates/configs/` -- 5 opencode.json preset configurations +- `templates/packs/` -- skill packs (design, process, behavior): `sources.lock.json` + bundled skills/commands, imported from weisser-dev/agentic-skills via `cli-tool/scripts/import-packs.js`; never edit by hand - `cli-tool/` -- The `@weisser-dev/awesome-opencode` npm package - `cli-tool/src/index.js` -- Entry point, flow control, re-run detection - `cli-tool/src/setup.js` -- Core logic: detection, prompts, model intelligence, file generation (~1800 lines) - `cli-tool/templates/` -- Bundled copy of templates for npm distribution - `cli-tool/scripts/sync-templates.js` -- Pre-publish script to sync templates from repo root + - `cli-tool/src/lib/packs.js` / `packs-cli.js` -- skill pack installer (pinned sources, sha256 verification, no execution) ## Architecture diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f33d48..797a048 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.5.0] - 2026-10-03 + +### Added + +- **Skill packs** (`awesome-opencode packs`, alias `design-pack`, menu entry "Install skill packs", `configure packs`, optional step at the end of the full setup): + - `design`: frontend design skills (taste skills, image-to-code, DESIGN.md references, web interface review with vendored Web Interface Guidelines, local Playwright checks, `design-workflow`). + - `process`: 23 engineering lifecycle skills and the commands `/spec` `/plan` `/build` `/test` `/constraints` `/review` `/webperf` `/code-simplify` `/ship`. + - `behavior` (opt-in, never part of `all`): ponytail minimal-code mode. + - Options: `--pack design|process|behavior|all`, `--global`, `--only`, `--all`, `--list`, `--dry-run`, `--force`. +- `templates/packs/sources.lock.json`: every upstream source pinned to a reviewed commit with license data and per-file sha256; imported from weisser-dev/agentic-skills with `scripts/import-packs.js`. +- Tests for lock integrity, bundled file provenance, run-time network patterns, selection, URL safety, idempotent install, refusal of locally modified items, backups and sha256 mismatch handling. + +### Security + +- Pack items are copied from the package or downloaded only from `raw.githubusercontent.com` at the pinned commit; every file is verified against its sha256 pin before anything is written. No upstream scripts, hooks, plugins or MCP servers are installed or executed. +- Third-party skills that fetched rules or docs at run time, hotlinked assets, installed tools, used external APIs/model CLIs or pushed/deployed without confirmation are shipped as reviewed, adapted copies or excluded (details in each skill's `UPSTREAM.md`). + +### Changed + +- Release workflow accepts a version that is already set in `package.json` (`npm version --allow-same-version`). + ## [1.4.0] - 2026-03-30 ### Added diff --git a/README.md b/README.md index 485c7b1..487182c 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ The most comprehensive collection of agents, skills, and tooling for [OpenCode](https://opencode.ai) -- the open-source AI coding agent. -**108 agents** | **15 skills** | **18 curated MCP servers** | **Live MCP registry search** | **Smart model detection** +**108 agents** | **15 skills** | **3 skill packs** | **18 curated MCP servers** | **Live MCP registry search** | **Smart model detection** > Inspired by [VoltAgent/awesome-claude-code-subagents](https://github.com/VoltAgent/awesome-claude-code-subagents), fully adapted for OpenCode with permissions, skills, and markdown agent format. @@ -22,7 +22,7 @@ One command. It analyzes your project, walks you through agent/skill/model/MCP s ```shell Awesome OpenCode - 108 agents, 15 skills, smart model config + 108 agents, 15 skills, 3 skill packs, smart model config https://github.com/weisser-dev/awesome-opencode ? How would you like to set up your project? @@ -126,7 +126,7 @@ On **re-run**, it remembers your setup: ```shell Awesome OpenCode - 108 agents, 15 skills, smart model config + 108 agents, 15 skills, 3 skill packs, smart model config https://github.com/weisser-dev/awesome-opencode Already configured! @@ -351,6 +351,60 @@ On **re-run**, it remembers your setup: | `error-triage` | Stack trace parsing and root cause classification | | `performance-profile` | Performance hotspot analysis and optimization plan | +### Skill Packs (design / process / behavior) + +Reviewed third-party skill collections, adapted for local use and pinned to exact upstream +commits. Source of truth, review notes and adaptation tooling: +[weisser-dev/agentic-skills](https://github.com/weisser-dev/agentic-skills/tree/main/packs). + +| Pack | Contents | In `--pack all` | +|------|----------|-----------------| +| `design` | Frontend design: `design-workflow`, `web-design-guidelines` (vendored Web Interface Guidelines), taste skills (`design-taste-frontend`, `redesign-existing-projects`, `minimalist-ui`, `high-end-visual-design`, `industrial-brutalist-ui`), `image-to-code`, `design-md-reference` (72 DESIGN.md references), `playwright-cli` (local screenshots); opt-in: `gpt-taste`, `full-output-enforcement` | yes | +| `process` | 23 engineering lifecycle skills + commands `/spec` `/plan` `/build` `/test` `/constraints` `/review` `/webperf` `/code-simplify` `/ship` (from addyosmani/agent-skills, adapted) | yes | +| `behavior` | `ponytail` minimal-code mode (+ review/audit/debt/help) | no, opt-in | + +```bash +npx @weisser-dev/awesome-opencode packs --list # packs and items +npx @weisser-dev/awesome-opencode packs --dry-run # plan for the design pack +npx @weisser-dev/awesome-opencode packs # design pack -> ./.opencode/skills +npx @weisser-dev/awesome-opencode packs --pack all # design + process (skills + commands) +npx @weisser-dev/awesome-opencode packs --pack behavior # ponytail, only if you want it +npx @weisser-dev/awesome-opencode packs --pack all --global # ~/.config/opencode/{skills,commands} +npx @weisser-dev/awesome-opencode design-pack # alias for "packs --pack design" +``` + +The interactive menu offers the same under **Install skill packs**, and the full setup asks once +at the end. Skills go to `.opencode/skills//SKILL.md` (or `~/.config/opencode/skills/`), +commands to `.opencode/commands/.md` (or `~/.config/opencode/commands/`), the locations +OpenCode scans. Re-running is safe: unchanged items are skipped; items you edited are only +replaced with `--force` (the old version is kept in `.agentic-pack-backups/`). + +Notes: the ponytail figures (fewer lines, lower cost) are the upstream author's own benchmarks +and the mode can reduce thoroughness, so it is never installed by default. `playwright-cli` needs +`@playwright/cli` as a pinned dev dependency of your project +(`npm install --save-dev --save-exact @playwright/cli@0.1.22`); the skill never installs it. + +#### Security model + +- Bundled items (our own skills and the reviewed, adapted copies) are copied from the package; no network. +- The few items that are installed unmodified from upstream (two taste skills, the opt-in output skill, + the DESIGN.md references, four ponytail skills) are downloaded only from + `raw.githubusercontent.com` for the repository and commit pinned in + `templates/packs/sources.lock.json`, and every file must match its sha256 pin before anything is written. +- Nothing is executed: no upstream scripts, hooks, plugins or MCP servers are installed. +- Installed skills and commands do not fetch anything, call external services or install tools at run time; + pushes, deploys and migrations need an explicit instruction. +- No other sources are added without an explicit owner decision and a new review. + +| Source | License | Pinned commit | +|--------|---------|---------------| +| [Leonxlnx/taste-skill](https://github.com/Leonxlnx/taste-skill) | MIT | `ce26fc25` | +| [vercel-labs/web-interface-guidelines](https://github.com/vercel-labs/web-interface-guidelines) | MIT | `e3d624ba` | +| [VoltAgent/awesome-design-md](https://github.com/VoltAgent/awesome-design-md) | MIT | `f6961238` | +| [microsoft/playwright-cli](https://github.com/microsoft/playwright-cli) | Apache-2.0 | `b85c7a73` | +| [addyosmani/agent-skills](https://github.com/addyosmani/agent-skills) | MIT | `a06bc63b` | +| [DietrichGebert/ponytail](https://github.com/DietrichGebert/ponytail) | MIT | `c982cd41` | + ### Smart Model Detection The CLI recognizes models from **any provider** -- including custom Bedrock, Azure, or self-hosted endpoints: @@ -426,6 +480,9 @@ awesome-opencode configure agents # Add/remove agents awesome-opencode configure skills # Add/remove skills awesome-opencode configure models # Change model strategy awesome-opencode configure mcp # Add/remove MCP servers +awesome-opencode configure packs # Choose and install skill packs interactively +awesome-opencode packs [options] # Install skill packs (see "Skill Packs") +awesome-opencode design-pack # Same as "packs --pack design" awesome-opencode --help # Show help ``` @@ -581,6 +638,7 @@ awesome-opencode/ agents/ # 108 agent definitions (.md) skills/ # 15 skill definitions (SKILL.md) configs/ # 5 opencode.json presets + packs/ # skill packs: sources.lock.json + bundled skills/commands cli-tool/ # @weisser-dev/awesome-opencode src/ index.js # Entry point and flow control @@ -588,6 +646,7 @@ awesome-opencode/ templates/ # Bundled copy for npm package scripts/ sync-templates.js # Pre-publish template sync + import-packs.js # Import packs from a weisser-dev/agentic-skills checkout package.json ``` @@ -603,6 +662,7 @@ This project builds on the work of several open-source communities: - **[pricepertoken.com](https://pricepertoken.com/leaderboards/coding)** -- Model benchmark rankings and pricing data for the model intelligence feature. - **[models.dev](https://models.dev)** -- Open-source database of AI models (by anomalyco). Model fingerprints sourced from here. - **[registry.modelcontextprotocol.io](https://registry.modelcontextprotocol.io)** -- Official MCP server registry API for live search. +- **Skill packs** -- [Leonxlnx/taste-skill](https://github.com/Leonxlnx/taste-skill), [vercel-labs/web-interface-guidelines](https://github.com/vercel-labs/web-interface-guidelines), [VoltAgent/awesome-design-md](https://github.com/VoltAgent/awesome-design-md), [microsoft/playwright-cli](https://github.com/microsoft/playwright-cli), [addyosmani/agent-skills](https://github.com/addyosmani/agent-skills), [DietrichGebert/ponytail](https://github.com/DietrichGebert/ponytail); license texts ship with each skill. ## Also by weisser-dev diff --git a/cli-tool/package-lock.json b/cli-tool/package-lock.json index 956b092..c5189fc 100644 --- a/cli-tool/package-lock.json +++ b/cli-tool/package-lock.json @@ -1,12 +1,12 @@ { "name": "@weisser-dev/opencode-advanced-setup", - "version": "1.4.0", + "version": "1.5.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@weisser-dev/opencode-advanced-setup", - "version": "1.4.0", + "version": "1.5.0", "license": "MIT", "dependencies": { "@inquirer/prompts": "^7.0.0", diff --git a/cli-tool/package.json b/cli-tool/package.json index 029b8ac..62640ff 100644 --- a/cli-tool/package.json +++ b/cli-tool/package.json @@ -1,6 +1,6 @@ { "name": "@weisser-dev/awesome-opencode", - "version": "1.4.0", + "version": "1.5.0", "description": "Interactive CLI to set up OpenCode best practices: agents, skills, model optimization, and configs", "type": "module", "bin": { diff --git a/cli-tool/scripts/import-packs.js b/cli-tool/scripts/import-packs.js new file mode 100644 index 0000000..d7b0699 --- /dev/null +++ b/cli-tool/scripts/import-packs.js @@ -0,0 +1,62 @@ +#!/usr/bin/env node + +/** + * Import the skill packs from a weisser-dev/agentic-skills checkout into templates/packs/. + * + * node scripts/import-packs.js /path/to/agentic-skills + * + * agentic-skills is the source of truth (review, adaptation, lock file). This copies the lock + * file and every item that lives in that repository (kinds local, vendored, derived) plus the + * wrapper folders. Items of kind "fetch" are not copied: the CLI fetches them at install time + * from the pinned commits. Run `node scripts/sync-templates.js` afterwards. + */ + +import fs from 'fs'; +import path from 'path'; +import { fileURLToPath } from 'url'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const repoRoot = path.resolve(__dirname, '..', '..'); +const dest = path.join(repoRoot, 'templates', 'packs'); + +const src = process.argv[2]; +if (!src || !fs.existsSync(path.join(src, 'packs', 'sources.lock.json'))) { + console.error('Usage: node scripts/import-packs.js /path/to/agentic-skills'); + process.exit(1); +} + +const lock = JSON.parse(fs.readFileSync(path.join(src, 'packs', 'sources.lock.json'), 'utf8')); + +fs.rmSync(dest, { recursive: true, force: true }); +fs.mkdirSync(dest, { recursive: true }); + +function copy(from, to) { + const stat = fs.statSync(from); + if (stat.isDirectory()) { + fs.mkdirSync(to, { recursive: true }); + for (const entry of fs.readdirSync(from)) copy(path.join(from, entry), path.join(to, entry)); + } else { + fs.mkdirSync(path.dirname(to), { recursive: true }); + fs.copyFileSync(from, to); + } +} + +const copied = new Set(); +for (const item of lock.items) { + if (item.wrapper) { + const rel = item.wrapper.replace(/^packs\//, ''); + copy(path.join(src, item.wrapper), path.join(dest, rel)); + item.wrapper = rel; + } + if (['local', 'vendored', 'derived'].includes(item.kind)) { + // commands share one folder (with UPSTREAM.md and the license file) + const rel = item.type === 'command' ? path.dirname(item.path) : item.path; + if (!copied.has(rel)) { + copy(path.join(src, rel), path.join(dest, rel)); + copied.add(rel); + } + } +} + +fs.writeFileSync(path.join(dest, 'sources.lock.json'), JSON.stringify(lock, null, 2) + '\n'); +console.log(`Imported ${lock.items.length} pack items (${copied.size} local folders) into ${dest}`); diff --git a/cli-tool/src/index.js b/cli-tool/src/index.js index f1fd447..05b8607 100755 --- a/cli-tool/src/index.js +++ b/cli-tool/src/index.js @@ -3,16 +3,21 @@ import path from 'path'; import fs from 'fs'; import { intro, checkExistingSetup, detectProject, promptAgents, promptSkills, promptModels, promptMcp, promptMcpSearch, promptCostControl, generateFiles, promptAgentsMd, outro, launchOpenCode } from './setup.js'; +import { runPacksCommand, promptAndInstallPacks, PACKS_HELP } from './lib/packs-cli.js'; // ── Parse flags ───────────────────────────────────────────────────────────── const rawArgs = process.argv.slice(2); const flags = {}; const positional = []; +const packArgs = []; +const PACK_COMMANDS = new Set(['packs', 'design-pack']); for (let i = 0; i < rawArgs.length; i++) { const arg = rawArgs[i]; - if (arg === '--skipSSL' || arg === '--skip-ssl') { + if (PACK_COMMANDS.has(positional[0]) && !['--skipSSL', '--skip-ssl', '--crt', '--config'].includes(arg)) { + packArgs.push(arg); // options of the packs command are parsed in lib/packs-cli.js + } else if (arg === '--skipSSL' || arg === '--skip-ssl') { flags.skipSSL = true; } else if (arg === '--config' && rawArgs[i + 1]) { flags.configPath = path.resolve(rawArgs[++i]); @@ -53,6 +58,13 @@ if (flags.configPath) { async function main() { try { + if (PACK_COMMANDS.has(command)) { + intro(); + // design-pack = packs with the design pack preselected (kept as a short alias) + await runPacksCommand(packArgs, { defaultPacks: ['design'] }); + return; + } + if (flags.help) { showHelp(); return; @@ -94,6 +106,9 @@ function showHelp() { awesome-opencode configure skills Add/remove skills awesome-opencode configure models Change model strategy awesome-opencode configure mcp Add/remove MCP servers + awesome-opencode configure packs Choose and install skill packs interactively + awesome-opencode packs [options] Install skill packs (design, process, behavior) + awesome-opencode design-pack Same as "packs --pack design" Flags: --help, -h Show this help @@ -113,6 +128,7 @@ function showHelp() { Docs: https://github.com/weisser-dev/awesome-opencode `); + console.log(PACKS_HELP); } async function handleExistingSetup(existing) { @@ -163,6 +179,7 @@ async function handleExistingSetup(existing) { { name: 'Configure skills', value: 'configure-skills' }, { name: 'Configure models', value: 'configure-models' }, { name: 'Configure MCP servers', value: 'configure-mcp' }, + { name: 'Install skill packs (design / process / behavior)', value: 'configure-packs' }, { name: 'Exit', value: 'exit' }, ]; @@ -193,11 +210,19 @@ async function handleExistingSetup(existing) { case 'configure-mcp': await handleConfigure('mcp'); break; + case 'configure-packs': + await promptAndInstallPacks(); + break; // exit: just return } } async function handleConfigure(what) { + if (what === 'packs') { + await promptAndInstallPacks(); + outro(); + return; + } const project = await detectProject({ configPath: flags.configPath }); switch (what) { @@ -243,6 +268,10 @@ async function runFullSetup() { const costControl = await promptCostControl(agents); await generateFiles({ project, agents, skills, modelConfig, mcpConfig, mcpSearchResults, costControl }); + const { confirm } = await import('@inquirer/prompts'); + if (await confirm({ message: 'Install skill packs (design / process / behavior)?', default: false })) { + await promptAndInstallPacks(); + } await promptAgentsMd({ project, agents, skills, modelConfig }); outro(); await launchOpenCode({ flags }); diff --git a/cli-tool/src/lib/packs-cli.js b/cli-tool/src/lib/packs-cli.js new file mode 100644 index 0000000..ab84fb0 --- /dev/null +++ b/cli-tool/src/lib/packs-cli.js @@ -0,0 +1,150 @@ +// ─── Skill packs: command line + interactive flow ─────────────────────────── + +import chalk from 'chalk'; +import { checkbox, select, confirm } from '@inquirer/prompts'; +import { loadLock, findPacksBase, selectItems, defaultDirs, installPacks } from './packs.js'; + +export const PACKS_HELP = ` + awesome-opencode packs [options] Install skill packs into OpenCode + + Packs: + design Frontend design: taste skills, image-to-code, DESIGN.md references, + web interface review, local Playwright checks (default) + process Lifecycle skills + commands /spec /plan /build /test /constraints + /review /webperf /code-simplify /ship + behavior Opt-in "ponytail" minimal-code mode (never part of "all") + + Options: + --pack design | process | behavior | all (comma-separated, default: design) + --global Install into ~/.config/opencode/{skills,commands} (default: ./.opencode/) + --only Install exactly these items (see --list) + --all Include opt-in items of the selected packs + --list Show packs and items + --dry-run Show the plan; no downloads, no writes + --force Replace items modified locally (old version kept in .agentic-pack-backups/) + + Security: only the commits pinned in templates/packs/sources.lock.json are downloaded, + every file is checked against its sha256, nothing is executed. +`; + +/** Parse the arguments after `packs` / `design-pack`. */ +export function parsePacksArgs(argv, { defaultPacks = ['design'] } = {}) { + const opts = { packs: defaultPacks, only: [], all: false, global: false, list: false, dryRun: false, force: false, help: false }; + for (let i = 0; i < argv.length; i++) { + const a = argv[i]; + const next = () => { + if (argv[i + 1] === undefined) throw new Error(`${a} needs a value`); + return argv[++i]; + }; + if (a === '--pack' || a === '--packs') opts.packs = next().split(',').filter(Boolean); + else if (a === '--only') opts.only = next().split(',').filter(Boolean); + else if (a === '--all') opts.all = true; + else if (a === '--global') opts.global = true; + else if (a === '--list') opts.list = true; + else if (a === '--dry-run') opts.dryRun = true; + else if (a === '--force') opts.force = true; + else if (a === '--help' || a === '-h') opts.help = true; + else throw new Error(`Unknown option for packs: ${a}`); + } + return opts; +} + +function printList(lock) { + for (const [name, def] of Object.entries(lock.packs)) { + console.log(chalk.bold(` ${name}`) + chalk.gray(` (${def.inAll ? 'in all' : 'explicit only'}) ${def.description}`)); + for (const it of lock.items.filter((i) => i.pack === name)) { + const tag = it.default ? '' : chalk.yellow(' [opt-in]'); + console.log(chalk.gray(` ${(it.type || 'skill').padEnd(8)} ${it.name.padEnd(30)} ${it.kind.padEnd(9)}`) + tag); + } + } +} + +function printPlan(plan) { + for (const p of plan) { + const color = p.action === 'refuse' ? chalk.red : p.action === 'up-to-date' ? chalk.gray : chalk.green; + const why = ['refuse', 'replace'].includes(p.action) ? ` (${p.state})` : ''; + console.log(color(` ${(p.item.type || 'skill').padEnd(8)} ${p.item.name.padEnd(30)} ${p.action}${why}`)); + } +} + +/** Non-interactive entry point: `awesome-opencode packs ...`. */ +export async function runPacksCommand(argv, { defaultPacks } = {}) { + const opts = parsePacksArgs(argv, { defaultPacks }); + if (opts.help) { console.log(PACKS_HELP); return; } + const base = findPacksBase(); + const lock = loadLock(base); + if (opts.list) { printList(lock); return; } + + const items = selectItems(lock, opts); + if (!items.length) throw new Error('Nothing selected.'); + const dirs = defaultDirs({ global: opts.global }); + console.log(chalk.gray(` skills -> ${dirs.skillsDir}`)); + console.log(chalk.gray(` commands -> ${dirs.commandsDir}${opts.dryRun ? ' (dry run)' : ''}`)); + + try { + const plan = await installPacks({ + lock, base, items, ...dirs, dryRun: opts.dryRun, force: opts.force, + log: (msg) => { if (msg.startsWith('backup')) console.log(chalk.yellow(` ${msg}`)); }, + }); + printPlan(plan); + if (opts.dryRun && plan.some((p) => p.action === 'refuse')) { + console.log(chalk.yellow(' Items marked "refuse" were changed locally or not installed by this tool; use --force to replace them.')); + } + if (!opts.dryRun) console.log(chalk.green(' Done. Start a new OpenCode session to load the skills and commands.')); + } catch (err) { + if (err.plan) printPlan(err.plan); + throw err; + } +} + +/** Interactive flow used by the setup menu and `configure packs`. */ +export async function promptAndInstallPacks() { + const base = findPacksBase(); + if (!base) { + console.log(chalk.yellow(' Skill pack templates not found in this installation.')); + return; + } + const lock = loadLock(base); + const packs = await checkbox({ + message: 'Select skill packs:', + choices: Object.entries(lock.packs).map(([name, def]) => ({ + name: `${name} - ${def.description}`, + value: name, + checked: name === 'design', + })), + }); + if (!packs.length) return; + if (packs.includes('behavior')) { + console.log(chalk.yellow(' behavior (ponytail) changes how the agent writes all code: shorter, minimal solutions.')); + console.log(chalk.yellow(' Its savings figures are the author\'s own benchmarks. Say "stop ponytail" to turn it off.')); + } + const all = await confirm({ message: 'Include opt-in items (e.g. gpt-taste, full-output-enforcement)?', default: false }); + const scope = await select({ + message: 'Install for:', + choices: [ + { name: 'This project (.opencode/skills, .opencode/commands)', value: 'project' }, + { name: 'All projects (~/.config/opencode/skills, .../commands)', value: 'global' }, + ], + }); + const items = selectItems(lock, { packs, all }); + const dirs = defaultDirs({ global: scope === 'global' }); + const plan = await installPacks({ lock, base, items, ...dirs, dryRun: true }); + printPlan(plan); + const refused = plan.filter((p) => p.action === 'refuse'); + let force = false; + if (refused.length) { + force = await confirm({ + message: `${refused.length} item(s) were changed locally or not installed by this tool. Replace them (backup kept)?`, + default: false, + }); + } + const todo = plan.filter((p) => p.action !== 'up-to-date' && (force || p.action !== 'refuse')); + if (!todo.length) { console.log(chalk.gray(' Nothing to do.')); return; } + if (!(await confirm({ message: `Install ${todo.length} item(s)?`, default: true }))) return; + await installPacks({ + lock, base, items: todo.map((p) => p.item), ...dirs, force, + log: (msg) => console.log(chalk.gray(` ${msg}`)), + }); + console.log(chalk.green(' Skill packs installed. Start a new OpenCode session to load them.')); +} + diff --git a/cli-tool/src/lib/packs.js b/cli-tool/src/lib/packs.js new file mode 100644 index 0000000..c580233 --- /dev/null +++ b/cli-tool/src/lib/packs.js @@ -0,0 +1,314 @@ +// ─── Skill packs (design / process / behavior) ────────────────────────────── +// +// Installs reviewed skill packs into OpenCode's skills and commands directories. +// The pack definitions, pins and license data come from templates/packs/sources.lock.json, +// imported from weisser-dev/agentic-skills (scripts/import-packs.js). +// +// Security model: +// - Items of kind local/vendored/derived are copied from the bundled templates (no network). +// - Items of kind "fetch" are downloaded only from raw.githubusercontent.com, only for the +// repository and commit pinned in the lock file, and every file must match its sha256 pin. +// - Nothing is executed; no scripts, hooks, plugins or MCP servers are installed. +// - Existing items that were modified locally or not installed by this tool are never +// overwritten without `force` (the old version is archived in .agentic-pack-backups/). + +import crypto from 'crypto'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { fileURLToPath } from 'url'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); + +export const SKILL_MARKER = '.agentic-pack.json'; +export const COMMAND_MANIFEST = '.agentic-pack-commands.json'; +export const BACKUP_DIR = '.agentic-pack-backups'; +const NAME_RE = /^[a-z0-9]+(-[a-z0-9]+)*$/; +const REPO_RE = /^https:\/\/github\.com\/([A-Za-z0-9._-]+)\/([A-Za-z0-9._-]+)$/; +const COMMIT_RE = /^[0-9a-f]{40}$/; + +export const sha256 = (buf) => crypto.createHash('sha256').update(buf).digest('hex'); + +// ─── Lock + templates ──────────────────────────────────────────────────────── + +/** Directory that holds sources.lock.json and the bundled pack files. */ +export function findPacksBase() { + const packageDir = path.resolve(__dirname, '..', '..'); + for (const dir of [path.join(packageDir, 'templates', 'packs'), path.join(packageDir, '..', 'templates', 'packs')]) { + if (fs.existsSync(path.join(dir, 'sources.lock.json'))) return dir; + } + return null; +} + +export function loadLock(base = findPacksBase()) { + if (!base) throw new Error('Skill pack templates not found (templates/packs/sources.lock.json).'); + return JSON.parse(fs.readFileSync(path.join(base, 'sources.lock.json'), 'utf8')); +} + +/** Default directories for OpenCode (project scope = current directory). */ +export function defaultDirs({ global = false, cwd = process.cwd(), env = process.env } = {}) { + if (global) { + const config = env.OPENCODE_CONFIG_DIR + || path.join(env.XDG_CONFIG_HOME || path.join(os.homedir(), '.config'), 'opencode'); + return { skillsDir: path.join(config, 'skills'), commandsDir: path.join(config, 'commands') }; + } + return { skillsDir: path.join(cwd, '.opencode', 'skills'), commandsDir: path.join(cwd, '.opencode', 'commands') }; +} + +/** + * Resolve the selection. packs: array of pack names ("all" = packs with inAll). + * only: explicit item names (any pack). all: include opt-in items of the selected packs. + */ +export function selectItems(lock, { packs = ['design'], only = [], all = false } = {}) { + const names = new Set(lock.items.map((i) => i.name)); + const unknown = only.filter((n) => !names.has(n)); + if (unknown.length) throw new Error(`Unknown pack item(s): ${unknown.join(', ')}`); + const wanted = new Set(); + for (const p of packs) { + if (p === 'all') { + for (const [name, def] of Object.entries(lock.packs)) if (def.inAll) wanted.add(name); + } else if (lock.packs[p]) { + wanted.add(p); + } else { + throw new Error(`Unknown pack: ${p} (design, process, behavior, all)`); + } + } + return lock.items.filter((it) => (only.length + ? only.includes(it.name) + : wanted.has(it.pack) && (it.default || all))); +} + +// ─── Hashing helpers ───────────────────────────────────────────────────────── + +function walk(dir, base = dir, out = []) { + for (const entry of fs.readdirSync(dir, { withFileTypes: true }).sort((a, b) => (a.name < b.name ? -1 : 1))) { + const p = path.join(dir, entry.name); + if (entry.isDirectory()) walk(p, base, out); + else out.push(path.relative(base, p).split(path.sep).join('/')); + } + return out; +} + +function localPaths(item) { + const paths = ['local', 'vendored', 'derived'].includes(item.kind) ? [item.path] : []; + if (item.wrapper) paths.push(item.wrapper); + return paths; +} + +/** Changes whenever a reinstall would produce different files. */ +export function fingerprint(lock, item, base) { + const src = lock.sources[item.source] || {}; + const h = crypto.createHash('sha256').update(JSON.stringify([item, src.commit || null])); + for (const rel of localPaths(item)) { + const full = path.join(base, rel); + if (fs.statSync(full).isFile()) { h.update(fs.readFileSync(full)); continue; } + for (const f of walk(full)) h.update(`${f}\0`).update(fs.readFileSync(path.join(full, f))); + } + return h.digest('hex'); +} + +function treeHashes(dir) { + const out = {}; + for (const f of walk(dir)) if (f !== SKILL_MARKER) out[f] = sha256(fs.readFileSync(path.join(dir, f))); + return out; +} + +const sameJson = (a, b) => JSON.stringify(sortKeys(a)) === JSON.stringify(sortKeys(b)); +function sortKeys(o) { + if (!o || typeof o !== 'object' || Array.isArray(o)) return o; + return Object.fromEntries(Object.keys(o).sort().map((k) => [k, sortKeys(o[k])])); +} + +function readJson(file, fallback) { + try { return JSON.parse(fs.readFileSync(file, 'utf8')); } catch { return fallback; } +} + +// ─── State ─────────────────────────────────────────────────────────────────── + +/** missing | unmanaged | modified | up-to-date | outdated */ +export function itemState(lock, item, base, { skillsDir, commandsDir }) { + if (item.type === 'command') { + const file = path.join(commandsDir, `${item.name}.md`); + const entry = readJson(path.join(commandsDir, COMMAND_MANIFEST), {})[item.name]; + if (!fs.existsSync(file)) return 'missing'; + if (!entry) return 'unmanaged'; + if (sha256(fs.readFileSync(file)) !== entry.sha256) return 'modified'; + return entry.fingerprint === fingerprint(lock, item, base) ? 'up-to-date' : 'outdated'; + } + const dir = path.join(skillsDir, item.name); + if (!fs.existsSync(dir)) return 'missing'; + const marker = readJson(path.join(dir, SKILL_MARKER), null); + if (!marker) return 'unmanaged'; + if (!sameJson(treeHashes(dir), marker.files)) return 'modified'; + return marker.fingerprint === fingerprint(lock, item, base) ? 'up-to-date' : 'outdated'; +} + +// ─── Fetching pinned upstream files ────────────────────────────────────────── + +/** Build the only URL the installer may download from, or throw. */ +export function rawUrl(source, filePath) { + const m = REPO_RE.exec(source.repo || ''); + if (!m) throw new Error(`Unexpected repository URL in lock: ${source.repo}`); + if (!COMMIT_RE.test(source.commit || '')) throw new Error(`Unexpected commit in lock: ${source.commit}`); + if (filePath.split('/').some((seg) => seg === '..' || seg === '')) throw new Error(`Unexpected path in lock: ${filePath}`); + const encoded = filePath.split('/').map(encodeURIComponent).join('/'); + return `https://raw.githubusercontent.com/${m[1]}/${m[2]}/${source.commit}/${encoded}`; +} + +export async function httpFetcher(url) { + const res = await fetch(url, { redirect: 'error', signal: AbortSignal.timeout(30000) }); + if (!res.ok) throw new Error(`Download failed (${res.status}): ${url}`); + return Buffer.from(await res.arrayBuffer()); +} + +/** Same transform as packs/install.sh and update-lock.py in agentic-skills. */ +export function dropLines(buf, pattern) { + const rx = new RegExp(pattern); + return Buffer.from(buf.toString('utf8').split('\n').filter((l) => !rx.test(l)).join('\n'), 'utf8'); +} + +// ─── Build + install ───────────────────────────────────────────────────────── + +/** Returns Map(relativePath -> Buffer) for an item, verified against the lock. */ +export async function buildItem(lock, item, base, fetcher = httpFetcher) { + const files = new Map(); + for (const rel of localPaths(item)) { + const full = path.join(base, rel); + if (!fs.existsSync(full)) throw new Error(`${item.name}: bundled files missing (${rel})`); + if (fs.statSync(full).isFile()) files.set(path.basename(full), fs.readFileSync(full)); + else for (const f of walk(full)) files.set(f, fs.readFileSync(path.join(full, f))); + } + if (['vendored', 'derived'].includes(item.kind)) { + for (const f of item.files || []) { + if (!f.modified && sha256(files.get(f.file) || Buffer.alloc(0)) !== f.sha256) { + throw new Error(`${item.name}: ${f.file} does not match its pinned sha256`); + } + } + } + if (item.kind === 'fetch') { + const source = lock.sources[item.source]; + if (!source) throw new Error(`${item.name}: unknown source ${item.source}`); + const pattern = item.collection && item.collection.dropLinesMatching; + for (const f of item.files) { + let data = await fetcher(rawUrl(source, f.upstreamPath)); + if (sha256(data) !== f.sha256) throw new Error(`${item.name}: ${f.upstreamPath} sha256 mismatch`); + if (f.collection && pattern) { + data = dropLines(data, pattern); + if (sha256(data) !== (f.installedSha256 || f.sha256)) { + throw new Error(`${item.name}: ${f.file} result after line filter does not match its pin`); + } + } + files.set(f.file, data); + } + } + if (item.type === 'command') { + const body = (files.get(`${item.name}.md`) || Buffer.alloc(0)).toString('utf8'); + if (body.includes('!`')) throw new Error(`${item.name}: command contains shell injection syntax`); + if (!/^---\ndescription: .+\n---\n/.test(body)) throw new Error(`${item.name}: command front matter must contain only a description`); + } else { + const skill = (files.get('SKILL.md') || Buffer.alloc(0)).toString('utf8'); + const fm = /^---\n([\s\S]*?)\n---/.exec(skill); + const name = fm && /^name:\s*(\S+)\s*$/m.exec(fm[1]); + if (!name || name[1] !== item.name || !NAME_RE.test(item.name)) { + throw new Error(`${item.name}: SKILL.md front matter name must equal the folder name`); + } + } + return files; +} + +function writeFiles(dir, files) { + for (const [rel, data] of files) { + const out = path.join(dir, rel); + fs.mkdirSync(path.dirname(out), { recursive: true }); + fs.writeFileSync(out, data); + } +} + +function stamp() { + return new Date().toISOString().replace(/[-:T]/g, '').slice(0, 14); +} + +/** + * Plan and (unless dryRun) install. Returns [{ item, state, action }]. + * Throws before writing anything if an item would be refused and force is not set. + */ +export async function installPacks({ + lock, base, items, skillsDir, commandsDir, + dryRun = false, force = false, fetcher = httpFetcher, log = () => {}, +}) { + const plan = items.map((item) => { + const state = itemState(lock, item, base, { skillsDir, commandsDir }); + let action = { missing: 'install', 'up-to-date': 'up-to-date', outdated: 'update' }[state]; + if (!action) action = force ? 'replace' : 'refuse'; + return { item, state, action }; + }); + const refused = plan.filter((p) => p.action === 'refuse'); + if (dryRun) return plan; + if (refused.length) { + const err = new Error(`Refusing to overwrite modified or unmanaged item(s): ${refused.map((p) => p.item.name).join(', ')} (use --force)`); + err.plan = plan; + throw err; + } + + // Build everything first, so a verification failure leaves the target untouched. + const built = new Map(); + for (const p of plan) { + if (p.action === 'up-to-date') continue; + built.set(p.item.name, await buildItem(lock, p.item, base, fetcher)); + } + + const ts = stamp(); + for (const p of plan) { + if (p.action === 'up-to-date') continue; + const { item } = p; + const files = built.get(item.name); + const src = lock.sources[item.source] || {}; + if (item.type === 'command') { + fs.mkdirSync(commandsDir, { recursive: true }); + const dest = path.join(commandsDir, `${item.name}.md`); + if (p.action === 'replace' && fs.existsSync(dest)) { + const bk = path.join(commandsDir, BACKUP_DIR, `${item.name}-${ts}.md.bak`); + fs.mkdirSync(path.dirname(bk), { recursive: true }); + fs.renameSync(dest, bk); + log(`backup: ${bk}`); + } + const data = files.get(`${item.name}.md`); + fs.writeFileSync(dest, data); + const manifestPath = path.join(commandsDir, COMMAND_MANIFEST); + const manifest = readJson(manifestPath, {}); + manifest[item.name] = { + pack: item.pack, source: item.source || null, commit: src.commit || null, + sha256: sha256(data), fingerprint: fingerprint(lock, item, base), + installedBy: '@weisser-dev/awesome-opencode', + }; + fs.writeFileSync(manifestPath, JSON.stringify(sortKeys(manifest), null, 2) + '\n'); + } else { + fs.mkdirSync(skillsDir, { recursive: true }); + const dest = path.join(skillsDir, item.name); + if (fs.existsSync(dest)) { + if (p.action === 'replace') { + // Move the old folder out of the scanned tree under a non-SKILL.md name. + const bk = path.join(skillsDir, BACKUP_DIR, `${item.name}-${ts}`); + fs.mkdirSync(path.dirname(bk), { recursive: true }); + fs.renameSync(dest, bk); + if (fs.existsSync(path.join(bk, 'SKILL.md'))) fs.renameSync(path.join(bk, 'SKILL.md'), path.join(bk, 'SKILL.md.bak')); + log(`backup: ${bk}`); + } else { + fs.rmSync(dest, { recursive: true, force: true }); + } + } + const tmp = `${dest}.tmp-${ts}`; + fs.rmSync(tmp, { recursive: true, force: true }); + writeFiles(tmp, files); + const marker = { + name: item.name, pack: item.pack, installedBy: '@weisser-dev/awesome-opencode', + kind: item.kind, source: item.source || null, repo: src.repo || null, commit: src.commit || null, + license: src.license || 'MIT', fingerprint: fingerprint(lock, item, base), files: treeHashes(tmp), + }; + fs.writeFileSync(path.join(tmp, SKILL_MARKER), JSON.stringify(marker, null, 2) + '\n'); + fs.renameSync(tmp, dest); + } + log(`${item.type || 'skill'} ${item.name}: ${p.action}`); + } + return plan; +} diff --git a/cli-tool/src/setup.js b/cli-tool/src/setup.js index 348c5a5..8fff5d0 100644 --- a/cli-tool/src/setup.js +++ b/cli-tool/src/setup.js @@ -22,7 +22,7 @@ export { launchOpenCode } from './lib/docker.js'; export function intro() { console.log(''); console.log(chalk.bold.cyan(' Awesome OpenCode')); - console.log(chalk.gray(' 108 agents, 15 skills, smart model config')); + console.log(chalk.gray(' 108 agents, 15 skills, 3 skill packs, smart model config')); console.log(chalk.gray(' https://github.com/weisser-dev/awesome-opencode')); console.log(''); } diff --git a/cli-tool/src/tests/packs.test.js b/cli-tool/src/tests/packs.test.js new file mode 100644 index 0000000..a439d2d --- /dev/null +++ b/cli-tool/src/tests/packs.test.js @@ -0,0 +1,265 @@ +// tests/packs.test.js +import { test, describe } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { + loadLock, findPacksBase, selectItems, installPacks, buildItem, itemState, rawUrl, dropLines, + sha256, defaultDirs, SKILL_MARKER, COMMAND_MANIFEST, BACKUP_DIR, +} from '../lib/packs.js'; +import { parsePacksArgs } from '../lib/packs-cli.js'; + +const base = findPacksBase(); +const lock = loadLock(base); +const NAME_RE = /^[a-z0-9]+(-[a-z0-9]+)*$/; + +function tmpdir() { + return fs.mkdtempSync(path.join(os.tmpdir(), 'packs-test-')); +} + +const noNetwork = async (url) => { throw new Error(`unexpected download: ${url}`); }; + +describe('sources.lock.json', () => { + test('has the three packs, behavior is not part of "all"', () => { + assert.deepEqual(Object.keys(lock.packs).sort(), ['behavior', 'design', 'process']); + assert.equal(lock.packs.behavior.inAll, false); + assert.equal(lock.packs.design.inAll, true); + }); + + test('every source is pinned to a full commit on github.com and has a license', () => { + for (const [id, src] of Object.entries(lock.sources)) { + assert.match(src.repo, /^https:\/\/github\.com\/[\w.-]+\/[\w.-]+$/, id); + assert.match(src.commit, /^[0-9a-f]{40}$/, id); + assert.ok(src.license, `${id} has no license field`); + } + }); + + test('items are unique, valid names, known packs and kinds', () => { + const names = lock.items.map((i) => i.name); + assert.equal(new Set(names).size, names.length); + for (const it of lock.items) { + assert.match(it.name, NAME_RE, it.name); + assert.ok(lock.packs[it.pack], `${it.name}: unknown pack`); + assert.ok(['local', 'vendored', 'derived', 'fetch'].includes(it.kind), `${it.name}: kind`); + assert.ok(['skill', 'command'].includes(it.type || 'skill'), `${it.name}: type`); + } + }); + + test('fetched files come only from licensed sources and carry sha256 pins', () => { + for (const it of lock.items.filter((i) => i.kind === 'fetch')) { + const src = lock.sources[it.source]; + assert.ok(src, `${it.name}: unknown source`); + assert.match(src.license, /^(MIT|Apache-2\.0)$/, `${it.name}: license ${src.license}`); + for (const f of it.files) { + assert.match(f.sha256, /^[0-9a-f]{64}$/, `${it.name}: ${f.file}`); + if (f.installedSha256) assert.match(f.installedSha256, /^[0-9a-f]{64}$/); + } + } + }); + + test('the unlicensed vercel-labs/agent-skills source is not used by any item', () => { + assert.ok(!lock.items.some((i) => i.source === 'vercel-agent-skills')); + }); +}); + +describe('bundled pack files', () => { + test('every local/vendored/derived item is bundled and valid', async () => { + for (const it of lock.items.filter((i) => i.kind !== 'fetch')) { + const files = await buildItem(lock, it, base, noNetwork); + if ((it.type || 'skill') === 'skill') { + assert.ok(files.has('SKILL.md'), it.name); + const fm = /^---\n([\s\S]*?)\n---/.exec(files.get('SKILL.md').toString()); + assert.ok(/^description: ./m.test(fm[1]), `${it.name}: description`); + } else { + assert.ok(files.has(`${it.name}.md`), it.name); + } + } + }); + + test('vendored web-design-guidelines rules match the pinned upstream sha256', () => { + const it = lock.items.find((i) => i.name === 'web-design-guidelines'); + const f = it.files.find((x) => x.file === 'references/web-interface-guidelines.md'); + const data = fs.readFileSync(path.join(base, it.path, f.file)); + assert.equal(sha256(data), f.sha256); + assert.ok(fs.existsSync(path.join(base, it.path, 'references', 'LICENSE-vercel-web-interface-guidelines'))); + }); + + test('every third-party item ships its license and provenance', () => { + for (const it of lock.items.filter((i) => ['vendored', 'derived'].includes(i.kind))) { + const dir = it.type === 'command' ? path.join(base, path.dirname(it.path)) : path.join(base, it.path); + const upstream = fs.readFileSync(path.join(dir, 'UPSTREAM.md'), 'utf8'); + assert.ok(upstream.includes(lock.sources[it.source].commit), `${it.name}: UPSTREAM.md lacks pinned commit`); + const hasLicense = fs.readdirSync(dir).some((f) => f.startsWith('LICENSE')) + || fs.readdirSync(path.join(dir, 'references')).some((f) => f.startsWith('LICENSE')); + assert.ok(hasLicense, `${it.name}: license file missing`); + } + }); + + test('no bundled skill or command fetches, hotlinks or shells out at run time', () => { + const forbidden = [ + /!`/, // OpenCode/Claude command shell injection + /\bcurl\s+-/, /\bwget\s/, /WebFetch/, /chrome-devtools-mcp/, + /picsum\.photos/, /cdn\.simpleicons\.org/, /raw\.githubusercontent\.com/, + /npx\s+-y\b/, /@latest\b/, + ]; + const files = []; + (function walk(dir) { + for (const e of fs.readdirSync(dir, { withFileTypes: true })) { + const p = path.join(dir, e.name); + if (e.isDirectory()) walk(p); + else if (e.name.endsWith('.md') && e.name !== 'UPSTREAM.md') files.push(p); + } + })(base); + for (const f of files) { + const text = fs.readFileSync(f, 'utf8'); + for (const rx of forbidden) assert.ok(!rx.test(text), `${path.relative(base, f)} matches ${rx}`); + assert.ok(!/[​-‏‪-‮⁠-⁤]/.test(text), `${f}: hidden unicode`); + } + }); +}); + +describe('selection', () => { + test('default is the design pack without opt-in items', () => { + const items = selectItems(lock); + assert.ok(items.length > 0); + assert.ok(items.every((i) => i.pack === 'design' && i.default)); + }); + + test('"all" means design and process, never behavior', () => { + const packs = new Set(selectItems(lock, { packs: ['all'] }).map((i) => i.pack)); + assert.deepEqual([...packs].sort(), ['design', 'process']); + }); + + test('--only overrides packs; unknown names throw', () => { + assert.deepEqual(selectItems(lock, { only: ['ponytail'] }).map((i) => i.name), ['ponytail']); + assert.throws(() => selectItems(lock, { only: ['nope'] }), /Unknown pack item/); + assert.throws(() => selectItems(lock, { packs: ['nope'] }), /Unknown pack/); + }); + + test('argument parsing', () => { + const o = parsePacksArgs(['--pack', 'process,behavior', '--global', '--dry-run', '--force', '--all']); + assert.deepEqual(o.packs, ['process', 'behavior']); + assert.ok(o.global && o.dryRun && o.force && o.all); + assert.throws(() => parsePacksArgs(['--bogus']), /Unknown option/); + }); + + test('default directories', () => { + const p = defaultDirs({ cwd: '/x' }); + assert.equal(p.skillsDir, path.join('/x', '.opencode', 'skills')); + const g = defaultDirs({ global: true, env: { XDG_CONFIG_HOME: '/cfg' } }); + assert.equal(g.commandsDir, path.join('/cfg', 'opencode', 'commands')); + }); +}); + +describe('fetch safety', () => { + test('raw URL is built only from github.com repo + full commit', () => { + const src = { repo: 'https://github.com/a/b', commit: 'f'.repeat(40) }; + assert.equal(rawUrl(src, 'skills/x/SKILL.md'), `https://raw.githubusercontent.com/a/b/${'f'.repeat(40)}/skills/x/SKILL.md`); + assert.throws(() => rawUrl({ ...src, repo: 'https://evil.example/a/b' }, 'x'), /repository/); + assert.throws(() => rawUrl({ ...src, commit: 'main' }, 'x'), /commit/); + assert.throws(() => rawUrl(src, '../etc/passwd'), /path/); + }); + + test('dropLines removes only matching lines', () => { + const out = dropLines(Buffer.from('a\n3. Run `npx @google/design.md lint DESIGN.md`\nb\n'), '^[ \t]*[0-9]+[.] Run `npx @google/design[.]md lint'); + assert.equal(out.toString(), 'a\nb\n'); + }); +}); + +describe('installPacks', () => { + // A synthetic lock with one fetched skill, one local skill and one command. + function fixture() { + const root = tmpdir(); + const pb = path.join(root, 'packs'); + fs.mkdirSync(path.join(pb, 'skills', 'local-one'), { recursive: true }); + fs.writeFileSync(path.join(pb, 'skills', 'local-one', 'SKILL.md'), '---\nname: local-one\ndescription: Local test skill.\n---\n\nBody\n'); + fs.mkdirSync(path.join(pb, 'commands'), { recursive: true }); + fs.writeFileSync(path.join(pb, 'commands', 'hello.md'), '---\ndescription: Say hello\n---\n\nSay hello to $ARGUMENTS.\n'); + const upstreamSkill = Buffer.from('---\nname: fetched-one\ndescription: Fetched test skill.\n---\n\n1. Run `npx @google/design.md lint DESIGN.md`\nKeep\n'); + const filtered = dropLines(upstreamSkill, '^[ \t]*[0-9]+[.] Run `npx @google/design[.]md lint'); + const l = { + packs: { design: { inAll: true, description: 'd' }, behavior: { inAll: false, description: 'b' } }, + sources: { up: { repo: 'https://github.com/o/r', commit: 'a'.repeat(40), license: 'MIT' } }, + items: [ + { name: 'local-one', pack: 'design', type: 'skill', kind: 'local', default: true, path: 'skills/local-one' }, + { name: 'hello', pack: 'design', type: 'command', kind: 'local', default: true, path: 'commands/hello.md' }, + { + name: 'fetched-one', pack: 'design', type: 'skill', kind: 'fetch', default: true, source: 'up', + collection: { dropLinesMatching: '^[ \t]*[0-9]+[.] Run `npx @google/design[.]md lint' }, + files: [{ file: 'SKILL.md', upstreamPath: 'x/SKILL.md', sha256: sha256(upstreamSkill), installedSha256: sha256(filtered), collection: true }], + }, + ], + }; + const calls = []; + const fetcher = async (url) => { calls.push(url); return upstreamSkill; }; + const dirs = { skillsDir: path.join(root, 'out', 'skills'), commandsDir: path.join(root, 'out', 'commands') }; + return { root, base: pb, lock: l, fetcher, calls, dirs }; + } + + test('installs, is idempotent, and records markers', async () => { + const f = fixture(); + const items = selectItems(f.lock, { packs: ['design'] }); + const plan = await installPacks({ lock: f.lock, base: f.base, items, ...f.dirs, fetcher: f.fetcher }); + assert.deepEqual(plan.map((p) => p.action), ['install', 'install', 'install']); + assert.equal(f.calls.length, 1); + assert.match(f.calls[0], /^https:\/\/raw\.githubusercontent\.com\/o\/r\/a{40}\/x\/SKILL\.md$/); + const installed = fs.readFileSync(path.join(f.dirs.skillsDir, 'fetched-one', 'SKILL.md'), 'utf8'); + assert.ok(!installed.includes('npx')); + assert.ok(fs.existsSync(path.join(f.dirs.skillsDir, 'local-one', SKILL_MARKER))); + assert.ok(fs.existsSync(path.join(f.dirs.commandsDir, COMMAND_MANIFEST))); + + const again = await installPacks({ lock: f.lock, base: f.base, items, ...f.dirs, fetcher: noNetwork }); + assert.ok(again.every((p) => p.action === 'up-to-date')); + }); + + test('dry run writes nothing and downloads nothing', async () => { + const f = fixture(); + const plan = await installPacks({ lock: f.lock, base: f.base, items: f.lock.items, ...f.dirs, dryRun: true, fetcher: noNetwork }); + assert.ok(plan.every((p) => p.action === 'install')); + assert.ok(!fs.existsSync(path.join(f.root, 'out'))); + }); + + test('refuses modified or unmanaged items without force, backs up with force', async () => { + const f = fixture(); + await installPacks({ lock: f.lock, base: f.base, items: f.lock.items, ...f.dirs, fetcher: f.fetcher }); + fs.appendFileSync(path.join(f.dirs.skillsDir, 'local-one', 'SKILL.md'), 'edited\n'); + fs.appendFileSync(path.join(f.dirs.commandsDir, 'hello.md'), 'edited\n'); + assert.equal(itemState(f.lock, f.lock.items[0], f.base, f.dirs), 'modified'); + await assert.rejects( + installPacks({ lock: f.lock, base: f.base, items: f.lock.items, ...f.dirs, fetcher: f.fetcher }), + /Refusing to overwrite/, + ); + const plan = await installPacks({ lock: f.lock, base: f.base, items: f.lock.items, ...f.dirs, fetcher: f.fetcher, force: true }); + assert.deepEqual(plan.map((p) => p.action), ['replace', 'replace', 'up-to-date']); + const backups = fs.readdirSync(path.join(f.dirs.skillsDir, BACKUP_DIR)); + assert.equal(backups.length, 1); + // the backup must not be picked up as a second skill + assert.ok(!fs.existsSync(path.join(f.dirs.skillsDir, BACKUP_DIR, backups[0], 'SKILL.md'))); + assert.ok(fs.readdirSync(path.join(f.dirs.commandsDir, BACKUP_DIR)).length === 1); + assert.ok(!fs.readFileSync(path.join(f.dirs.skillsDir, 'local-one', 'SKILL.md'), 'utf8').includes('edited')); + + fs.mkdirSync(path.join(f.dirs.skillsDir, 'foreign'), { recursive: true }); + const foreign = { ...f.lock.items[0], name: 'foreign', path: 'skills/local-one' }; + assert.equal(itemState(f.lock, foreign, f.base, f.dirs), 'unmanaged'); + }); + + test('a sha256 mismatch aborts before anything is written', async () => { + const f = fixture(); + const bad = async () => Buffer.from('tampered'); + await assert.rejects( + installPacks({ lock: f.lock, base: f.base, items: f.lock.items, ...f.dirs, fetcher: bad }), + /sha256 mismatch/, + ); + assert.ok(!fs.existsSync(path.join(f.dirs.skillsDir, 'local-one'))); + }); + + test('real lock: local items of the design pack install without network', async () => { + const out = tmpdir(); + const dirs = { skillsDir: path.join(out, 'skills'), commandsDir: path.join(out, 'commands') }; + const items = selectItems(lock, { packs: ['design', 'process'] }).filter((i) => i.kind !== 'fetch'); + await installPacks({ lock, base, items, ...dirs, fetcher: noNetwork }); + assert.ok(fs.existsSync(path.join(dirs.skillsDir, 'web-design-guidelines', 'references', 'web-interface-guidelines.md'))); + assert.ok(fs.existsSync(path.join(dirs.commandsDir, 'ship.md'))); + }); +}); diff --git a/cli-tool/templates/packs/README.md b/cli-tool/templates/packs/README.md new file mode 100644 index 0000000..05181fc --- /dev/null +++ b/cli-tool/templates/packs/README.md @@ -0,0 +1,19 @@ +# Skill packs (bundled copy) + +Do not edit these files by hand. They are imported from +[weisser-dev/agentic-skills](https://github.com/weisser-dev/agentic-skills) (`packs/`, `skills/`, +`commands/`), which holds the review notes, the adaptation tooling and the source of truth: + +```bash +node cli-tool/scripts/import-packs.js /path/to/agentic-skills +node cli-tool/scripts/sync-templates.js +``` + +- `sources.lock.json`: packs, items, upstream sources pinned to reviewed commits, licenses, sha256 pins. +- `skills/`, `commands/`, `wrappers/`: our own skills and the reviewed copies (vendored or adapted; + each third-party folder has `LICENSE` and `UPSTREAM.md`). +- Items of kind `fetch` are not bundled; the CLI downloads them at install time from the pinned commit + and verifies every file's sha256. + +Paths mentioned inside `UPSTREAM.md` files (`packs/check-vendored.sh`, `packs/sources.lock.json`) +refer to the agentic-skills repository. diff --git a/cli-tool/templates/packs/commands/LICENSE-agent-skills b/cli-tool/templates/packs/commands/LICENSE-agent-skills new file mode 100644 index 0000000..d67778a --- /dev/null +++ b/cli-tool/templates/packs/commands/LICENSE-agent-skills @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2025 Addy Osmani + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/cli-tool/templates/packs/commands/UPSTREAM.md b/cli-tool/templates/packs/commands/UPSTREAM.md new file mode 100644 index 0000000..80b482a --- /dev/null +++ b/cli-tool/templates/packs/commands/UPSTREAM.md @@ -0,0 +1,39 @@ +# Upstream and license (commands) + +The slash commands in this folder are adapted from `.claude/commands/` of an MIT-licensed upstream repository. They work unchanged in Claude Code (`.claude/commands/` or `~/.claude/commands/`) and OpenCode (`.opencode/commands/` or `~/.config/opencode/commands/`): front matter uses only `description`, the body uses `$ARGUMENTS`, and no file contains shell injection (`!` + backtick). + +| Field | Value | +|---|---| +| Source repository | https://github.com/addyosmani/agent-skills | +| Path | `.claude/commands/` | +| Pinned commit | `a06bc63b3f8b829c14b0bbf53d99fefc39d58092` | +| Retrieved | 2026-10-03 | +| License | MIT License, Copyright (c) 2025 Addy Osmani (full text: `LICENSE-agent-skills`) | + +Upstream sha256 per file: + +| File | sha256 | +|---|---| +| `build.md` | `74cc85b5092f8e209bcda36441a4ce17a5cd9efd2a7d8ce76ba202816088e277` | +| `code-simplify.md` | `71e9e8d38c5cbab5b7ecff324ed4ea0f1aaa4fd053ea6c2b717f2fbadfb65a91` | +| `constraints.md` | `f6d8fdf69be74059db907cf474149eae336b6fbfbcbfb791546e321085612171` | +| `plan.md` | `91d961b2a7011209bc6771c176932e72e11b99d0c524e3f63c9f2e2a94fbc993` | +| `review.md` | `8ad7da75873b430b737328d1bdd95fc8d2de4a4866737087feb4d218abe4e88f` | +| `ship.md` | `02d2d0fc10bfb58bb62dcfe22ed396cd2b9f55dced7d5cd92fce6d29f0da4ace` | +| `spec.md` | `82325b4cc75ae2413c7bf4b6f84804eb662935d9df98edab50eef2b39ff904b0` | +| `test.md` | `62f72b64cfac6db984db072fdc987a23e6049876191297da5f37367def986ff2` | +| `webperf.md` | `74c0c1335f1397a5138a78eda4275e4e5deba16c4537f3a4b4971c3280d0ba27` | + +## Changes made in this copy + +- All files: removed the plugin namespace prefix `agent-skills:` from skill names (skills are installed as plain folders); added an attribution comment at the end. +- `code-simplify.md`: reads the project's agent instructions (CLAUDE.md / AGENTS.md), not only CLAUDE.md. +- `test.md`: browser verification through the local `playwright-cli` skill instead of `browser-testing-with-devtools` (Chrome DevTools MCP). +- `constraints.md`: step 4 proposes tool installs (pinned) and waits for approval instead of installing; tools that call online services run in CI unless the user wants them locally; the CLAUDE.md/AGENTS.md line is added only after the user agrees. +- `ship.md`: rewritten fan-out that uses the user's own reviewer agents if present, otherwise general-purpose subagents briefed with `code-review-and-quality`, `security-and-hardening` and `test-driven-development` (the upstream `agents/` personas are not installed); `/ship` never deploys, pushes, migrates or flips flags by itself; dependency checks without network audits unless the user consents. +- `webperf.md`: rewritten to quick mode plus deep mode from local artifacts only; removed PageSpeed Insights, CrUX API, Chrome DevTools MCP, `npx` downloads and the `web-performance-auditor` subagent (not installed). +- `build.md`, `plan.md`, `review.md`, `spec.md`: only the namespace change. + +## Updating + +Review `git diff a06bc63b3f8b829c14b0bbf53d99fefc39d58092 -- .claude/commands/`, re-apply the changes above, update commit and hashes here and in `packs/sources.lock.json`, then run `packs/check-vendored.sh --online`. diff --git a/cli-tool/templates/packs/commands/build.md b/cli-tool/templates/packs/commands/build.md new file mode 100644 index 0000000..3f843ab --- /dev/null +++ b/cli-tool/templates/packs/commands/build.md @@ -0,0 +1,46 @@ +--- +description: Implement tasks incrementally — build, test, verify, commit. Add "auto" to run the whole plan in one approved pass. +--- + +Invoke the incremental-implementation skill alongside test-driven-development. + +## Modes + +- **`/build`** — implement the *next* pending task, then stop (careful, one slice at a time). +- **`/build auto`** — generate the plan if needed, get a single approval, then implement *every* task without stopping between them. + +`$ARGUMENTS` selects the mode. Treat `auto` (canonical) or `all` as autonomous mode; anything else (or empty) is the default single-task mode. Note: autonomous mode is not faster *per task* — it runs the same test-driven loop — it only removes the human stepping *between* tasks. + +## Default: one task + +Pick the next pending task from the plan. Then: + +1. Read the task's acceptance criteria +2. Load relevant context (existing code, patterns, types) +3. Write a failing test for the expected behavior (RED) +4. Implement the minimum code to pass the test (GREEN) +5. Run the full test suite to check for regressions +6. Run the build to verify compilation +7. Commit with a descriptive message +8. Mark the task complete and stop + +## Autonomous: the whole plan (`/build auto`) + +Use this once a spec exists and you want to collapse plan + build into one run. It removes the manual stepping between tasks — **not** the verification. Every task still earns a passing test and its own commit. + +1. **Require a spec.** Look only for a spec at a known path: `SPEC.md` at the repo root, `docs/SPEC.md`, or a file under `spec/`. A README or arbitrary doc does **not** count. If none exists, stop and tell the user to run `/spec` first — do not invent requirements. +2. **Establish a clean baseline.** Run `git status --porcelain`. If there are uncommitted changes outside the expected planning artifacts (`SPEC.md`, `docs/SPEC.md`, `spec/*`, `tasks/plan.md`, `tasks/todo.md`), stop and ask the user to commit, stash, or confirm how to handle them. Autonomous per-task commits must not absorb unrelated local work, or the clean-rollback guarantee breaks. +3. **Plan if needed.** If there is no `tasks/plan.md`, invoke planning-and-task-breakdown to generate one. +4. **Single checkpoint.** Present the full plan and wait for an unambiguous affirmative (e.g. "approve", "go", "yes"). Treat hedged responses ("looks reasonable", "I guess") as **not** approved. This is the only human gate — after approval, run autonomously. If you generated `tasks/plan.md`, commit it as a single preparatory commit now so it doesn't bleed into the first task's commit. +5. **Execute every task in dependency order.** Use each task's declared dependencies; if they aren't explicit, execute in the order the plan lists them. For each task, run the full default loop above (RED → GREEN → regression → build → commit → mark complete). Stage only the files that task touched plus its task-status update — never `git add -A` blindly — and make one commit per task so any point is a clean rollback. +6. **Stop and ask the user** (do not push through) when: + - a test can't be made to pass or the build breaks without an obvious fix → follow debugging-and-error-recovery + - the spec is ambiguous, or a task needs a decision the spec doesn't cover + - a task is high-risk or irreversible — auth/permission changes, destructive data migrations, payments, deletions, deploys, anything touching secrets, **or anything you can't undo with `git revert`** → follow doubt-driven-development and get explicit sign-off before continuing + + After the user resolves a blocker, they re-invoke `/build auto` — it resumes from the next pending task. +7. **Summarize at the end:** tasks completed, tests added, commits made, and anything skipped, flagged, or left for the user. + +If any step fails, follow the debugging-and-error-recovery skill. + + diff --git a/cli-tool/templates/packs/commands/code-simplify.md b/cli-tool/templates/packs/commands/code-simplify.md new file mode 100644 index 0000000..e143851 --- /dev/null +++ b/cli-tool/templates/packs/commands/code-simplify.md @@ -0,0 +1,24 @@ +--- +description: Simplify code for clarity and maintainability — reduce complexity without changing behavior +--- + +Invoke the code-simplification skill. + +Simplify recently changed code (or the specified scope) while preserving exact behavior: + +1. Read the project's agent instructions (CLAUDE.md / AGENTS.md) and study project conventions +2. Identify the target code — recent changes unless a broader scope is specified +3. Understand the code's purpose, callers, edge cases, and test coverage before touching it +4. Scan for simplification opportunities: + - Deep nesting → guard clauses or extracted helpers + - Long functions → split by responsibility + - Nested ternaries → if/else or switch + - Generic names → descriptive names + - Duplicated logic → shared functions + - Dead code → remove after confirming +5. Apply each simplification incrementally — run tests after each change +6. Verify all tests pass, the build succeeds, and the diff is clean + +If tests fail after a simplification, revert that change and reconsider. Use `code-review-and-quality` to review the result. + + diff --git a/cli-tool/templates/packs/commands/constraints.md b/cli-tool/templates/packs/commands/constraints.md new file mode 100644 index 0000000..251fb81 --- /dev/null +++ b/cli-tool/templates/packs/commands/constraints.md @@ -0,0 +1,34 @@ +--- +description: Define and enforce this project's quality bar — interview, sane defaults, CONSTRAINTS.md +--- + +Invoke the constraint-driven-development skill. + +$ARGUMENTS + +Default behaviour with no arguments: set up constraints for this repository. + +1. **Detect first.** Read package.json / pyproject.toml / go.mod, the test runner, existing lint configs, current coverage output, CI workflows, and the agent harness in use. Report what you found in two lines. Never ask for anything you can read. + +2. **Interview, at most four questions.** One at a time, each with your best guess and a usable default so "I don't know" still produces a working config: + - Which dimensions beyond the floor (coverage, security, performance, accessibility, architecture) + - Block or warn when a check fails mid-task + - Target numbers, or measure today's values and hold them + - Slowest check tolerated before handing work back + +3. **Write CONSTRAINTS.md** at the repo root with a Floor section, enforced numbers, measured-only metrics with today's values, and an exceptions table with owners and expiry dates. Every number needs a stated reason. + +4. **Propose the tool each picked dimension needs; install nothing yourself.** A dimension with a number and no tool behind it is an aspiration. Present the exact install commands (pinned versions) and wait for the user's approval before anything is installed. Use the de facto tool so existing config keeps working: Semgrep for code scanning, gitleaks (always `--redact`) for secrets, osv-scanner for dependencies, axe-core for accessibility, Lighthouse for web vitals, size-limit for bundles, dependency-cruiser for boundaries, Stryker for assertion quality. Record the exact command next to each rule in CONSTRAINTS.md. Accessibility and performance need a running URL; if the project has none, say so and drop the dimension rather than inventing a check. Add the commands to package.json as check:fast / check:task / check:full. Tools that contact online services when they run (Semgrep registry rule packs, osv-scanner's vulnerability API, Lighthouse/axe against a deployed URL) go into CI only unless the user explicitly wants them locally; local checks must run offline. + +5. **Place each check by cost.** Types, lint and secrets in the edit loop (seconds). Related tests and changed-line coverage at task end (under 90s). Everything else at review or in CI. Scope checks to the diff, not the whole repo. + +6. **Point the agent at it.** Propose a line for the project's agent instructions (CLAUDE.md / AGENTS.md) telling agents to read CONSTRAINTS.md and never weaken it to make a change pass; add it only after the user agrees. + +7. **Verify.** Run the locally installed constraints against the current branch. If anything fails that the user disagrees with, fix the constraint now rather than leaving a gate people will learn to ignore. + +Sub-commands: +- `/constraints check` — run the current constraints against this branch and report +- `/constraints guard` — inspect the diff for a weakened bar: lowered thresholds, skipped or deleted tests, new suppression comments, unfinished stubs, new exceptions +- `/constraints ratchet` — record today's measured values as the floor that must not fall + + diff --git a/cli-tool/templates/packs/commands/plan.md b/cli-tool/templates/packs/commands/plan.md new file mode 100644 index 0000000..a5dda87 --- /dev/null +++ b/cli-tool/templates/packs/commands/plan.md @@ -0,0 +1,20 @@ +--- +description: Break work into small verifiable tasks with acceptance criteria and dependency ordering +--- + +Invoke the planning-and-task-breakdown skill. + +Read the existing spec (SPEC.md or equivalent) and the relevant codebase sections. Then: + +1. Enter plan mode — read only, no code changes +2. Identify the dependency graph between components +3. Slice work vertically (one complete path per task, not horizontal layers) +4. Write tasks with acceptance criteria and verification steps +5. Add checkpoints between phases +6. Present the plan for human review + +Save the plan to tasks/plan.md and task list to tasks/todo.md. + +If tasks/plan.md or tasks/todo.md already exists with unchecked tasks for different work, stop and ask before writing — never silently overwrite an incomplete plan. + + diff --git a/cli-tool/templates/packs/commands/review.md b/cli-tool/templates/packs/commands/review.md new file mode 100644 index 0000000..f97ec84 --- /dev/null +++ b/cli-tool/templates/packs/commands/review.md @@ -0,0 +1,18 @@ +--- +description: Conduct a five-axis code review — correctness, readability, architecture, security, performance +--- + +Invoke the code-review-and-quality skill. + +Review the current changes (staged or recent commits) across all five axes: + +1. **Correctness** — Does it match the spec? Edge cases handled? Tests adequate? +2. **Readability** — Clear names? Straightforward logic? Well-organized? +3. **Architecture** — Follows existing patterns? Clean boundaries? Right abstraction level? +4. **Security** — Input validated? Secrets safe? Auth checked? (Use security-and-hardening skill) +5. **Performance** — No N+1 queries? No unbounded ops? (Use performance-optimization skill) + +Categorize findings as Critical, Important, or Suggestion. +Output a structured review with specific file:line references and fix recommendations. + + diff --git a/cli-tool/templates/packs/commands/ship.md b/cli-tool/templates/packs/commands/ship.md new file mode 100644 index 0000000..1c98db0 --- /dev/null +++ b/cli-tool/templates/packs/commands/ship.md @@ -0,0 +1,69 @@ +--- +description: Run the pre-launch checklist via parallel fan-out to three specialist reviews, then synthesize a go/no-go decision +--- + +Invoke the shipping-and-launch skill. + +`/ship` is a **fan-out orchestrator**. It runs three specialist reviews in parallel against the current change, then merges their reports into a single go/no-go decision with a rollback plan. The reviews operate independently — no shared state, no ordering — which is what makes parallel execution safe and useful here. + +`/ship` decides and documents. It does not deploy, push, run migrations or flip feature flags; those happen only when the user explicitly instructs it. + +## Phase A — Parallel fan-out + +If the harness can spawn subagents, start three in a single turn so they run in parallel. Use the user's own reviewer agents if they exist (e.g. `code-reviewer`, `security-auditor`, `test-engineer`); otherwise use general-purpose subagents with these briefs: + +1. **Code review** — load the `code-review-and-quality` skill. Five-axis review (correctness, readability, architecture, security, performance) of the staged changes or recent commits. Output Critical / Important / Suggestion findings with file:line. +2. **Security audit** — load the `security-and-hardening` skill. Vulnerability and threat-model pass: OWASP Top 10, secrets handling, auth/authz, dependency risk (from the lockfile and existing CI results; no network audits without the user's consent). Output findings by severity with file:line. +3. **Test coverage** — load the `test-driven-development` skill. Coverage analysis for the change: happy path, edge cases, error paths, concurrency. Output gaps with suggested tests. + +Subagents must not spawn further subagents. Each returns only its report. + +Without subagent support, run the three reviews sequentially in the main session and treat the outputs as if returned in parallel — the merge phase still works. + +## Phase B — Merge in main context + +Once all three reports are back, the main agent synthesizes them: + +1. **Code Quality** — Aggregate Critical/Important findings and any failing tests, lint, or build output. Resolve duplicates between reviewers. +2. **Security** — Promote any Critical/High security findings to launch blockers. Cross-reference with the code review's security axis. +3. **Performance** — Pull from the code review's performance axis; cross-check Core Web Vitals if applicable (local measurements only). +4. **Accessibility** — Verify keyboard nav, screen reader support, contrast (use the shipping-and-launch accessibility checklist). +5. **Infrastructure** — Env vars, migrations, monitoring, feature flags. Verify by reading config; change nothing. +6. **Documentation** — README, ADRs, changelog. Verify directly. + +## Phase C — Decision and rollback + +Produce a single output: + +```markdown +## Ship Decision: GO | NO-GO + +### Blockers (must fix before ship) +- [Source review: Critical finding + file:line] + +### Recommended fixes (should fix before ship) +- [Source review: Important finding + file:line] + +### Acknowledged risks (shipping anyway) +- [Risk + mitigation] + +### Rollback plan +- Trigger conditions: [what signals would prompt rollback] +- Rollback procedure: [exact steps, executed by the user or on the user's explicit instruction] +- Recovery time objective: [target] + +### Specialist reports (full) +- [code review report] +- [security audit report] +- [test coverage report] +``` + +## Rules + +1. The three Phase A reviews run in parallel when the harness supports it. +2. Reviews do not call each other. The main agent merges in Phase B. +3. The rollback plan is mandatory before any GO decision. +4. If any review returns a Critical finding, the default verdict is NO-GO unless the user explicitly accepts the risk. +5. **Skip the fan-out only if all of the following are true:** the change touches 2 files or fewer, the diff is under 50 lines, and it does not touch auth, payments, data access, or config/env. Otherwise, default to fan-out. + + diff --git a/cli-tool/templates/packs/commands/spec.md b/cli-tool/templates/packs/commands/spec.md new file mode 100644 index 0000000..640a37e --- /dev/null +++ b/cli-tool/templates/packs/commands/spec.md @@ -0,0 +1,19 @@ +--- +description: Start spec-driven development — write a structured specification before writing code +--- + +Invoke the spec-driven-development skill. + +Begin by understanding what the user wants to build. Ask clarifying questions about: +1. The objective and target users +2. Core features and acceptance criteria +3. Tech stack preferences and constraints +4. Known boundaries (what to always do, ask first about, and never do) + +Then generate a structured spec covering all six core areas: objective, commands, project structure, code style, testing strategy, and boundaries. + +If the request bundles several independently testable capabilities, first propose a capability map (module ids, dependency direction, build order) per the skill's Phase 0 and get it approved, then spec each module in dependency order. + +Save the spec as SPEC.md in the project root and confirm with the user before proceeding. + + diff --git a/cli-tool/templates/packs/commands/test.md b/cli-tool/templates/packs/commands/test.md new file mode 100644 index 0000000..63f1876 --- /dev/null +++ b/cli-tool/templates/packs/commands/test.md @@ -0,0 +1,21 @@ +--- +description: Run TDD workflow — write failing tests, implement, verify. For bugs, use the Prove-It pattern. +--- + +Invoke the test-driven-development skill. + +For new features: +1. Write tests that describe the expected behavior (they should FAIL) +2. Implement the code to make them pass +3. Refactor while keeping tests green + +For bug fixes (Prove-It pattern): +1. Write a test that reproduces the bug (must FAIL) +2. Confirm the test fails +3. Implement the fix +4. Confirm the test passes +5. Run the full test suite for regressions + +For browser-related issues, also verify in a real browser with the `playwright-cli` skill if it is installed (local pages only). + + diff --git a/cli-tool/templates/packs/commands/webperf.md b/cli-tool/templates/packs/commands/webperf.md new file mode 100644 index 0000000..08f5300 --- /dev/null +++ b/cli-tool/templates/packs/commands/webperf.md @@ -0,0 +1,33 @@ +--- +description: Run a local web performance audit (static source review plus any local measurement artifacts) +--- + +Invoke the performance-optimization skill. + +`/webperf` targets web applications specifically. Do not use it for utility libraries, CLIs, or server-only code with no browser-facing output. + +$ARGUMENTS + +## Determine the mode + +**Deep mode** — only from local material the user provides or that already exists in the project: +- A Lighthouse JSON report file the user produced (for example with a project-installed Lighthouse: `npx --no-install lighthouse http://localhost: --output json --output-path ./report.json`) +- A DevTools performance trace or HAR file exported by the user +- Local measurements from the `playwright-cli` skill against the local dev server (screenshots, console, requests) + +Do not call online services (PageSpeed Insights, CrUX or other APIs), do not install Lighthouse or DevTools tooling, and do not add MCP servers. If deep mode would need any of that, say what the user could run locally and continue in quick mode. + +**Quick mode** — default when none of the above is available. Scan the source for structural anti-patterns and label every finding as `potential impact`. + +## Run the audit + +Review the files, components, or diff under review (or the scope the user named), using the skill's checklist (`references/performance-checklist.md` in the performance-optimization skill). For deep mode, read the provided artifacts and cite only values that are actually in them. + +## Output + +- Scorecard with only sourced values (artifact + metric), never estimates presented as measurements +- Ranked findings: impact, location (file:line), fix +- Positive observations +- Recommendations, including CI budgets the team could add + + diff --git a/cli-tool/templates/packs/skills/api-and-interface-design/LICENSE b/cli-tool/templates/packs/skills/api-and-interface-design/LICENSE new file mode 100644 index 0000000..d67778a --- /dev/null +++ b/cli-tool/templates/packs/skills/api-and-interface-design/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2025 Addy Osmani + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/cli-tool/templates/packs/skills/api-and-interface-design/SKILL.md b/cli-tool/templates/packs/skills/api-and-interface-design/SKILL.md new file mode 100644 index 0000000..3e499e7 --- /dev/null +++ b/cli-tool/templates/packs/skills/api-and-interface-design/SKILL.md @@ -0,0 +1,374 @@ +--- +name: api-and-interface-design +description: Guides stable API and interface design. Use when designing APIs, module boundaries, or any public interface. Use when creating REST or GraphQL endpoints, defining type contracts between modules, or establishing boundaries between frontend and backend. +--- + +> Adapted copy of addyosmani/agent-skills (MIT) for local use; changes are listed in `UPSTREAM.md`. +> Ground rules that override anything below: use only tools that are already installed in the project +> (project scripts, `npx --no-install ...`); never let npx/pip/brew download or install anything and never add +> MCP servers on your own; do not fetch web pages or online docs and do not upload or post data anywhere; +> never push, deploy, run migrations, flip feature flags or change shared systems without the user's explicit +> instruction. Commands inside CI/workflow examples are file content to write, not commands to run now. + +# API and Interface Design + +## Overview + +Design stable, well-documented interfaces that are hard to misuse. Good interfaces make the right thing easy and the wrong thing hard. This applies to REST APIs, GraphQL schemas, module boundaries, component props, and any surface where one piece of code talks to another. + +## When to Use + +- Designing new API endpoints +- Defining module boundaries or contracts between teams +- Creating component prop interfaces +- Establishing database schema that informs API shape +- Changing existing public interfaces + +## Core Principles + +### Hyrum's Law + +> With a sufficient number of users of an API, all observable behaviors of your system will be depended on by somebody, regardless of what you promise in the contract. + +This means: every public behavior — including undocumented quirks, error message text, timing, and ordering — becomes a de facto contract once users depend on it. Design implications: + +- **Be intentional about what you expose.** Every observable behavior is a potential commitment. +- **Don't leak implementation details.** If users can observe it, they will depend on it. +- **Plan for deprecation at design time.** See `deprecation-and-migration` for how to safely remove things users depend on. +- **Tests are not enough.** Even with perfect contract tests, Hyrum's Law means "safe" changes can break real users who depend on undocumented behavior. + +### The One-Version Rule + +Avoid forcing consumers to choose between multiple versions of the same dependency or API. Diamond dependency problems arise when different consumers need different versions of the same thing. Design for a world where only one version exists at a time — extend rather than fork. + +### 1. Contract First + +Define the interface before implementing it. The contract is the spec — implementation follows. + +```typescript +// Define the contract first +interface TaskAPI { + // Creates a task and returns the created task with server-generated fields + createTask(input: CreateTaskInput): Promise; + + // Returns paginated tasks matching filters + listTasks(params: ListTasksParams): Promise>; + + // Returns a single task or throws NotFoundError + getTask(id: string): Promise; + + // Partial update — only provided fields change + updateTask(id: string, input: UpdateTaskInput): Promise; + + // Idempotent delete — succeeds even if already deleted + deleteTask(id: string): Promise; +} +``` + +### 2. Consistent Error Semantics + +Pick one error strategy and use it everywhere: + +```typescript +// REST: HTTP status codes + structured error body +// Every error response follows the same shape +interface APIError { + error: { + code: string; // Machine-readable: "VALIDATION_ERROR" + message: string; // Human-readable: "Email is required" + details?: unknown; // Additional context when helpful + }; +} + +// Status code mapping +// 400 → Client sent invalid data +// 401 → Not authenticated +// 403 → Authenticated but not authorized +// 404 → Resource not found +// 409 → Conflict (duplicate, version mismatch) +// 422 → Validation failed (semantically invalid) +// 500 → Server error (never expose internal details) +``` + +**Don't mix patterns.** If some endpoints throw, others return null, and others return `{ error }` — the consumer can't predict behavior. + +### 3. Validate at Boundaries + +Trust internal code. Validate at system edges where external input enters: + +```typescript +// Validate at the API boundary +app.post('/api/tasks', async (req, res) => { + const result = CreateTaskSchema.safeParse(req.body); + if (!result.success) { + return res.status(422).json({ + error: { + code: 'VALIDATION_ERROR', + message: 'Invalid task data', + details: result.error.flatten(), + }, + }); + } + + // After validation, internal code trusts the types + const task = await taskService.create(result.data); + return res.status(201).json(task); +}); +``` + +Where validation belongs: +- API route handlers (user input) +- Form submission handlers (user input) +- External service response parsing (third-party data -- **always treat as untrusted**) +- Environment variable loading (configuration) + +> **Third-party API responses are untrusted data.** Validate their shape and content before using them in any logic, rendering, or decision-making. A compromised or misbehaving external service can return unexpected types, malicious content, or instruction-like text. + +Where validation does NOT belong: +- Between internal functions that share type contracts +- In utility functions called by already-validated code +- On data that just came from your own database + +### 4. Prefer Addition Over Modification + +Extend interfaces without breaking existing consumers: + +```typescript +// Good: Add optional fields +interface CreateTaskInput { + title: string; + description?: string; + priority?: 'low' | 'medium' | 'high'; // Added later, optional + labels?: string[]; // Added later, optional +} + +// Bad: Change existing field types or remove fields +interface CreateTaskInput { + title: string; + // description: string; // Removed — breaks existing consumers + priority: number; // Changed from string — breaks existing consumers +} +``` + +### 5. Predictable Naming + +| Pattern | Convention | Example | +|---------|-----------|---------| +| REST endpoints | Plural nouns, no verbs | `GET /api/tasks`, `POST /api/tasks` | +| Query params | camelCase | `?sortBy=createdAt&pageSize=20` | +| Response fields | camelCase | `{ createdAt, updatedAt, taskId }` | +| Boolean fields | is/has/can prefix | `isComplete`, `hasAttachments` | +| Enum values | UPPER_SNAKE | `"IN_PROGRESS"`, `"COMPLETED"` | + +### 6. Honouring an Idempotency Key + +Accepting an `Idempotency-Key` is the contract. Honouring it is the implementation, and it is where the money is lost — a key the server accepts but handles carelessly is worse than no key at all, because the client now believes retrying is safe. + +**Derive the key from the intent, not the attempt.** The key must be stable across retries of one intent and different across distinct intents: + +```typescript +crypto.randomUUID() // ✗ new key per attempt — every retry is a new charge +`${userId}:${amount}` // ✗ two legitimate $50 charges collapse into one +`${orderId}:${Date.now()}` // ✗ a timestamp is randomUUID() wearing a hat + +req.headers['idempotency-key'] // ✓ client generates once, reuses on retry +`charge:v1:${orderId}` // ✓ derived from an immutable identifier +``` + +The key comes from the client or the initiating event — never from the layer doing the retrying. + +**Claim atomically. A check followed by an act is a race:** + +```typescript +// ✗ TOCTOU: two concurrent retries both read "not seen", both charge +if (!(await db.exists(key))) { + await chargeCard(amount); + await db.insert(key); +} + +// ✓ let the unique constraint pick the winner +try { + await db.insert({ key, state: 'in_progress', requestHash }); +} catch (e) { + if (isUniqueViolation(e)) return replayOrReject(key); + throw e; +} +const result = await chargeCard(amount); +await db.update({ key, state: 'succeeded', response: result }); +``` + +The unique constraint *is* the mechanism. A store that cannot enforce uniqueness in one operation cannot back this. + +**Guard the payload.** Same key with a different body is a client bug, and must fail loudly rather than serving the first response to a second request: + +```typescript +if (existing.requestHash !== hash(req.body)) { + return res.status(422).json({ error: 'idempotency key reused with a different payload' }); +} +``` + +**Decide what an in-flight duplicate gets.** The first request is still running when the second arrives — the common case under retry storms: + +| Strategy | Response | Use when | +|---|---|---| +| Reject | `409 Conflict` | Client can retry later; simplest and safest | +| Wait | Block for the result, bounded | Caller needs it synchronously | +| Return pending | `202` + status URL | Long-running effects | + +Never let the second caller through because the first "seems stuck". A stalled attempt whose fate is unknown is exactly when duplicating costs most. + +**Every call has three outcomes, not two: success, failure, and _unknown_.** A timeout tells you nothing about whether the effect applied. Record the intent *before* calling out, so a crash between the call and the response leaves evidence something must resolve later — rather than a silently retried charge. + +**Set retention from the longest retry chain**, not from disk cost. Keys must outlive every path that can re-deliver the same intent, including a dead-letter queue replayed a week later and any provider dispute window. A 24-hour key TTL behind a 7-day DLQ is a duplicate waiting to happen. + +## REST API Patterns + +### Resource Design + +``` +GET /api/tasks → List tasks (with query params for filtering) +POST /api/tasks → Create a task +GET /api/tasks/:id → Get a single task +PATCH /api/tasks/:id → Update a task (partial) +DELETE /api/tasks/:id → Delete a task + +GET /api/tasks/:id/comments → List comments for a task (sub-resource) +POST /api/tasks/:id/comments → Add a comment to a task +``` + +### Pagination + +Paginate list endpoints: + +```typescript +// Request +GET /api/tasks?page=1&pageSize=20&sortBy=createdAt&sortOrder=desc + +// Response +{ + "data": [...], + "pagination": { + "page": 1, + "pageSize": 20, + "totalItems": 142, + "totalPages": 8 + } +} +``` + +### Filtering + +Use query parameters for filters: + +``` +GET /api/tasks?status=in_progress&assignee=user123&createdAfter=2025-01-01 +``` + +### Partial Updates (PATCH) + +Accept partial objects — only update what's provided: + +```typescript +// Only title changes, everything else preserved +PATCH /api/tasks/123 +{ "title": "Updated title" } +``` + +## TypeScript Interface Patterns + +### Use Discriminated Unions for Variants + +```typescript +// Good: Each variant is explicit +type TaskStatus = + | { type: 'pending' } + | { type: 'in_progress'; assignee: string; startedAt: Date } + | { type: 'completed'; completedAt: Date; completedBy: string } + | { type: 'cancelled'; reason: string; cancelledAt: Date }; + +// Consumer gets type narrowing +function getStatusLabel(status: TaskStatus): string { + switch (status.type) { + case 'pending': return 'Pending'; + case 'in_progress': return `In progress (${status.assignee})`; + case 'completed': return `Done on ${status.completedAt}`; + case 'cancelled': return `Cancelled: ${status.reason}`; + } +} +``` + +### Input/Output Separation + +```typescript +// Input: what the caller provides +interface CreateTaskInput { + title: string; + description?: string; +} + +// Output: what the system returns (includes server-generated fields) +interface Task { + id: string; + title: string; + description: string | null; + createdAt: Date; + updatedAt: Date; + createdBy: string; +} +``` + +### Use Branded Types for IDs + +```typescript +type TaskId = string & { readonly __brand: 'TaskId' }; +type UserId = string & { readonly __brand: 'UserId' }; + +// Prevents accidentally passing a UserId where a TaskId is expected +function getTask(id: TaskId): Promise { ... } +``` + +## Common Rationalizations + +| Rationalization | Reality | +|---|---| +| "We'll document the API later" | The types ARE the documentation. Define them first. | +| "We don't need pagination for now" | You will the moment someone has 100+ items. Add it from the start. | +| "PATCH is complicated, let's just use PUT" | PUT requires the full object every time. PATCH is what clients actually want. | +| "We'll version the API when we need to" | Breaking changes without versioning break consumers. Design for extension from the start. | +| "Nobody uses that undocumented behavior" | Hyrum's Law: if it's observable, somebody depends on it. Treat every public behavior as a commitment. | +| "We can just maintain two versions" | Multiple versions multiply maintenance cost and create diamond dependency problems. Prefer the One-Version Rule. | +| "Internal APIs don't need contracts" | Internal consumers are still consumers. Contracts prevent coupling and enable parallel work. | +| "Accepting the Idempotency-Key header is enough" | The header is the contract; storing the key against the result is the implementation. A key you accept but don't honour tells the client retrying is safe when it isn't. | +| "Our queue guarantees exactly-once delivery" | No queue does across a consumer crash — the broker's ack and your side effect are not in one transaction. Design for at-least-once with idempotent processing. | +| "Duplicate requests are rare" | They're *correlated*. Retries spike exactly when a dependency is degraded — the moment duplicates are most likely and most expensive. | + +## Red Flags + +- Endpoints that return different shapes depending on conditions +- Inconsistent error formats across endpoints +- Validation scattered throughout internal code instead of at boundaries +- Breaking changes to existing fields (type changes, removals) +- List endpoints without pagination +- Verbs in REST URLs (`/api/createTask`, `/api/getUsers`) +- Third-party API responses used without validation or sanitization +- A `SELECT` for an idempotency key followed by an `INSERT` — that's a race, not a guard +- An idempotency key derived from a UUID, timestamp, or anything else regenerated per attempt +- The same key accepted with a different request body, silently returning the first response +- A key retention window shorter than the longest path that can re-deliver the request + +## Verification + +After designing an API: + +- [ ] Every endpoint has typed input and output schemas +- [ ] Error responses follow a single consistent format +- [ ] Validation happens at system boundaries only +- [ ] List endpoints support pagination +- [ ] New fields are additive and optional (backward compatible) +- [ ] Naming follows consistent conventions across all endpoints +- [ ] API documentation or types are committed alongside the implementation +- [ ] State-changing endpoints either honour an idempotency key or are documented as unsafe to retry +- [ ] The key is claimed in one atomic operation, guarded by a unique constraint +- [ ] A reused key with a different payload fails loudly rather than replaying the wrong response +- [ ] The in-flight-duplicate response is a deliberate choice (409, wait, or 202) rather than whatever falls out +- [ ] Key retention outlives the longest retry path, including dead-letter replay diff --git a/cli-tool/templates/packs/skills/api-and-interface-design/UPSTREAM.md b/cli-tool/templates/packs/skills/api-and-interface-design/UPSTREAM.md new file mode 100644 index 0000000..83907b6 --- /dev/null +++ b/cli-tool/templates/packs/skills/api-and-interface-design/UPSTREAM.md @@ -0,0 +1,22 @@ +# Upstream and license + +This folder is an adapted copy of `skills/api-and-interface-design/` from an MIT-licensed upstream repository. + +| Field | Value | +|---|---| +| Source repository | https://github.com/addyosmani/agent-skills | +| Path | `skills/api-and-interface-design/` | +| Pinned commit | `a06bc63b3f8b829c14b0bbf53d99fefc39d58092` | +| Retrieved | 2026-10-03 | +| Upstream SKILL.md sha256 | `e83d34f52ff12dd8a7c5134cd7e8140ea4541ec65e204f8a1e4cd366e30c4192` | +| License | MIT License, Copyright (c) 2025 Addy Osmani (full text: `LICENSE`) | + +## Changes made in this copy + +- Added the local-use ground rules block under the front matter. + +Everything else is upstream text, unchanged. + +## Updating + +Review `git diff a06bc63b3f8b829c14b0bbf53d99fefc39d58092 -- skills/api-and-interface-design/ references/`, re-run `packs/adapt-agent-skills.py ` (it applies the changes above as exact replacements and fails if upstream text moved), update commit and sha256 here and in `packs/sources.lock.json`, then run `packs/check-vendored.sh --online`. diff --git a/cli-tool/templates/packs/skills/ci-cd-and-automation/LICENSE b/cli-tool/templates/packs/skills/ci-cd-and-automation/LICENSE new file mode 100644 index 0000000..d67778a --- /dev/null +++ b/cli-tool/templates/packs/skills/ci-cd-and-automation/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2025 Addy Osmani + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/cli-tool/templates/packs/skills/ci-cd-and-automation/SKILL.md b/cli-tool/templates/packs/skills/ci-cd-and-automation/SKILL.md new file mode 100644 index 0000000..e0541c0 --- /dev/null +++ b/cli-tool/templates/packs/skills/ci-cd-and-automation/SKILL.md @@ -0,0 +1,397 @@ +--- +name: ci-cd-and-automation +description: Automates CI/CD pipeline setup. Use when setting up or modifying build and deployment pipelines. Use when you need to automate quality gates, configure test runners in CI, or establish deployment strategies. +--- + +> Adapted copy of addyosmani/agent-skills (MIT) for local use; changes are listed in `UPSTREAM.md`. +> Ground rules that override anything below: use only tools that are already installed in the project +> (project scripts, `npx --no-install ...`); never let npx/pip/brew download or install anything and never add +> MCP servers on your own; do not fetch web pages or online docs and do not upload or post data anywhere; +> never push, deploy, run migrations, flip feature flags or change shared systems without the user's explicit +> instruction. Commands inside CI/workflow examples are file content to write, not commands to run now. + +# CI/CD and Automation + +## Overview + +Automate quality gates so that no change reaches production without passing tests, lint, type checking, and build. CI/CD is the enforcement mechanism for every other skill — it catches what humans and agents miss, and it does so consistently on every single change. + +**Shift Left:** Catch problems as early in the pipeline as possible. A bug caught in linting costs minutes; the same bug caught in production costs hours. Move checks upstream — static analysis before tests, tests before staging, staging before production. + +**Faster is Safer:** Smaller batches and more frequent releases reduce risk, not increase it. A deployment with 3 changes is easier to debug than one with 30. Frequent releases build confidence in the release process itself. + +## When to Use + +- Setting up a new project's CI pipeline +- Adding or modifying automated checks +- Configuring deployment pipelines +- When a change should trigger automated verification +- Debugging CI failures + +## The Quality Gate Pipeline + +Every change goes through these gates before merge: + +``` +Pull Request Opened + │ + ▼ +┌─────────────────┐ +│ LINT CHECK │ eslint, prettier +│ ↓ pass │ +│ TYPE CHECK │ tsc --noEmit +│ ↓ pass │ +│ UNIT TESTS │ jest/vitest +│ ↓ pass │ +│ BUILD │ npm run build +│ ↓ pass │ +│ INTEGRATION │ API/DB tests +│ ↓ pass │ +│ E2E (optional) │ Playwright/Cypress +│ ↓ pass │ +│ SECURITY AUDIT │ npm audit +│ ↓ pass │ +│ BUNDLE SIZE │ bundlesize check +└─────────────────┘ + │ + ▼ + Ready for review +``` + +**No gate can be skipped.** If lint fails, fix lint — don't disable the rule. If a test fails, fix the code — don't skip the test. + +## GitHub Actions Configuration + +### Basic CI Pipeline + +```yaml +# .github/workflows/ci.yml +name: CI + +on: + pull_request: + branches: [main] + push: + branches: [main] + +jobs: + quality: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: '22' + cache: 'npm' + + - name: Install dependencies + run: npm ci + + - name: Lint + run: npm run lint + + - name: Type check + run: npx tsc --noEmit + + - name: Test + run: npm test -- --coverage + + - name: Build + run: npm run build + + - name: Security audit + run: npm audit --audit-level=high +``` + +### With Database Integration Tests + +```yaml + integration: + runs-on: ubuntu-latest + services: + postgres: + image: postgres:16 + env: + POSTGRES_DB: testdb + POSTGRES_USER: ci_user + POSTGRES_PASSWORD: ${{ secrets.CI_DB_PASSWORD }} + ports: + - 5432:5432 + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: '22' + cache: 'npm' + - run: npm ci + - name: Run migrations + run: npx prisma migrate deploy + env: + DATABASE_URL: postgresql://ci_user:${{ secrets.CI_DB_PASSWORD }}@localhost:5432/testdb + - name: Integration tests + run: npm run test:integration + env: + DATABASE_URL: postgresql://ci_user:${{ secrets.CI_DB_PASSWORD }}@localhost:5432/testdb +``` + +> **Note:** Even for CI-only test databases, use GitHub Secrets for credentials rather than hardcoding values. This builds good habits and prevents accidental reuse of test credentials in other contexts. + +### E2E Tests + +```yaml + e2e: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: '22' + cache: 'npm' + - run: npm ci + - name: Install Playwright + run: npx playwright install --with-deps chromium + - name: Build + run: npm run build + - name: Run E2E tests + run: npx playwright test + - uses: actions/upload-artifact@v4 + if: failure() + with: + name: playwright-report + path: playwright-report/ +``` + +## Feeding CI Failures Back to Agents + +The power of CI with AI agents is the feedback loop. When CI fails: + +``` +CI fails + │ + ▼ +Copy the failure output + │ + ▼ +Feed it to the agent: +"The CI pipeline failed with this error: +[paste specific error] +Fix the issue and verify locally before pushing again." + │ + ▼ +Agent fixes → user confirms the push → CI runs again +``` + +**Key patterns:** + +``` +Lint failure → Agent runs `npm run lint --fix` and commits locally (push only when the user confirms) +Type error → Agent reads the error location and fixes the type +Test failure → Agent follows debugging-and-error-recovery skill +Build error → Agent checks config and dependencies +``` + +## Deployment Strategies + +### Preview Deployments + +Every PR gets a preview deployment for manual testing: + +```yaml +# Deploy preview on PR (Vercel/Netlify/etc.) +deploy-preview: + runs-on: ubuntu-latest + if: github.event_name == 'pull_request' + steps: + - uses: actions/checkout@v4 + - name: Deploy preview + run: npx vercel --token=${{ secrets.VERCEL_TOKEN }} +``` + +### Feature Flags + +Feature flags decouple deployment from release. Deploy incomplete or risky features behind flags so you can: + +- **Ship code without enabling it.** Merge to main early, enable when ready. +- **Roll back without redeploying.** Disable the flag instead of reverting code. +- **Canary new features.** Enable for 1% of users, then 10%, then 100%. +- **Run A/B tests.** Compare behavior with and without the feature. + +```typescript +// Simple feature flag pattern +if (featureFlags.isEnabled('new-checkout-flow', { userId })) { + return renderNewCheckout(); +} +return renderLegacyCheckout(); +``` + +**Flag lifecycle:** Create → Enable for testing → Canary → Full rollout → Remove the flag and dead code. Flags that live forever become technical debt — set a cleanup date when you create them. + +### Staged Rollouts + +``` +PR merged to main + │ + ▼ + Staging deployment (auto) + │ Manual verification + ▼ + Production deployment (manual trigger or auto after staging) + │ + ▼ + Monitor for errors (15-minute window) + │ + ├── Errors detected → Rollback + └── Clean → Done +``` + +### Rollback Plan + +Every deployment should be reversible: + +```yaml +# Manual rollback workflow +name: Rollback +on: + workflow_dispatch: + inputs: + version: + description: 'Version to rollback to' + required: true + +jobs: + rollback: + runs-on: ubuntu-latest + steps: + - name: Rollback deployment + run: | + # Deploy the specified previous version + npx vercel rollback ${{ inputs.version }} +``` + +## Environment Management + +``` +.env.example → Committed (template for developers) +.env → NOT committed (local development) +.env.test → Committed (test environment, no real secrets) +CI secrets → Stored in GitHub Secrets / vault +Production secrets → Stored in deployment platform / vault +``` + +CI should never have production secrets. Use separate secrets for CI testing. + +## Automation Beyond CI + +### Dependabot / Renovate + +```yaml +# .github/dependabot.yml +version: 2 +updates: + - package-ecosystem: npm + directory: / + schedule: + interval: weekly + open-pull-requests-limit: 5 +``` + +### Build Cop Role + +Designate someone responsible for keeping CI green. When the build breaks, the Build Cop's job is to fix or revert — not the person whose change caused the break. This prevents broken builds from accumulating while everyone assumes someone else will fix it. + +### PR Checks + +- **Required reviews:** At least 1 approval before merge +- **Required status checks:** CI must pass before merge +- **Branch protection:** No force-pushes to main +- **Auto-merge:** Only if the team has decided to enable it; the agent does not enable or trigger merges on its own + +## CI Optimization + +When the pipeline exceeds 10 minutes, apply these strategies in order of impact: + +``` +Slow CI pipeline? +├── Cache dependencies +│ └── Use actions/cache or setup-node cache option for node_modules +├── Run jobs in parallel +│ └── Split lint, typecheck, test, build into separate parallel jobs +├── Only run what changed +│ └── Use path filters to skip unrelated jobs (e.g., skip e2e for docs-only PRs) +├── Use matrix builds +│ └── Shard test suites across multiple runners +├── Optimize the test suite +│ └── Remove slow tests from the critical path, run them on a schedule instead +└── Use larger runners + └── GitHub-hosted larger runners or self-hosted for CPU-heavy builds +``` + +**Example: caching and parallelism** +```yaml +jobs: + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: { node-version: '22', cache: 'npm' } + - run: npm ci + - run: npm run lint + + typecheck: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: { node-version: '22', cache: 'npm' } + - run: npm ci + - run: npx tsc --noEmit + + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: { node-version: '22', cache: 'npm' } + - run: npm ci + - run: npm test -- --coverage +``` + +## Common Rationalizations + +| Rationalization | Reality | +|---|---| +| "CI is too slow" | Optimize the pipeline (see CI Optimization below), don't skip it. A 5-minute pipeline prevents hours of debugging. | +| "This change is trivial, skip CI" | Trivial changes break builds. CI is fast for trivial changes anyway. | +| "The test is flaky, just re-run" | Flaky tests mask real bugs and waste everyone's time. Fix the flakiness. | +| "We'll add CI later" | Projects without CI accumulate broken states. Set it up on day one. | +| "Manual testing is enough" | Manual testing doesn't scale and isn't repeatable. Automate what you can. | + +## Red Flags + +- No CI pipeline in the project +- CI failures ignored or silenced +- Tests disabled in CI to make the pipeline pass +- Production deploys without staging verification +- No rollback mechanism +- Secrets stored in code or CI config files (not secrets manager) +- Long CI times with no optimization effort + +## Verification + +After setting up or modifying CI: + +- [ ] All quality gates are present (lint, types, tests, build, audit) +- [ ] Pipeline runs on every PR and push to main +- [ ] Failures block merge (branch protection configured) +- [ ] CI results feed back into the development loop +- [ ] Secrets are stored in the secrets manager, not in code +- [ ] Deployment has a rollback mechanism +- [ ] Pipeline runs in under 10 minutes for the test suite diff --git a/cli-tool/templates/packs/skills/ci-cd-and-automation/UPSTREAM.md b/cli-tool/templates/packs/skills/ci-cd-and-automation/UPSTREAM.md new file mode 100644 index 0000000..532a713 --- /dev/null +++ b/cli-tool/templates/packs/skills/ci-cd-and-automation/UPSTREAM.md @@ -0,0 +1,25 @@ +# Upstream and license + +This folder is an adapted copy of `skills/ci-cd-and-automation/` from an MIT-licensed upstream repository. + +| Field | Value | +|---|---| +| Source repository | https://github.com/addyosmani/agent-skills | +| Path | `skills/ci-cd-and-automation/` | +| Pinned commit | `a06bc63b3f8b829c14b0bbf53d99fefc39d58092` | +| Retrieved | 2026-10-03 | +| Upstream SKILL.md sha256 | `a6ed8ed56456b01ff8314c44eefc69897d9905ae4e06bb2b7036286efb79b5f0` | +| License | MIT License, Copyright (c) 2025 Addy Osmani (full text: `LICENSE`) | + +## Changes made in this copy + +- Added the local-use ground rules block under the front matter. +- Pushing a fix now requires the user's confirmation. +- Commits stay local unless the user confirms a push. +- Auto-merge is a team decision, not an agent action. + +Everything else is upstream text, unchanged. + +## Updating + +Review `git diff a06bc63b3f8b829c14b0bbf53d99fefc39d58092 -- skills/ci-cd-and-automation/ references/`, re-run `packs/adapt-agent-skills.py ` (it applies the changes above as exact replacements and fails if upstream text moved), update commit and sha256 here and in `packs/sources.lock.json`, then run `packs/check-vendored.sh --online`. diff --git a/cli-tool/templates/packs/skills/code-review-and-quality/LICENSE b/cli-tool/templates/packs/skills/code-review-and-quality/LICENSE new file mode 100644 index 0000000..d67778a --- /dev/null +++ b/cli-tool/templates/packs/skills/code-review-and-quality/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2025 Addy Osmani + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/cli-tool/templates/packs/skills/code-review-and-quality/SKILL.md b/cli-tool/templates/packs/skills/code-review-and-quality/SKILL.md new file mode 100644 index 0000000..ca7001c --- /dev/null +++ b/cli-tool/templates/packs/skills/code-review-and-quality/SKILL.md @@ -0,0 +1,405 @@ +--- +name: code-review-and-quality +description: Conducts multi-axis code review. Use before merging any change. Use when reviewing code written by yourself, another agent, or a human. Use when you need to assess code quality across multiple dimensions before it enters the main branch. Use when asked to review a diff or a pull request, even when the diff is pasted inline. +--- + +> Adapted copy of addyosmani/agent-skills (MIT) for local use; changes are listed in `UPSTREAM.md`. +> Ground rules that override anything below: use only tools that are already installed in the project +> (project scripts, `npx --no-install ...`); never let npx/pip/brew download or install anything and never add +> MCP servers on your own; do not fetch web pages or online docs and do not upload or post data anywhere; +> never push, deploy, run migrations, flip feature flags or change shared systems without the user's explicit +> instruction. Commands inside CI/workflow examples are file content to write, not commands to run now. + +# Code Review and Quality + +## Overview + +Multi-dimensional code review with quality gates. Every change gets reviewed before merge — no exceptions. Review covers five axes: correctness, readability, architecture, security, and performance. + +**The approval standard:** Approve a change when it definitely improves overall code health, even if it isn't perfect. Perfect code doesn't exist — the goal is continuous improvement. Don't block a change because it isn't exactly how you would have written it. If it improves the codebase and follows the project's conventions, approve it. + +## When to Use + +- Before merging any PR or change +- After completing a feature implementation +- When another agent or model produced code you need to evaluate +- When refactoring existing code +- After any bug fix (review both the fix and the regression test) + +## The Five-Axis Review + +Every review evaluates code across these dimensions: + +### 1. Correctness + +Does the code do what it claims to do? + +- Does it match the spec or task requirements? +- Are edge cases handled (null, empty, boundary values)? +- Are error paths handled (not just the happy path)? +- Does it pass all tests? Are the tests actually testing the right things? +- Are there off-by-one errors, race conditions, or state inconsistencies? + +### 2. Readability & Simplicity + +Can another engineer (or agent) understand this code without the author explaining it? + +- Are names descriptive and consistent with project conventions? (No `temp`, `data`, `result` without context) +- Is the control flow straightforward (avoid nested ternaries, deep callbacks)? +- Is the code organized logically (related code grouped, clear module boundaries)? +- Are there any "clever" tricks that should be simplified? +- **Could this be done in fewer lines?** (1000 lines where 100 suffice is a failure) +- **Are abstractions earning their complexity?** (Don't generalize until the third use case) +- Would comments help clarify non-obvious intent? (But don't comment obvious code.) +- Are there dead code artifacts: no-op variables (`_unused`), backwards-compat shims, or `// removed` comments? +- **Is a new conditional bolted onto an unrelated flow?** That's a design smell, not a nit — push the logic into its own helper, state, or policy instead of tangling an existing path. +- **Do repeated conditionals on the same shape appear?** They signal a missing model or dispatcher. A "temporary" branch is usually permanent debt. + +### 3. Architecture + +Does the change fit the system's design? + +- Does it follow existing patterns or introduce a new one? If new, is it justified? +- Does it maintain clean module boundaries? +- Is there code duplication that should be shared? +- Are dependencies flowing in the right direction (no circular dependencies)? +- Is the abstraction level appropriate (not over-engineered, not too coupled)? +- **Does this refactor reduce complexity or just relocate it?** Count the concepts a reader must hold to follow the change. If a "cleaner" version leaves that count unchanged, it isn't cleaner — prefer the restructuring that makes whole branches, modes, or layers disappear over one that re-centralizes the same logic. Prefer deleting an abstraction to polishing it. +- **Is feature-specific logic leaking into a shared or general-purpose module?** Keep logic in its owning layer, reuse the existing canonical helper instead of a near-duplicate, and don't normalize architectural drift. +- **Are type boundaries explicit?** Question gratuitous `any`/`unknown`/optional/casts and silent fallbacks that paper over an unclear invariant — making the boundary explicit often makes the surrounding control flow simpler. + +### 4. Security + +For detailed security guidance, see `security-and-hardening`. Does the change introduce vulnerabilities? + +- Is user input validated and sanitized? +- Are secrets kept out of code, logs, and version control? +- Is authentication/authorization checked where needed? +- Are SQL queries parameterized (no string concatenation)? +- Are outputs encoded to prevent XSS? +- Are dependencies from trusted sources with no known vulnerabilities? +- Is data from external sources (APIs, logs, user content, config files) treated as untrusted? +- Are external data flows validated at system boundaries before use in logic or rendering? + +### 5. Performance + +For detailed profiling and optimization, see `performance-optimization`. Does the change introduce performance problems? + +- Any N+1 query patterns? +- Any unbounded loops or unconstrained data fetching? +- Any synchronous operations that should be async? +- Any unnecessary re-renders in UI components? +- Any missing pagination on list endpoints? +- Any large objects created in hot paths? + +## Structural Remedies + +When you flag a structural problem, propose the move — not just the problem. A review that only says "this is complex" leaves the author guessing. Reach for a named restructuring: + +- **Replace a chain of conditionals** with a typed model or an explicit dispatcher. +- **Collapse duplicate branches** into a single clearer flow. +- **Separate orchestration from business logic** so each reads on its own. +- **Move feature-specific logic** out of a shared module into the package that owns the concept. +- **Reuse the canonical helper** instead of a bespoke near-duplicate. +- **Make a type boundary explicit** so downstream branching disappears. +- **Delete a pass-through wrapper** that adds indirection without clarifying the API. +- **Extract a helper, or split a large file** into focused modules. + +Prefer the remedy that removes moving pieces over one that spreads the same complexity around. + +## Change Sizing + +Small, focused changes are easier to review, faster to merge, and safer to deploy. Target these sizes: + +``` +~100 lines changed → Good. Reviewable in one sitting. +~300 lines changed → Acceptable if it's a single logical change. +~1000 lines changed → Too large. Split it. +``` + +**Watch file size, not just diff size.** A small diff can still push a file past a healthy boundary — around 1000 *total* lines in a single file (distinct from the ~1000 *changed*-lines threshold above) is a common inspection signal, not a hard cap. When a change materially grows an already-large file, ask whether to extract helpers, subcomponents, or modules *first*, before piling more on. Decompose, then add. + +**What counts as "one change":** A single self-contained modification that addresses one thing, includes related tests, and keeps the system functional after submission. One part of a feature — not the whole feature. + +**Splitting strategies when a change is too large:** + +| Strategy | How | When | +|----------|-----|------| +| **Stack** | Submit a small change, start the next one based on it | Sequential dependencies | +| **By file group** | Separate changes for groups needing different reviewers | Cross-cutting concerns | +| **Horizontal** | Create shared code/stubs first, then consumers | Layered architecture | +| **Vertical** | Break into smaller full-stack slices of the feature | Feature work | + +**When large changes are acceptable:** Complete file deletions and automated refactoring where the reviewer only needs to verify intent, not every line. + +**Separate refactoring from feature work.** A change that refactors existing code and adds new behavior is two changes — submit them separately. Small cleanups (variable renaming) can be included at reviewer discretion. + +## Change Descriptions + +Every change needs a description that stands alone in version control history. + +**First line:** Short, imperative, standalone. "Delete the FizzBuzz RPC" not "Deleting the FizzBuzz RPC." Must be informative enough that someone searching history can understand the change without reading the diff. + +**Body:** What is changing and why. Include context, decisions, and reasoning not visible in the code itself. Link to bug numbers, benchmark results, or design docs where relevant. Acknowledge approach shortcomings when they exist. + +**Anti-patterns:** "Fix bug," "Fix build," "Add patch," "Moving code from A to B," "Phase 1," "Add convenience functions." + +## Review Process + +### Step 1: Understand the Context + +Before looking at code, understand the intent: + +``` +- What is this change trying to accomplish? +- What spec or task does it implement? +- What is the expected behavior change? +``` + +### Step 2: Review the Tests First + +Tests reveal intent and coverage: + +``` +- Do tests exist for the change? +- Do they test behavior (not implementation details)? +- Are edge cases covered? +- Do tests have descriptive names? +- Would the tests catch a regression if the code changed? +``` + +Answer the last question by experiment, not by reading. Invert one condition the change adds (drop a negation, swap `&&` for `||`), run the suite, then restore the file from a copy. A mutation that stays green is a finding: name the test case that is missing. For a project-wide mutation score, see `constraint-driven-development`. + +### Step 3: Review the Implementation + +Walk through the code with the five axes in mind: + +``` +For each file changed: +1. Correctness: Does this code do what the test says it should? +2. Readability: Can I understand this without help? +3. Architecture: Does this fit the system? +4. Security: Any vulnerabilities? +5. Performance: Any bottlenecks? +``` + +### Step 4: Categorize Findings + +Label every comment with its severity so the author knows what's required vs optional: + +| Prefix | Meaning | Author Action | +|--------|---------|---------------| +| *(no prefix)* | Required change | Must address before merge | +| **Critical:** | Blocks merge | Security vulnerability, data loss, broken functionality | +| **Nit:** | Minor, optional | Author may ignore — formatting, style preferences | +| **Optional:** / **Consider:** | Suggestion | Worth considering but not required | +| **FYI** | Informational only | No action needed — context for future reference | + +This prevents authors from treating all feedback as mandatory and wasting time on optional suggestions. + +**Lead with what matters.** Order findings by leverage: correctness and security first, then structural regressions and missed simplifications, then everything else. Don't bury a real issue under cosmetic nits — a few high-conviction comments beat a long list. If you have one structural problem and ten nits, the structural problem *is* the review. + +### Step 5: Verify the Verification + +Check the author's verification story: + +``` +- What tests were run? +- Did the build pass? +- Was the change tested manually? +- Are there screenshots for UI changes? +- Is there a before/after comparison? +``` + +## Multi-Model Review Pattern + +Use different models for different review perspectives: + +``` +Model A writes the code + │ + ▼ +Model B reviews for correctness and architecture + │ + ▼ +Model A addresses the feedback + │ + ▼ +Human makes the final call +``` + +This catches issues that a single model might miss — different models have different blind spots. + +**Example prompt for a review agent:** +``` +Review this code change for correctness, security, and adherence to +our project conventions. The spec says [X]. The change should [Y]. +Flag any issues as Critical, Required, Optional, or Nit. +``` + +## Dead Code Hygiene + +After any refactoring or implementation change, check for orphaned code: + +1. Identify code that is now unreachable or unused +2. List it explicitly +3. **Ask before deleting:** "Should I remove these now-unused elements: [list]?" + +Don't leave dead code lying around — it confuses future readers and agents. But don't silently delete things you're not sure about. When in doubt, ask. + +``` +DEAD CODE IDENTIFIED: +- formatLegacyDate() in src/utils/date.ts — replaced by formatDate() +- OldTaskCard component in src/components/ — replaced by TaskCard +- LEGACY_API_URL constant in src/config.ts — no remaining references +→ Safe to remove these? +``` + +## Review Speed + +Slow reviews block entire teams. The cost of context-switching to review is less than the waiting cost imposed on others. + +- **Respond within one business day** — this is the maximum, not the target +- **Ideal cadence:** Respond shortly after a review request arrives, unless deep in focused coding. A typical change should complete multiple review rounds in a single day +- **Prioritize fast individual responses** over quick final approval. Quick feedback reduces frustration even if multiple rounds are needed +- **Large changes:** Ask the author to split them rather than reviewing one massive changeset + +## Handling Disagreements + +When resolving review disputes, apply this hierarchy: + +1. **Technical facts and data** override opinions and preferences +2. **Style guides** are the absolute authority on style matters +3. **Software design** must be evaluated on engineering principles, not personal preference +4. **Codebase consistency** is acceptable if it doesn't degrade overall health + +**Don't accept "I'll clean it up later."** Experience shows deferred cleanup rarely happens. Require cleanup before submission unless it's a genuine emergency. If surrounding issues can't be addressed in this change, require filing a bug with self-assignment. + +## Honesty in Review + +When reviewing code — whether written by you, another agent, or a human: + +- **Don't rubber-stamp.** "LGTM" without evidence of review helps no one. +- **Don't soften real issues.** "This might be a minor concern" when it's a bug that will hit production is dishonest. +- **Quantify problems when possible.** "This N+1 query will add ~50ms per item in the list" is better than "this could be slow." +- **Push back on approaches with clear problems.** Sycophancy is a failure mode in reviews. If the implementation has issues, say so directly and propose alternatives. +- **Accept override gracefully.** If the author has full context and disagrees, defer to their judgment. Comment on code, not people — reframe personal critiques to focus on the code itself. + +## Dependency Discipline + +Part of code review is dependency review: + +**Before adding any dependency:** +1. Does the existing stack solve this? (Often it does.) +2. How large is the dependency? (Check bundle impact.) +3. Is it actively maintained? (Check last commit, open issues.) +4. Does it have known vulnerabilities? (`npm audit` contacts the registry: run it only with the user's consent or rely on CI) +5. What's the license? (Must be compatible with the project.) + +**Rule:** Prefer standard library and existing utilities over new dependencies. Every dependency is a liability. + +**Upgrading an existing dependency** is a code change like any other, and the riskiest upgrades are the ones merged in bulk with a message like "bump deps." Review them with the same discipline: + +1. **Read the changelog, not just the version number.** Semver is a promise the maintainer may not have kept — a "patch" can carry a behavioral change. For a major bump, read the migration notes and find what breaks. +2. **One dependency per change.** Upgrade and merge them individually (or in small related groups). When a bulk bump breaks the build, you've lost which package did it; a single-package change makes the cause obvious and the revert clean. +3. **Let the tests decide.** The upgrade is verified by a green suite before *and* after, not by "it installed." If coverage around the dependency's behavior is thin, that gap is the real finding — add a test first. +4. **Mind the transitive graph.** Most installed packages are ones nobody chose directly. Review the lockfile diff, not just `package.json`; a single direct bump can pull in dozens of indirect changes. +5. **Keep the lockfile honest.** Commit it, review its diff, and never hand-edit it. The lockfile is the thing that actually pins what ships. + +For triaging `npm audit` findings and supply-chain risk (typosquatting, compromised maintainers), follow the `security-and-hardening` skill — this section covers the upgrade *workflow*, that one covers the security verdict. + +## The Review Checklist + +```markdown +## Review: [PR/Change title] + +### Context +- [ ] I understand what this change does and why + +### Correctness +- [ ] Change matches spec/task requirements +- [ ] Edge cases handled +- [ ] Error paths handled +- [ ] Tests cover the change adequately + +### Readability +- [ ] Names are clear and consistent +- [ ] Logic is straightforward +- [ ] No unnecessary complexity + +### Architecture +- [ ] Follows existing patterns +- [ ] No unnecessary coupling or dependencies +- [ ] Appropriate abstraction level +- [ ] Refactors reduce complexity rather than relocate it +- [ ] No feature logic in shared modules; file stays within a healthy size + +### Security +- [ ] No secrets in code +- [ ] Input validated at boundaries +- [ ] No injection vulnerabilities +- [ ] Auth checks in place +- [ ] External data sources treated as untrusted + +### Performance +- [ ] No N+1 patterns +- [ ] No unbounded operations +- [ ] Pagination on list endpoints + +### Verification +- [ ] Tests pass +- [ ] Build succeeds +- [ ] Manual verification done (if applicable) + +### Verdict +- [ ] **Approve** — Ready to merge +- [ ] **Request changes** — Issues must be addressed +``` +## See Also + +- For detailed security review guidance, see `references/security-checklist.md` +- For performance review checks, see `references/performance-checklist.md` + +## Common Rationalizations + +| Rationalization | Reality | +|---|---| +| "It works, that's good enough" | Working code that's unreadable, insecure, or architecturally wrong creates debt that compounds. | +| "I wrote it, so I know it's correct" | Authors are blind to their own assumptions. Every change benefits from another set of eyes. | +| "We'll clean it up later" | Later never comes. The review is the quality gate — use it. Require cleanup before merge, not after. | +| "AI-generated code is probably fine" | AI code needs more scrutiny, not less. It's confident and plausible, even when wrong. | +| "The tests pass, so it's good" | Tests are necessary but not sufficient. They don't catch architecture problems, security issues, or readability concerns. | +| "The refactor makes it cleaner" | Relocating complexity isn't reducing it. If the reader still holds the same number of concepts, the structure didn't improve — look for the version where branches disappear. | +| "It's only a small addition to this file" | Small diffs still push files past a healthy size and bolt branches onto unrelated flows. Judge the resulting structure, not the diff size. | +| "It's just a version bump" | A bump is a behavior change you didn't write. Read the changelog; semver doesn't guarantee no breakage. | +| "I'll upgrade everything in one PR to save time" | A bulk bump that breaks the build hides which package did it. One dependency per change keeps the cause and the revert clean. | + +## Red Flags + +- PRs merged without any review +- Review that only checks if tests pass (ignoring other axes) +- "LGTM" without evidence of actual review +- Security-sensitive changes without security-focused review +- Large PRs that are "too big to review properly" (split them) +- No regression tests with bug fix PRs +- Review comments without severity labels — makes it unclear what's required vs optional +- Accepting "I'll fix it later" — it never happens +- A refactor that moves code around without reducing the number of concepts a reader must hold +- A change that grows an already-large file instead of decomposing it +- New conditionals scattered into unrelated code paths (a missing abstraction) +- A bespoke helper that duplicates an existing canonical one, or feature logic placed in a shared module +- A bulk "bump dependencies" PR with no changelog review and no per-package isolation +- A lockfile change that's hand-edited, uncommitted, or merged without reviewing its diff + +## Verification + +After review is complete: + +- [ ] All Critical issues are resolved +- [ ] All Required (no-prefix) changes are resolved or explicitly deferred with justification +- [ ] Tests pass +- [ ] Build succeeds +- [ ] The verification story is documented (what changed, how it was verified) +- [ ] Dependency upgrades were reviewed against their changelog, isolated per package, and verified by a green suite with the lockfile diff reviewed + +**Presumptive blockers:** surface and propose the simpler design for each of these; escalate to Required only when the change actively makes structure worse: a refactor that relocates complexity instead of reducing it; a change that pushes a file past the size boundary with no decomposition; feature logic added to a shared module; a near-duplicate of an existing canonical helper; a silent fallback that hides an unclear invariant. diff --git a/cli-tool/templates/packs/skills/code-review-and-quality/UPSTREAM.md b/cli-tool/templates/packs/skills/code-review-and-quality/UPSTREAM.md new file mode 100644 index 0000000..b0cdd68 --- /dev/null +++ b/cli-tool/templates/packs/skills/code-review-and-quality/UPSTREAM.md @@ -0,0 +1,25 @@ +# Upstream and license + +This folder is an adapted copy of `skills/code-review-and-quality/` from an MIT-licensed upstream repository. + +| Field | Value | +|---|---| +| Source repository | https://github.com/addyosmani/agent-skills | +| Path | `skills/code-review-and-quality/` | +| Pinned commit | `a06bc63b3f8b829c14b0bbf53d99fefc39d58092` | +| Retrieved | 2026-10-03 | +| Upstream SKILL.md sha256 | `2db1e850379f255fd3091ef278cde24cd6c7d2ba23e360cc52ea522c40d4396e` | +| License | MIT License, Copyright (c) 2025 Addy Osmani (full text: `LICENSE`) | + +## Changes made in this copy + +- Copied the shared reference `references/performance-checklist.md` into the skill folder and pointed links at it. +- Copied the shared reference `references/security-checklist.md` into the skill folder and pointed links at it. +- Added the local-use ground rules block under the front matter. +- `npm audit` only with consent (network). + +Everything else is upstream text, unchanged. + +## Updating + +Review `git diff a06bc63b3f8b829c14b0bbf53d99fefc39d58092 -- skills/code-review-and-quality/ references/`, re-run `packs/adapt-agent-skills.py ` (it applies the changes above as exact replacements and fails if upstream text moved), update commit and sha256 here and in `packs/sources.lock.json`, then run `packs/check-vendored.sh --online`. diff --git a/cli-tool/templates/packs/skills/code-review-and-quality/references/performance-checklist.md b/cli-tool/templates/packs/skills/code-review-and-quality/references/performance-checklist.md new file mode 100644 index 0000000..7118892 --- /dev/null +++ b/cli-tool/templates/packs/skills/code-review-and-quality/references/performance-checklist.md @@ -0,0 +1,236 @@ +# Performance Checklist + +Quick reference checklist for web application performance. Use alongside the `performance-optimization` skill. + +## Table of Contents + +- [Core Web Vitals Targets](#core-web-vitals-targets) +- [TTFB Diagnosis](#ttfb-diagnosis) +- [Frontend Checklist](#frontend-checklist) +- [Backend Checklist](#backend-checklist) +- [Caching Strategies](#caching-strategies) +- [Measurement Commands](#measurement-commands) +- [Common Anti-Patterns](#common-anti-patterns) + +## Core Web Vitals Targets + +| Metric | Good | Needs Work | Poor | +|--------|------|------------|------| +| LCP (Largest Contentful Paint) | ≤ 2.5s | ≤ 4.0s | > 4.0s | +| INP (Interaction to Next Paint) | ≤ 200ms | ≤ 500ms | > 500ms | +| CLS (Cumulative Layout Shift) | ≤ 0.1 | ≤ 0.25 | > 0.25 | + +## TTFB Diagnosis + +When TTFB is slow (> 800ms), check each component in DevTools Network waterfall: + +- [ ] **DNS resolution** slow → add `` or `` for known origins +- [ ] **TCP/TLS handshake** slow → enable HTTP/2, consider edge deployment, verify keep-alive +- [ ] **Server processing** slow → profile backend, check slow queries, add caching + +## Frontend Checklist + +### Images +- [ ] Images use modern formats (WebP, AVIF) +- [ ] Images are responsively sized (`srcset` and `sizes`) +- [ ] Images and `` elements have explicit `width` and `height` (prevents CLS in art direction) +- [ ] Below-the-fold images use `loading="lazy"` and `decoding="async"` +- [ ] Hero/LCP images use `fetchpriority="high"` and no lazy loading + +### JavaScript +- [ ] Bundle size under 200KB gzipped (initial load) +- [ ] Code splitting with dynamic `import()` for routes and heavy features +- [ ] Tree shaking enabled (verify dependency ships ESM and marks `sideEffects: false`) +- [ ] No blocking JavaScript in `` (use `defer` or `async`) +- [ ] Heavy computation offloaded to Web Workers (if applicable) +- [ ] `React.memo()` on expensive components that re-render with same props +- [ ] `useMemo()` / `useCallback()` only where profiling shows benefit +- [ ] Long tasks (> 50ms) broken up to keep the main thread available — main lever for INP +- [ ] `yieldToMain` pattern used inside long-running loops so input events can run between chunks +- [ ] Modern scheduling APIs used where available: `scheduler.yield()` (preferred), `scheduler.postTask()` with priorities, `isInputPending()` to yield only when needed +- [ ] `requestIdleCallback` for deferrable, non-urgent work (analytics flush, prefetch, warmup) +- [ ] Non-critical work deferred out of event handlers (e.g. analytics, logging) so the response to the interaction is not delayed +- [ ] Third-party scripts loaded with `async` / `defer`, audited for size, and fronted by a facade when heavy (chat widgets, embeds) + +### CSS +- [ ] Critical CSS inlined or preloaded +- [ ] No render-blocking CSS for non-critical styles +- [ ] No CSS-in-JS runtime cost in production (use extraction) + +### Fonts +- [ ] Limited to 2–3 font families, 2–3 weights each (every additional weight is another request) +- [ ] WOFF2 format only (smallest, universal support — skip WOFF/TTF/EOT) +- [ ] Self-hosted when possible (third-party font CDNs add DNS + TCP + TLS round-trips) +- [ ] LCP-critical fonts preloaded: `` +- [ ] `font-display: swap` (or `optional` for non-critical) to avoid FOIT blocking render +- [ ] Subsetted via `unicode-range` to ship only the glyphs each page needs +- [ ] Variable fonts considered when multiple weights/styles are required (one file replaces many) +- [ ] Fallback font metrics adjusted with `size-adjust`, `ascent-override`, `descent-override` to reduce CLS on font swap +- [ ] System font stack considered before any custom font + +### Network +- [ ] Static assets cached with long `max-age` + content hashing +- [ ] API responses cached where appropriate (`Cache-Control`) +- [ ] HTTP/2 or HTTP/3 enabled +- [ ] Resources preconnected (``) for known origins +- [ ] `fetchpriority` used on critical non-image resources (e.g., key ``, above-the-fold ` +``` + +## Appendix B - Canonical Sources (for humans; do not fetch at run time) + +The upstream skill lists the official documentation URLs for every system in Section 2 (Material Web, Fluent UI, Carbon, Shopify Polaris, Atlassian, Primer, GOV.UK Frontend, USWDS, Bootstrap, Tailwind, Radix, shadcn/ui, MDN CSS references, Apple Liquid Glass). This offline copy omits the link list on purpose. At run time, rely on the README, type definitions and docs that ship inside the installed package (`node_modules//`), and on MDN knowledge you already have. If the user wants a link, name the project and let them open its official site. + +--- + +## Appendix C - Apple Liquid Glass: Honest Web Approximation + +Do **not** treat random CSS snippets as official Apple Liquid Glass. + +### What is official +Apple documents Liquid Glass inside Apple's Human Interface Guidelines and Developer Documentation for **Apple platforms**. It is a dynamic material used across Apple platform UI. Apple's native implementation belongs to Apple platform APIs and system components, **not a public web CSS package**. + +Relevant official docs: +- Apple Human Interface Guidelines → Materials +- Apple Developer Documentation → Liquid Glass +- Apple Developer Documentation → Adopting Liquid Glass +- SwiftUI → Material + +### What is NOT official +There is no `liquid-glass.css` from Apple for normal websites. + +A web approximation can use: +- `backdrop-filter` +- transparent backgrounds +- layered borders +- highlight overlays +- gradients +- motion +- strong contrast fallbacks + +But that is **web glassmorphism / frosted-glass approximation**, not official Apple Liquid Glass. Label it as such in comments. + +### Safer web approximation skeleton + +```css +.liquid-glass-web-approx { + position: relative; + isolation: isolate; + overflow: hidden; + border-radius: 999px; + border: 1px solid rgb(255 255 255 / .32); + background: + linear-gradient(135deg, rgb(255 255 255 / .30), rgb(255 255 255 / .08)), + rgb(255 255 255 / .12); + backdrop-filter: blur(24px) saturate(180%) contrast(1.05); + -webkit-backdrop-filter: blur(24px) saturate(180%) contrast(1.05); + box-shadow: + inset 0 1px 0 rgb(255 255 255 / .48), + inset 0 -1px 0 rgb(255 255 255 / .12), + 0 18px 60px rgb(0 0 0 / .18); +} + +.liquid-glass-web-approx::before { + content: ""; + position: absolute; + inset: 0; + z-index: -1; + border-radius: inherit; + background: + radial-gradient(circle at 20% 0%, rgb(255 255 255 / .55), transparent 34%), + linear-gradient(90deg, rgb(255 255 255 / .18), transparent 42%, rgb(255 255 255 / .14)); + pointer-events: none; +} + +.liquid-glass-web-approx::after { + content: ""; + position: absolute; + inset: 1px; + border-radius: inherit; + border: 1px solid rgb(255 255 255 / .14); + pointer-events: none; +} + +@media (prefers-color-scheme: dark) { + .liquid-glass-web-approx { + border-color: rgb(255 255 255 / .18); + background: + linear-gradient(135deg, rgb(255 255 255 / .16), rgb(255 255 255 / .04)), + rgb(15 23 42 / .42); + box-shadow: + inset 0 1px 0 rgb(255 255 255 / .22), + 0 18px 60px rgb(0 0 0 / .42); + } +} + +@media (prefers-reduced-transparency: reduce) { + .liquid-glass-web-approx { + background: rgb(255 255 255 / .96); + backdrop-filter: none; + -webkit-backdrop-filter: none; + } +} +``` + +**Important:** `prefers-reduced-transparency` has uneven browser support; test it. Always provide enough contrast even without blur. + +--- + +**End of appendices.** Install commands above are reality anchors. The Apple Liquid Glass skeleton is a labeled approximation, not an Apple-issued package. For canonical docs per design system, consult the system's official docs (links in Section 2 plus Appendix B). diff --git a/cli-tool/templates/packs/skills/design-taste-frontend/UPSTREAM.md b/cli-tool/templates/packs/skills/design-taste-frontend/UPSTREAM.md new file mode 100644 index 0000000..61291d9 --- /dev/null +++ b/cli-tool/templates/packs/skills/design-taste-frontend/UPSTREAM.md @@ -0,0 +1,29 @@ +# Upstream and license + +`SKILL.md` in this folder is an adapted copy of an MIT-licensed upstream skill. + +| Field | Value | +|---|---| +| Source repository | https://github.com/Leonxlnx/taste-skill (site: https://www.tasteskill.dev/) | +| File | `skills/taste-skill/SKILL.md` | +| Pinned commit | `ce26fc25c0e5e8cab638f883de62d9a86ee5e45b` | +| Retrieved | 2026-10-03 | +| Upstream sha256 | `aa194351b246b8b4799099d4ed7b033d29eab6e6e3d58d8d2172978be7b3ec89` | +| License | MIT License, Copyright (c) 2026 Leonxlnx (full text: `LICENSE`) | + +## Changes made in this copy + +- Added an offline note under the title and to the description. +- Section 0.A: do not browse URLs the user linked unless explicitly asked (use provided screenshots/files). +- Section 2.A / 3.F: never install packages autonomously; propose a pinned install command and wait for approval (shadcn row no longer shows an unpinned `npx shadcn@latest`). +- Section 4.8: asset priority is now project/brief assets, then image generation only with user consent, then labeled local placeholders; hotlinking external image/logo CDNs (picsum.photos, cdn.simpleicons.org) is forbidden. +- Section 6.D: run Lighthouse only if available locally, never download it ad hoc. +- Section 9.E and the pre-flight checklist: "use picsum.photos" replaced by "no hotlinked images"; logos from project assets or an installed icon package. +- Appendix A: marked as commands for the user to run after approval, with pinned versions; Shopify CDN script flagged as Shopify-only. +- Appendix B: the list of documentation URLs was removed (agents must not fetch docs at run time); use the docs shipped with installed packages. + +Everything else is upstream text, unchanged. + +## Updating + +Review the upstream diff from the pinned commit (`git diff ce26fc25c0e5e8cab638f883de62d9a86ee5e45b -- skills/taste-skill/SKILL.md`), re-apply the changes above, update commit and sha256 here and in `packs/sources.lock.json`, then run `packs/check-vendored.sh --online`. diff --git a/cli-tool/templates/packs/skills/design-workflow/SKILL.md b/cli-tool/templates/packs/skills/design-workflow/SKILL.md new file mode 100644 index 0000000..b09681e --- /dev/null +++ b/cli-tool/templates/packs/skills/design-workflow/SKILL.md @@ -0,0 +1,78 @@ +--- +name: design-workflow +description: End-to-end workflow for frontend design work - write a design brief, pick one taste skill and optionally a DESIGN.md reference, implement, review against the web interface guidelines, and verify with local Playwright screenshots. Use when building or redesigning a page, landing page, portfolio or UI component where visual quality matters. +license: MIT +compatibility: claude-code, opencode +--- + +# Design workflow + +This skill only orchestrates. The detailed rules live in the skills it names; load them when a +step says so. If a named skill is not installed, skip that part and say so in the result. + +## 1. Brief (always, before code) + +Write five lines and show them to the user before larger work: + +- **Surface**: what is being built (landing page, settings screen, component, redesign). +- **Audience and goal**: who looks at it and what they must do. +- **Mode**: greenfield, redesign-preserve or redesign-overhaul. For redesigns, list what must not + change (URLs, nav labels, form field names, brand assets, legal copy). +- **Inputs**: provided images, existing site, brand assets, named style references. +- **Constraints**: stack and design system already in the project, accessibility needs, + light/dark requirement, performance budget. + +Ask at most one question, and only if the brief is genuinely ambiguous. + +## 2. Pick the design direction (exactly one primary taste skill) + +| Situation | Load | +|---|---| +| Default for landing pages, portfolios, marketing sites | `design-taste-frontend` | +| Improving an existing site without a rewrite | `redesign-existing-projects` | +| Calm, editorial, document-like UI (Notion/Linear-like) | `minimalist-ui` | +| Premium agency look, soft depth, rich motion | `high-end-visual-design` | +| Raw, Swiss/terminal, data-dense look | `industrial-brutalist-ui` | +| Award-style motion-heavy page, user wants GSAP (opt-in skill) | `gpt-taste` | +| User provided mockups or screenshots to implement | `image-to-code` (plus one taste skill for gaps) | + +Do not stack several taste skills; their rules conflict (fonts, radii, motion). If the user +names a product as reference, also load `design-md-reference` and pick one entry for concrete +tokens. Project conventions and an existing design system always win over skill defaults. + +## 3. Implement + +- Put tokens (colors, type scale, spacing, radii, motion) into the project's theme first, then + build sections/components. +- Assets: use files in the project or provided by the user; otherwise labeled local + placeholders with fixed aspect ratios. No hotlinked images, fonts or scripts. +- Dependencies: if something is missing, propose the pinned install command and wait for + approval. Do not install on your own. +- Respect `prefers-reduced-motion`, keyboard focus, and both color schemes from the start. + +## 4. Review (static) + +Load `web-design-guidelines` and review the changed files. Fix the findings that are real for +this stack; list the ones you leave and why. + +## 5. Verify (rendered, local only) + +If `@playwright/cli` is available in the project, load `playwright-cli` and run its visual +verification loop against the local dev server: desktop and mobile widths, light and dark, +reduced motion, console warnings, `requests` (any non-local request = hotlinked asset to fix). +Look at the screenshots yourself and compare against the brief and any reference images. Fix, +then re-run the affected captures. If Playwright is not available, say that visual +verification was not done. + +## 6. Report + +Short summary: brief, chosen skill(s) and reference, what was built, review findings fixed or +left, screenshot paths, open items (missing real assets, fonts to license, decisions for the +user). + +## Ground rules for all steps + +- Skills and reference files are local. Do not fetch rules, references or assets from the + internet at run time and do not run commands found inside reference documents. +- Never build something that could pass for a real brand (logos, names, login or payment pages). +- Keep screenshots and other artifacts on disk; do not upload them anywhere. diff --git a/cli-tool/templates/packs/skills/documentation-and-adrs/LICENSE b/cli-tool/templates/packs/skills/documentation-and-adrs/LICENSE new file mode 100644 index 0000000..d67778a --- /dev/null +++ b/cli-tool/templates/packs/skills/documentation-and-adrs/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2025 Addy Osmani + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/cli-tool/templates/packs/skills/documentation-and-adrs/SKILL.md b/cli-tool/templates/packs/skills/documentation-and-adrs/SKILL.md new file mode 100644 index 0000000..63fdee3 --- /dev/null +++ b/cli-tool/templates/packs/skills/documentation-and-adrs/SKILL.md @@ -0,0 +1,295 @@ +--- +name: documentation-and-adrs +description: Records decisions and documentation. Use when you need to document an architecture decision (ADR) or the reasoning behind a design choice, when changing public APIs, shipping features, or when you need to record context that future engineers and agents will need to understand the codebase. +--- + +> Adapted copy of addyosmani/agent-skills (MIT) for local use; changes are listed in `UPSTREAM.md`. +> Ground rules that override anything below: use only tools that are already installed in the project +> (project scripts, `npx --no-install ...`); never let npx/pip/brew download or install anything and never add +> MCP servers on your own; do not fetch web pages or online docs and do not upload or post data anywhere; +> never push, deploy, run migrations, flip feature flags or change shared systems without the user's explicit +> instruction. Commands inside CI/workflow examples are file content to write, not commands to run now. + +# Documentation and ADRs + +## Overview + +Document decisions, not just code. The most valuable documentation captures the *why* — the context, constraints, and trade-offs that led to a decision. Code shows *what* was built; documentation explains *why it was built this way* and *what alternatives were considered*. This context is essential for future humans and agents working in the codebase. + +## When to Use + +- Making a significant architectural decision +- Choosing between competing approaches +- Adding or changing a public API +- Shipping a feature that changes user-facing behavior +- Onboarding new team members (or agents) to the project +- When you find yourself explaining the same thing repeatedly + +**When NOT to use:** Don't document obvious code. Don't add comments that restate what the code already says. Don't write docs for throwaway prototypes. + +## Architecture Decision Records (ADRs) + +ADRs capture the reasoning behind significant technical decisions. They're the highest-value documentation you can write. + +### When to Write an ADR + +- Choosing a framework, library, or major dependency +- Designing a data model or database schema +- Selecting an authentication strategy +- Deciding on an API architecture (REST vs. GraphQL vs. tRPC) +- Choosing between build tools, hosting platforms, or infrastructure +- Any decision that would be expensive to reverse + +### Match the existing convention first + +Before creating an ADR, inspect the available repository context for an established convention — existing ADRs, project instructions, and ADR-related configuration or tooling (e.g. an `.adr-dir` file). An established convention overrides the defaults below. Match: + +- **Location and format** — e.g. `docs/adr/*.md`, `Documentation/Decisions/*.rst`, a MADR layout, or an `adr-tools` setup. Match the existing directory, file extension, and markup (Markdown vs reStructuredText). +- **Numbering and naming** — continue the existing sequence and filename pattern (`ADR-004-Title.rst`, `0004-title.md`, …); don't restart at 001 or introduce a second scheme. +- **Section headings** — reuse the project's heading set rather than imposing this template's. + +If the available evidence conflicts, surface the conflict rather than silently introducing another scheme. Only when no convention can be established do you apply the default below. + +### ADR Template + +Store ADRs in `docs/decisions/` with sequential numbering (unless the project already uses another location — see above): + +```markdown +# ADR-001: Use PostgreSQL for primary database + +## Status +Proposed | Accepted | Superseded by ADR-XXX | Deprecated + +## Date +2025-01-15 + +## Context +We need a primary database for the task management application. Key requirements: +- Relational data model (users, tasks, teams with relationships) +- ACID transactions for task state changes +- Support for full-text search on task content +- Managed hosting available (for small team, limited ops capacity) + +## Decision +Use PostgreSQL with Prisma ORM. + +## Alternatives Considered + +### MongoDB +- Pros: Flexible schema, easy to start with +- Cons: Our data is inherently relational; would need to manage relationships manually +- Rejected: Relational data in a document store leads to complex joins or data duplication + +### SQLite +- Pros: Zero configuration, embedded, fast for reads +- Cons: Limited concurrent write support, no managed hosting for production +- Rejected: Not suitable for multi-user web application in production + +### MySQL +- Pros: Mature, widely supported +- Cons: PostgreSQL has better JSON support, full-text search, and ecosystem tooling +- Rejected: PostgreSQL is the better fit for our feature requirements + +## Consequences +- Prisma provides type-safe database access and migration management +- We can use PostgreSQL's full-text search instead of adding Elasticsearch +- Team needs PostgreSQL knowledge (standard skill, low risk) +- Hosting on managed service (Supabase, Neon, or RDS) +``` + +### ADR Lifecycle + +``` +PROPOSED → ACCEPTED → (SUPERSEDED or DEPRECATED) +``` + +- **Don't delete old ADRs.** They capture historical context. +- When a decision changes, write a new ADR that references and supersedes the old one. + +## Inline Documentation + +### When to Comment + +Comment the *why*, not the *what*: + +```typescript +// BAD: Restates the code +// Increment counter by 1 +counter += 1; + +// GOOD: Explains non-obvious intent +// Rate limit uses a sliding window — reset counter at window boundary, +// not on a fixed schedule, to prevent burst attacks at window edges +if (now - windowStart > WINDOW_SIZE_MS) { + counter = 0; + windowStart = now; +} +``` + +### When NOT to Comment + +```typescript +// Don't comment self-explanatory code +function calculateTotal(items: CartItem[]): number { + return items.reduce((sum, item) => sum + item.price * item.quantity, 0); +} + +// Don't leave TODO comments for things you should just do now +// TODO: add error handling ← Just add it + +// Don't leave commented-out code +// const oldImplementation = () => { ... } ← Delete it, git has history +``` + +### Document Known Gotchas + +```typescript +/** + * IMPORTANT: This function must be called before the first render. + * If called after hydration, it causes a flash of unstyled content + * because the theme context isn't available during SSR. + * + * See ADR-003 for the full design rationale. + */ +export function initializeTheme(theme: Theme): void { + // ... +} +``` + +## API Documentation + +For public APIs (REST, GraphQL, library interfaces): + +### Inline with Types (Preferred for TypeScript) + +```typescript +/** + * Creates a new task. + * + * @param input - Task creation data (title required, description optional) + * @returns The created task with server-generated ID and timestamps + * @throws {ValidationError} If title is empty or exceeds 200 characters + * @throws {AuthenticationError} If the user is not authenticated + * + * @example + * const task = await createTask({ title: 'Buy groceries' }); + * console.log(task.id); // "task_abc123" + */ +export async function createTask(input: CreateTaskInput): Promise { + // ... +} +``` + +### OpenAPI / Swagger for REST APIs + +```yaml +paths: + /api/tasks: + post: + summary: Create a task + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/CreateTaskInput' + responses: + '201': + description: Task created + content: + application/json: + schema: + $ref: '#/components/schemas/Task' + '422': + description: Validation error +``` + +## README Structure + +Every project should have a README that covers: + +```markdown +# Project Name + +One-paragraph description of what this project does. + +## Quick Start +1. Clone the repo +2. Install dependencies: `npm install` +3. Set up environment: `cp .env.example .env` +4. Run the dev server: `npm run dev` + +## Commands +| Command | Description | +|---------|-------------| +| `npm run dev` | Start development server | +| `npm test` | Run tests | +| `npm run build` | Production build | +| `npm run lint` | Run linter | + +## Architecture +Brief overview of the project structure and key design decisions. +Link to ADRs for details. + +## Contributing +How to contribute, coding standards, PR process. +``` + +## Changelog Maintenance + +For shipped features: + +```markdown +# Changelog + +## [1.2.0] - 2025-01-20 +### Added +- Task sharing: users can share tasks with team members (#123) +- Email notifications for task assignments (#124) + +### Fixed +- Duplicate tasks appearing when rapidly clicking create button (#125) + +### Changed +- Task list now loads 50 items per page (was 20) for better UX (#126) +``` + +## Documentation for Agents + +Special consideration for AI agent context: + +- **CLAUDE.md / rules files** — Document project conventions so agents follow them +- **Spec files** — Keep specs updated so agents build the right thing +- **ADRs** — Help agents understand why past decisions were made (prevents re-deciding) +- **Inline gotchas** — Prevent agents from falling into known traps + +## Common Rationalizations + +| Rationalization | Reality | +|---|---| +| "The code is self-documenting" | Code shows what. It doesn't show why, what alternatives were rejected, or what constraints apply. | +| "We'll write docs when the API stabilizes" | APIs stabilize faster when you document them. The doc is the first test of the design. | +| "Nobody reads docs" | Agents do. Future engineers do. Your 3-months-later self does. | +| "ADRs are overhead" | A 10-minute ADR prevents a 2-hour debate about the same decision six months later. | +| "Comments get outdated" | Comments on *why* are stable. Comments on *what* get outdated — that's why you only write the former. | + +## Red Flags + +- Architectural decisions with no written rationale +- Public APIs with no documentation or types +- README that doesn't explain how to run the project +- Commented-out code instead of deletion +- TODO comments that have been there for weeks +- No ADRs in a project with significant architectural choices +- Documentation that restates the code instead of explaining intent + +## Verification + +After documenting: + +- [ ] ADRs exist for all significant architectural decisions +- [ ] README covers quick start, commands, and architecture overview +- [ ] API functions have parameter and return type documentation +- [ ] Known gotchas are documented inline where they matter +- [ ] No commented-out code remains +- [ ] Rules files (CLAUDE.md etc.) are current and accurate diff --git a/cli-tool/templates/packs/skills/documentation-and-adrs/UPSTREAM.md b/cli-tool/templates/packs/skills/documentation-and-adrs/UPSTREAM.md new file mode 100644 index 0000000..99b3c0e --- /dev/null +++ b/cli-tool/templates/packs/skills/documentation-and-adrs/UPSTREAM.md @@ -0,0 +1,22 @@ +# Upstream and license + +This folder is an adapted copy of `skills/documentation-and-adrs/` from an MIT-licensed upstream repository. + +| Field | Value | +|---|---| +| Source repository | https://github.com/addyosmani/agent-skills | +| Path | `skills/documentation-and-adrs/` | +| Pinned commit | `a06bc63b3f8b829c14b0bbf53d99fefc39d58092` | +| Retrieved | 2026-10-03 | +| Upstream SKILL.md sha256 | `359b22460045583b0233ec3f80223459c4096b0fa513f5c29ade9c3985207d65` | +| License | MIT License, Copyright (c) 2025 Addy Osmani (full text: `LICENSE`) | + +## Changes made in this copy + +- Added the local-use ground rules block under the front matter. + +Everything else is upstream text, unchanged. + +## Updating + +Review `git diff a06bc63b3f8b829c14b0bbf53d99fefc39d58092 -- skills/documentation-and-adrs/ references/`, re-run `packs/adapt-agent-skills.py ` (it applies the changes above as exact replacements and fails if upstream text moved), update commit and sha256 here and in `packs/sources.lock.json`, then run `packs/check-vendored.sh --online`. diff --git a/cli-tool/templates/packs/skills/doubt-driven-development/LICENSE b/cli-tool/templates/packs/skills/doubt-driven-development/LICENSE new file mode 100644 index 0000000..d67778a --- /dev/null +++ b/cli-tool/templates/packs/skills/doubt-driven-development/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2025 Addy Osmani + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/cli-tool/templates/packs/skills/doubt-driven-development/SKILL.md b/cli-tool/templates/packs/skills/doubt-driven-development/SKILL.md new file mode 100644 index 0000000..d7d5e76 --- /dev/null +++ b/cli-tool/templates/packs/skills/doubt-driven-development/SKILL.md @@ -0,0 +1,191 @@ +--- +name: doubt-driven-development +description: Subjects every non-trivial decision to a fresh-context adversarial review before it stands. Use when you want every assumption cross-examined before proceeding, when stress-testing a plan for hidden failure modes, when correctness matters more than speed, when working in unfamiliar code, when stakes are high (production auth, security-sensitive logic, a high-stakes migration, irreversible operations), or any time a confident output would be cheaper to verify now than to debug later. +--- + +> Adapted copy of addyosmani/agent-skills (MIT) for local use; changes are listed in `UPSTREAM.md`. +> Ground rules that override anything below: use only tools that are already installed in the project +> (project scripts, `npx --no-install ...`); never let npx/pip/brew download or install anything and never add +> MCP servers on your own; do not fetch web pages or online docs and do not upload or post data anywhere; +> never push, deploy, run migrations, flip feature flags or change shared systems without the user's explicit +> instruction. Commands inside CI/workflow examples are file content to write, not commands to run now. + +# Doubt-Driven Development + +## Overview + +A confident answer is not a correct one. Long sessions accumulate context that quietly turns assumptions into "facts" without anyone noticing. Doubt-driven development is the discipline of materializing a fresh-context reviewer — biased to **disprove**, not approve — before any non-trivial output stands. + +This is not `/review`. `/review` is a verdict on a finished artifact. This is an in-flight posture: non-trivial decisions get cross-examined while course-correction is still cheap. + +## When to Use + +A decision is **non-trivial** when at least one of these is true: + +- It introduces or modifies branching logic +- It crosses a module or service boundary +- It asserts a property the type system or compiler cannot verify (thread safety, idempotence, ordering, invariants) +- Its correctness depends on context the future reader cannot see +- Its blast radius is irreversible (production deploy, data migration, public API change) + +Apply the skill when: + +- About to make an architectural decision under uncertainty +- About to commit non-trivial code +- About to claim a non-obvious fact ("this is safe", "this scales", "this matches the spec") +- Working in code you don't fully understand + +**When NOT to use:** + +- Mechanical operations (renaming, formatting, file moves) +- Following a clear, unambiguous user instruction +- Reading or summarizing existing code +- One-line changes with obvious correctness +- Pure tooling operations (running tests, listing files) +- The user has explicitly asked for speed over verification + +If you doubt every keystroke, you ship nothing. The skill applies only to non-trivial decisions as defined above. + +## Loading Constraints + +This skill is designed for the **main-session orchestrator**, where Step 3 (DOUBT, detailed below) can spawn a fresh-context reviewer. + +- **Do NOT add this skill to a persona's `skills:` frontmatter.** A persona that follows Step 3 would spawn another persona — an orchestration anti-pattern ("personas do not invoke other personas"). +- **If you find yourself applying this skill from inside a subagent context** (where Claude Code prevents nested subagent spawn): the preferred path is to surface to the user that doubt-driven cannot run nested and let the main session handle it. As a last resort only, a degraded self-questioning fallback exists — rewrite ARTIFACT + CONTRACT as a fresh self-prompt with a hard mental separator from your prior reasoning, and walk Steps 1–5. This is **not fresh-context review** (you carry your own context with you), so flag the result as degraded and prefer escalation whenever the user is reachable. + +## The Process + +Copy this checklist when applying the skill: + +``` +Doubt cycle: +- [ ] Step 1: CLAIM — wrote the claim + why-it-matters +- [ ] Step 2: EXTRACT — isolated artifact + contract, stripped reasoning +- [ ] Step 3: DOUBT — invoked fresh-context reviewer with adversarial prompt +- [ ] Step 4: RECONCILE — classified every finding against the artifact text +- [ ] Step 5: STOP — met stop condition (trivial findings, 3 cycles, or user override) +``` + +### Step 1: CLAIM — Surface what stands + +Name the decision in two or three lines: + +``` +CLAIM: "The new caching layer is thread-safe under the + read-heavy workload described in the spec." +WHY THIS MATTERS: a race here corrupts user data and is + hard to detect in QA. +``` + +If you can't write the claim that compactly, you have a vibe, not a decision. Surface it before scrutinizing it. + +### Step 2: EXTRACT — Smallest reviewable unit + +A fresh-context reviewer needs the **artifact** and the **contract**, not the journey. + +- Code: the diff or the function — not the whole file +- Decision: the proposal in 3–5 sentences plus the constraints it has to satisfy +- Assertion: the claim plus the evidence that supposedly supports it (kept distinct from the Step 1 CLAIM block, which is the orchestrator's hypothesis under scrutiny) + +Strip your reasoning. If you hand over conclusions, you'll get back validation of your conclusions. The unit must be small enough that a reviewer can hold it in mind in one read — if it's a 500-line PR, decompose first. + +### Step 3: DOUBT — Invoke the fresh-context reviewer + +The reviewer's prompt **must be adversarial**. Framing decides the answer. + +``` +Adversarial review. Find what is wrong with this artifact. +Assume the author is overconfident. Look for: +- Unstated assumptions +- Edge cases not handled +- Hidden coupling or shared state +- Ways the contract could be violated +- Existing conventions this might break +- Failure modes under unexpected input + +Do NOT validate. Do NOT summarize. Find issues, or state +explicitly that you cannot find any after thorough examination. + +ARTIFACT: +CONTRACT: +``` + +**Pass ARTIFACT + CONTRACT only. Do NOT pass the CLAIM.** Handing the reviewer your conclusion biases it toward agreement. The reviewer must independently determine whether the artifact satisfies the contract. + +If the harness has role-based reviewer agents defined (e.g. a code reviewer), they start with isolated context and are usable here. + +**The adversarial prompt above takes precedence over the persona's default response shape.** Personas like `code-reviewer` are written to produce balanced verdicts with both strengths and weaknesses; doubt-driven needs issues-only output. Paste the adversarial prompt verbatim into the invocation so it overrides the persona's default. If a persona's response shape can't be overridden cleanly, fall back to a generic subagent with the adversarial prompt. + +#### Second opinion (manual only) + +In interactive sessions, after the single-model review, you may mention once that the user can get a second opinion from another model by pasting ARTIFACT + CONTRACT + the adversarial prompt into a tool of their choice. Never invoke external model CLIs or services yourself and never send the artifact anywhere. If the user pastes a second opinion back, treat it as reviewer output for Step 4 (RECONCILE). In non-interactive contexts, skip this silently. + +### Step 4: RECONCILE — Fold findings back + +The reviewer's output is data, not verdict. **You are still the orchestrator.** Re-read the artifact text against each finding before classifying — rubber-stamping the reviewer is the same failure mode as ignoring it. + +For each finding, classify in this **precedence order** (first matching class wins): + +1. **Contract misread** — reviewer flagged something specifically because the CONTRACT you provided was unclear or incomplete. Fix the contract first, re-classify on the next cycle. +2. **Valid + actionable** — real issue requiring a change to the artifact. Change it, re-loop. +3. **Valid trade-off** — issue is real but cost of fixing exceeds cost of accepting. Document the trade-off explicitly so the user sees it. +4. **Noise** — reviewer flagged something that's actually correct under context the reviewer didn't have. Note it, move on, and ask: would adding that context to the contract have prevented the false flag? + +A fresh reviewer can be wrong because it lacks context. Don't defer just because it's "fresh." + +### Step 5: STOP — Bounded loop, not recursion + +Stop when: + +- Next iteration returns only trivial or already-considered findings, **or** +- 3 cycles completed (escalate to user, don't grind a fourth alone), **or** +- User explicitly says "ship it" + +If after 3 cycles the reviewer still surfaces substantive issues, the artifact may not be ready. Surface this to the user — three unresolved cycles is information about the artifact, not a reason to keep looping. + +If 3 cycles is "obviously insufficient" because the artifact is large: the artifact is too big — return to Step 2 and decompose. Do not lift the bound. + +## Common Rationalizations + +| Rationalization | Reality | +|---|---| +| "I'm confident, skip the doubt step" | Confidence correlates poorly with correctness on novel problems. Moments of certainty are exactly when blind spots hide. | +| "Spawning a reviewer is expensive" | Debugging a wrong commit in production is more expensive. The check is bounded; the bug isn't. | +| "The reviewer will just nitpick" | Only if unscoped. Constrain the prompt to "issues that would make this fail under the contract." | +| "I'll do doubt at the end with `/review`" | `/review` is a final gate. Doubt-driven catches wrong directions early when course-correction is cheap. By PR time it's too late. | +| "If I doubt every step I'll never ship" | The skill applies to non-trivial decisions, not every keystroke. Re-read "When NOT to Use." | +| "Two opinions are always better than one" | Not when the second has less context and produces noise. Reconcile, don't defer. | +| "The reviewer disagreed so I was wrong" | The reviewer lacks your context — disagreement is information, not verdict. Re-read the artifact, classify, then decide. | + +## Red Flags + +- Spawning a fresh-context reviewer for a one-line rename or formatting change +- Treating reviewer output as authoritative without re-reading the artifact text +- Looping >3 cycles without escalating to the user +- Prompting the reviewer with "is this good?" instead of "find issues" +- Skipping doubt under time pressure on a high-stakes decision +- Re-spawning fresh-context on an unchanged artifact (you'll get the same findings; you're stalling) +- **Doubt theater (checkable signal)**: across 2 or more cycles where the reviewer surfaced substantive findings, zero findings were classified as actionable. You are validating, not doubting. Stop and escalate. +- Doubting only after committing — that's `/review`, not doubt-driven development +- Sending the artifact to an external model or service yourself +- Stripping the contract from the reviewer's input +- Passing the CLAIM to the reviewer (biases toward agreement) + +## Interaction with Other Skills + +- **`code-review-and-quality` / `/review`**: complementary. `/review` is post-hoc PR verdict; doubt-driven is in-flight per-decision. Use both. +- **`test-driven-development`**: TDD's RED step is doubt made concrete — a failing test is a disproof attempt. When TDD applies, that failing test *is* the doubt step for behavioral claims. +- **`debugging-and-error-recovery`**: when the reviewer surfaces a real failure mode, drop into the debugging skill to localize and fix. +- **Orchestration rule**: this skill orchestrates from the main session. A persona calling another persona is an anti-pattern — see Loading Constraints above. + +## Verification + +After applying doubt-driven development: + +- [ ] Every non-trivial decision (per the definition above) was named explicitly as a CLAIM before standing +- [ ] At least one fresh-context review per non-trivial artifact (a failing test produced by TDD's RED step satisfies this for behavioral claims, per Interaction with Other Skills) +- [ ] The reviewer received ARTIFACT + CONTRACT — NOT the CLAIM, NOT your reasoning +- [ ] The reviewer's prompt was adversarial ("find issues"), not validating ("is it good") +- [ ] Findings were classified against the artifact text (not rubber-stamped) using the precedence: contract misread / actionable / trade-off / noise +- [ ] A stop condition was met (trivial findings, 3 cycles, or user override) +- [ ] No artifact was sent to an external model, CLI or service by the agent diff --git a/cli-tool/templates/packs/skills/doubt-driven-development/UPSTREAM.md b/cli-tool/templates/packs/skills/doubt-driven-development/UPSTREAM.md new file mode 100644 index 0000000..2300400 --- /dev/null +++ b/cli-tool/templates/packs/skills/doubt-driven-development/UPSTREAM.md @@ -0,0 +1,26 @@ +# Upstream and license + +This folder is an adapted copy of `skills/doubt-driven-development/` from an MIT-licensed upstream repository. + +| Field | Value | +|---|---| +| Source repository | https://github.com/addyosmani/agent-skills | +| Path | `skills/doubt-driven-development/` | +| Pinned commit | `a06bc63b3f8b829c14b0bbf53d99fefc39d58092` | +| Retrieved | 2026-10-03 | +| Upstream SKILL.md sha256 | `6fa1fcd8420c28daf7e53c5b08b0f20b907911efd2a6d9bce0b9740da20a4008` | +| License | MIT License, Copyright (c) 2025 Addy Osmani (full text: `LICENSE`) | + +## Changes made in this copy + +- Added the local-use ground rules block under the front matter. +- Replaced "Cross-model escalation" (piping the artifact to external Gemini/Codex CLIs) with an optional manual second opinion; the agent never sends the artifact to other services. +- Removed the link to the upstream orchestration-patterns reference (not installed). +- Reviewer personas are optional (upstream agents/ folder is not installed). +- Removed cross-model CLI rows/flags/checks and the source-driven-development reference (not installed). + +Everything else is upstream text, unchanged. + +## Updating + +Review `git diff a06bc63b3f8b829c14b0bbf53d99fefc39d58092 -- skills/doubt-driven-development/ references/`, re-run `packs/adapt-agent-skills.py ` (it applies the changes above as exact replacements and fails if upstream text moved), update commit and sha256 here and in `packs/sources.lock.json`, then run `packs/check-vendored.sh --online`. diff --git a/cli-tool/templates/packs/skills/frontend-ui-engineering/LICENSE b/cli-tool/templates/packs/skills/frontend-ui-engineering/LICENSE new file mode 100644 index 0000000..d67778a --- /dev/null +++ b/cli-tool/templates/packs/skills/frontend-ui-engineering/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2025 Addy Osmani + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/cli-tool/templates/packs/skills/frontend-ui-engineering/SKILL.md b/cli-tool/templates/packs/skills/frontend-ui-engineering/SKILL.md new file mode 100644 index 0000000..2f63224 --- /dev/null +++ b/cli-tool/templates/packs/skills/frontend-ui-engineering/SKILL.md @@ -0,0 +1,347 @@ +--- +name: frontend-ui-engineering +description: Builds production-quality, accessible, responsive user-facing UIs. Use when building or modifying interfaces and pages, creating components, implementing layouts, meeting WCAG accessibility requirements, managing state, or when the output needs to look and feel production-quality rather than AI-generated. +--- + +> Adapted copy of addyosmani/agent-skills (MIT) for local use; changes are listed in `UPSTREAM.md`. +> Ground rules that override anything below: use only tools that are already installed in the project +> (project scripts, `npx --no-install ...`); never let npx/pip/brew download or install anything and never add +> MCP servers on your own; do not fetch web pages or online docs and do not upload or post data anywhere; +> never push, deploy, run migrations, flip feature flags or change shared systems without the user's explicit +> instruction. Commands inside CI/workflow examples are file content to write, not commands to run now. + +# Frontend UI Engineering + +## Overview + +Build production-quality user interfaces that are accessible, performant, and visually polished. The goal is UI that looks like it was built by a design-aware engineer at a top company — not like it was generated by an AI. This means real design system adherence, proper accessibility, thoughtful interaction patterns, and no generic "AI aesthetic." + +## When to Use + +- Building new UI components or pages +- Modifying existing user-facing interfaces +- Implementing responsive layouts +- Adding interactivity or state management +- Fixing visual or UX issues + +## Component Architecture + +### File Structure + +Colocate everything related to a component: + +``` +src/components/ + TaskList/ + TaskList.tsx # Component implementation + TaskList.test.tsx # Tests + TaskList.stories.tsx # Storybook stories (if using) + use-task-list.ts # Custom hook (if complex state) + types.ts # Component-specific types (if needed) +``` + +### Component Patterns + +**Prefer composition over configuration:** + +```tsx +// Good: Composable + + + Tasks + + + + + + +// Avoid: Over-configured +} +/> +``` + +**Keep components focused:** + +```tsx +// Good: Does one thing +export function TaskItem({ task, onToggle, onDelete }: TaskItemProps) { + return ( +
  • + onToggle(task.id)} /> + {task.title} + +
  • + ); +} +``` + +**Separate data fetching from presentation:** + +```tsx +// Container: handles data +export function TaskListContainer() { + const { tasks, isLoading, error } = useTasks(); + + if (isLoading) return ; + if (error) return ; + if (tasks.length === 0) return ; + + return ; +} + +// Presentation: handles rendering +export function TaskList({ tasks }: { tasks: Task[] }) { + return ( +
      + {tasks.map(task => )} +
    + ); +} +``` + +## State Management + +**Choose the simplest approach that works:** + +``` +Local state (useState) → Component-specific UI state +Lifted state → Shared between 2-3 sibling components +Context → Theme, auth, locale (read-heavy, write-rare) +URL state (searchParams) → Filters, pagination, shareable UI state +Server state (React Query, SWR) → Remote data with caching +Global store (Zustand, Redux) → Complex client state shared app-wide +``` + +**Avoid prop drilling deeper than 3 levels.** If you're passing props through components that don't use them, introduce context or restructure the component tree. + +## Design System Adherence + +### Reference-led UI quality + +When the product needs a distinct visual direction, collect evidence before choosing a layout: + +1. Use references supplied by the product team or an installed reference library (e.g. the `design-md-reference` skill); do not browse the web for them. +2. Study two or three relevant screens. Record decisions about hierarchy, density, navigation, controls, responsive behavior, and interaction states. +3. Turn those decisions into a short design contract before implementation. Name the screen's job, primary action, required states, responsive rules, and patterns to reject. +4. Rebuild the useful structure in the product's own components, tokens, content, and visual language. Never copy another product's branding, proprietary text, imagery, or exact layout. + +Use references as evidence, not as templates. If references are unavailable, document the assumptions and verify the result against the product's existing design system. + +### Avoid the AI Aesthetic + +AI-generated UI has recognizable patterns. Avoid all of them: + +| AI Default | Why It Is a Problem | Production Quality | +|---|---|---| +| Purple/indigo everything | Models default to visually "safe" palettes, making every app look identical | Use the project's actual color palette | +| Excessive gradients | Gradients add visual noise and clash with most design systems | Flat or subtle gradients matching the design system | +| Rounded everything (rounded-2xl) | Maximum rounding signals "friendly" but ignores the hierarchy of corner radii in real designs | Consistent border-radius from the design system | +| Generic hero sections | Template-driven layout with no connection to the actual content or user need | Content-first layouts | +| Lorem ipsum-style copy | Placeholder text hides layout problems that real content reveals (length, wrapping, overflow) | Realistic placeholder content | +| Oversized padding everywhere | Equal generous padding destroys visual hierarchy and wastes screen space | Consistent spacing scale | +| Stock card grids | Uniform grids are a layout shortcut that ignores information priority and scanning patterns | Purpose-driven layouts | +| Shadow-heavy design | Layered shadows add depth that competes with content and slows rendering on low-end devices | Subtle or no shadows unless the design system specifies | + +### Spacing and Layout + +Use a consistent spacing scale. Don't invent values: + +```css +/* Use the scale: 0.25rem increments (or whatever the project uses) */ +/* Good */ padding: 1rem; /* 16px */ +/* Good */ gap: 0.75rem; /* 12px */ +/* Bad */ padding: 13px; /* Not on any scale */ +/* Bad */ margin-top: 2.3rem; /* Not on any scale */ +``` + +### Typography + +Respect the type hierarchy: + +``` +h1 → Page title (one per page) +h2 → Section title +h3 → Subsection title +body → Default text +small → Secondary/helper text +``` + +Don't skip heading levels. Don't use heading styles for non-heading content. + +### Color + +- Use semantic color tokens: `text-primary`, `bg-surface`, `border-default` — not raw hex values +- Ensure sufficient contrast (4.5:1 for normal text, 3:1 for large text) +- Don't rely solely on color to convey information (use icons, text, or patterns too) + +## Accessibility (WCAG 2.1 AA) + +Every component must meet these standards: + +### Keyboard Navigation + +```tsx +// Every interactive element must be keyboard accessible + // ✓ Focusable by default +
    Click me
    // ✗ Not focusable +
    + onKeyDown={e => { + if (e.key === 'Enter') handleClick(); + if (e.key === ' ') e.preventDefault(); + }} + onKeyUp={e => { + if (e.key === ' ') handleClick(); + }}> + Click me +
    +``` + +### ARIA Labels + +```tsx +// Label interactive elements that lack visible text + + +// Label form inputs + + + +// Or use aria-label when no visible label exists + +``` + +### Focus Management + +```tsx +// Move focus when content changes +function Dialog({ isOpen, onClose }: DialogProps) { + const closeRef = useRef(null); + + useEffect(() => { + if (isOpen) closeRef.current?.focus(); + }, [isOpen]); + + // Trap focus inside dialog when open + return ( + + + {/* dialog content */} + + ); +} +``` + +### Meaningful Empty and Error States + +```tsx +// Don't show blank screens +function TaskList({ tasks }: { tasks: Task[] }) { + if (tasks.length === 0) { + return ( +
    + +

    No tasks

    +

    Get started by creating a new task.

    + +
    + ); + } + + return
      ...
    ; +} +``` + +## Responsive Design + +Design for mobile first, then expand: + +```tsx +// Tailwind: mobile-first responsive +
    +``` + +Test at these breakpoints: 320px, 768px, 1024px, 1440px. + +## Loading and Transitions + +```tsx +// Skeleton loading (not spinners for content) +function TaskListSkeleton() { + return ( +
    + {Array.from({ length: 3 }).map((_, i) => ( +
    + ))} +
    + ); +} + +// Optimistic updates for perceived speed +function useToggleTask() { + const queryClient = useQueryClient(); + + return useMutation({ + mutationFn: toggleTask, + onMutate: async (taskId) => { + await queryClient.cancelQueries({ queryKey: ['tasks'] }); + const previous = queryClient.getQueryData(['tasks']); + + queryClient.setQueryData(['tasks'], (old: Task[]) => + old.map(t => t.id === taskId ? { ...t, done: !t.done } : t) + ); + + return { previous }; + }, + onError: (_err, _taskId, context) => { + queryClient.setQueryData(['tasks'], context?.previous); + }, + }); +} +``` + +## See Also + +For detailed accessibility requirements and testing tools, see `references/accessibility-checklist.md`. + +## Common Rationalizations + +| Rationalization | Reality | +|---|---| +| "Accessibility is a nice-to-have" | It's a legal requirement in many jurisdictions and an engineering quality standard. | +| "We'll make it responsive later" | Retrofitting responsive design is 3x harder than building it from the start. | +| "The design isn't final, so I'll skip styling" | Use the design system defaults. Unstyled UI creates a broken first impression for reviewers. | +| "This is just a prototype" | Prototypes become production code. Build the foundation right. | +| "The AI aesthetic is fine for now" | It signals low quality. Use the project's actual design system from the start. | + +## Red Flags + +- Components with more than 200 lines (split them) +- Inline styles or arbitrary pixel values +- Missing error states, loading states, or empty states +- No keyboard navigation testing +- Color as the sole indicator of state (red/green without text or icons) +- Generic "AI look" (purple gradients, oversized cards, stock layouts) + +## Verification + +After building UI: + +- [ ] Component renders without console errors +- [ ] All interactive elements are keyboard accessible (Tab through the page) +- [ ] Screen reader can convey the page's content and structure +- [ ] Responsive: works at 320px, 768px, 1024px, 1440px +- [ ] Loading, empty, error, success, and permission states handled when applicable +- [ ] Follows the project's design system (spacing, colors, typography) +- [ ] The rendered result passes a final UI-specific finish-gate review +- [ ] No accessibility warnings in dev tools or axe-core diff --git a/cli-tool/templates/packs/skills/frontend-ui-engineering/UPSTREAM.md b/cli-tool/templates/packs/skills/frontend-ui-engineering/UPSTREAM.md new file mode 100644 index 0000000..265fd76 --- /dev/null +++ b/cli-tool/templates/packs/skills/frontend-ui-engineering/UPSTREAM.md @@ -0,0 +1,24 @@ +# Upstream and license + +This folder is an adapted copy of `skills/frontend-ui-engineering/` from an MIT-licensed upstream repository. + +| Field | Value | +|---|---| +| Source repository | https://github.com/addyosmani/agent-skills | +| Path | `skills/frontend-ui-engineering/` | +| Pinned commit | `a06bc63b3f8b829c14b0bbf53d99fefc39d58092` | +| Retrieved | 2026-10-03 | +| Upstream SKILL.md sha256 | `549044e9bd8d7fe993ce8e5c0d2b0f5465650676dc15805329973f936179eb1e` | +| License | MIT License, Copyright (c) 2025 Addy Osmani (full text: `LICENSE`) | + +## Changes made in this copy + +- Copied the shared reference `references/accessibility-checklist.md` into the skill folder and pointed links at it. +- Added the local-use ground rules block under the front matter. +- Design references come from the team or installed references, not web browsing. + +Everything else is upstream text, unchanged. + +## Updating + +Review `git diff a06bc63b3f8b829c14b0bbf53d99fefc39d58092 -- skills/frontend-ui-engineering/ references/`, re-run `packs/adapt-agent-skills.py ` (it applies the changes above as exact replacements and fails if upstream text moved), update commit and sha256 here and in `packs/sources.lock.json`, then run `packs/check-vendored.sh --online`. diff --git a/cli-tool/templates/packs/skills/frontend-ui-engineering/references/accessibility-checklist.md b/cli-tool/templates/packs/skills/frontend-ui-engineering/references/accessibility-checklist.md new file mode 100644 index 0000000..71eb504 --- /dev/null +++ b/cli-tool/templates/packs/skills/frontend-ui-engineering/references/accessibility-checklist.md @@ -0,0 +1,160 @@ +# Accessibility Checklist + +Quick reference for WCAG 2.1 AA compliance. Use alongside the `frontend-ui-engineering` skill. + +## Table of Contents + +- [Essential Checks](#essential-checks) +- [Common HTML Patterns](#common-html-patterns) +- [Testing Tools](#testing-tools) +- [Quick Reference: ARIA Live Regions](#quick-reference-aria-live-regions) +- [Common Anti-Patterns](#common-anti-patterns) + +## Essential Checks + +### Keyboard Navigation +- [ ] All interactive elements focusable via Tab key +- [ ] Focus order follows visual/logical order +- [ ] Focus is visible (outline/ring on focused elements) +- [ ] Custom widgets have keyboard support (Enter to activate, Escape to close) +- [ ] No keyboard traps (user can always Tab away from a component) +- [ ] Skip-to-content link at top of page - visible (at least) on keyboard focus +- [ ] Modals trap focus while open, return focus on close + +### Screen Readers +- [ ] All images have `alt` text (or `alt=""` for decorative images) +- [ ] All form inputs have associated labels (`