diff --git a/.changeset/guided-maintainer-adoption.md b/.changeset/guided-maintainer-adoption.md index 76eb881e..6dc26c02 100644 --- a/.changeset/guided-maintainer-adoption.md +++ b/.changeset/guided-maintainer-adoption.md @@ -2,4 +2,4 @@ '@tanstack/intent': minor --- -Add guided adoption of existing package-owned skills with a read-only JSON plan, explicit batch registration and distribution choices, and interactive confirmation. Preserve authored guidance and prior records, reject stale plans, leave semantic review pending, and keep CI noninteractive. +`maintainer setup` replaces `maintainer adopt`, which is removed along with its JSON plan and `--apply`; update any script that called it. Setup registers existing package-owned skills found under `skills/` directories that the skill tree does not record yet. Skill contents are preserved; the domain comes from `metadata.domain`, the domain map, or the parent directory, and defaults to `uncategorized` for the maintainer to edit. Invalid or conflicting skills are reported and left unregistered. Agent skill directories, dependencies, and packages outside the workspace are not scanned. diff --git a/benchmarks/intent/maintainer.bench.ts b/benchmarks/intent/maintainer.bench.ts new file mode 100644 index 00000000..853353cf --- /dev/null +++ b/benchmarks/intent/maintainer.bench.ts @@ -0,0 +1,72 @@ +import { execFileSync, spawnSync } from 'node:child_process' +import { rmSync } from 'node:fs' +import { join } from 'node:path' +import { fileURLToPath } from 'node:url' +import { afterAll, beforeAll, bench, describe } from 'vitest' +import { createTempDir, writeFile, writeJson, writeSkill } from './helpers.ts' + +const cliPath = fileURLToPath( + new URL('../../packages/intent/dist/cli.mjs', import.meta.url), +) + +for (const count of [20, 200]) { + describe(`maintainer setup with ${count} existing skills`, () => { + let root: string | undefined + function setup() { + if (root) return + root = createTempDir('maintainer-setup') + writeJson(join(root, 'package.json'), { + name: '@bench/library', + version: '1.0.0', + }) + writeFile( + join(root, '.github/workflows/check-skills.yml'), + '# Existing repository workflow\n', + ) + for (let index = 0; index < count; index++) { + writeSkill(root, `task-${index}`, { + description: `Use for task ${index}.`, + sources: ['package.json'], + }) + } + execFileSync('git', ['-c', 'core.fsmonitor=false', 'init', '-q'], { + cwd: root, + }) + } + function teardown() { + if (root) rmSync(root, { recursive: true, force: true }) + root = undefined + } + beforeAll(setup) + afterAll(teardown) + + bench( + 'registers the complete batch', + () => { + setup() + // Repeat first setup, without including fixture creation or a network lookup. + for (const path of ['skills/_artifacts', '.intent', 'AGENTS.md']) { + rmSync(join(root!, path), { recursive: true, force: true }) + } + const result = spawnSync( + process.execPath, + [cliPath, 'maintainer', 'setup'], + { + cwd: root, + encoding: 'utf8', + timeout: 30_000, + }, + ) + if ( + result.status !== 0 || + result.stdout.split('Registered skills/').length - 1 !== count + ) { + throw new Error( + `Incomplete registration: ${result.stdout}${result.stderr}`, + ) + } + }, + { warmupIterations: 3, time: 3_000, setup, teardown }, + ) + }) +} diff --git a/packages/intent/src/cli.ts b/packages/intent/src/cli.ts index 8f48be28..06580f25 100644 --- a/packages/intent/src/cli.ts +++ b/packages/intent/src/cli.ts @@ -197,7 +197,7 @@ function createCli( 'Set up, author, synchronize, and check library skills', ) .usage( - 'maintainer [name] [options]', + 'maintainer [name] [options]', ) .option( '--artifacts ', @@ -207,14 +207,7 @@ function createCli( '--package ', 'Owning package directory, relative to the repository root', ) - .option( - '--path ', - 'Skill path for add, or repository-relative custom directory for adopt', - ) - .option( - '--apply ', - 'Apply reviewed adoption choices from a JSON plan', - ) + .option('--path ', 'Skill path relative to the owning package') .option('--domain ', 'Domain for a new skill') .option( '--distribution ', @@ -247,15 +240,12 @@ function createCli( '--interactive', 'Inspect and record maintainer review outcomes in a terminal', ) - .option('--json', 'Output an adoption plan, status, or review as JSON') + .option('--json', 'Output status or review as JSON') .option( '--record ', 'Record outcomes from an annotated review report', ) .example('maintainer setup') - .example('maintainer adopt') - .example('maintainer adopt --json') - .example('maintainer adopt --apply adoption.json') .example( 'maintainer add caching --domain queries --description "Use when caching queries." --source "src/**"', ) diff --git a/packages/intent/src/commands/maintainer.ts b/packages/intent/src/commands/maintainer.ts index 168e8e7c..353451ff 100644 --- a/packages/intent/src/commands/maintainer.ts +++ b/packages/intent/src/commands/maintainer.ts @@ -1,5 +1,4 @@ -import { readFileSync } from 'node:fs' -import { dirname, relative, resolve } from 'node:path' +import { dirname, relative } from 'node:path' import { isCI } from 'std-env' import { resolveProjectContext } from '../core/project-context.js' import { fail } from '../shared/cli-error.js' @@ -8,9 +7,9 @@ import { resolveMaintainerProject, setupRecords, } from '../maintainer/project.js' -import { addSkill } from '../maintainer/add.js' +import { addSkill, planAddSkills } from '../maintainer/add.js' +import { findExistingSkills } from '../maintainer/existing.js' import { retireSkill } from '../maintainer/remove.js' -import { createAdoptionPlan, planAdoptionChanges } from '../maintainer/adopt.js' import { planMaintainerSync } from '../maintainer/sync.js' import { withMaintainerLock, writeChanges } from '../maintainer/files.js' import { createReview } from '../review/review.js' @@ -28,13 +27,11 @@ import { import { runReviewCommand } from './review.js' import { runValidateCommand } from './validate.js' import type { DistributionOptions } from '../maintainer/distribution.js' -import type { AdoptionPrompts } from '../maintainer/adopt.js' import type { ReviewPrompts } from '../review/interactive.js' export interface MaintainerCommandRuntime { isTTY?: boolean isCI?: boolean - adoptionPrompts?: AdoptionPrompts reviewPrompts?: ReviewPrompts } @@ -55,11 +52,6 @@ const optionHelp: Record = { 'Owning package directory, relative to the repository root (default: the package that owns the current directory)', ], path: ['--path ', 'Skill path relative to the owning package'], - adoptPath: [ - '--path ', - 'Repository-relative custom skill directory to scan', - ], - apply: ['--apply ', 'Apply reviewed adoption choices from a JSON plan'], domain: ['--domain ', 'Task domain for the skill'], distribution: [ '--distribution ', @@ -89,9 +81,10 @@ const optionHelp: Record = { export const maintainerActions: Record = { setup: { usage: 'maintainer setup [--distribution repo|none] [options]', - summary: 'Initialize planning records, agent instructions, and CI.', + summary: + 'Initialize planning records, register existing skills, install agent instructions and CI.', writes: - 'skill_tree.yaml, domain_map.yaml, skill_spec.md, the intent-maintainer block in AGENTS.md (or the existing agent instruction file), and .github/workflows/check-skills.yml when it does not exist.', + 'skill_tree.yaml, domain_map.yaml, skill_spec.md (registering any SKILL.md under skills/ that is not yet recorded), the intent-maintainer block in AGENTS.md (or the existing agent instruction file), and .github/workflows/check-skills.yml when it does not exist.', options: [ 'artifacts', 'distribution', @@ -100,16 +93,6 @@ export const maintainerActions: Record = { 'skill', ].map((key) => optionHelp[key]!), }, - adopt: { - usage: - 'maintainer adopt [--json | --apply ] [--path ]', - summary: 'Register existing skills from a reviewed plan.', - writes: - 'The three planning records and the agent instruction block. Skill contents stay as authored.', - options: ['artifacts', 'json', 'adoptPath', 'apply'].map( - (key) => optionHelp[key]!, - ), - }, add: { usage: 'maintainer add --domain [--description --source ...] [options]', @@ -212,7 +195,6 @@ export interface MaintainerCommandOptions extends DistributionOptions { base?: string json?: boolean record?: string - apply?: string interactive?: boolean } @@ -239,7 +221,6 @@ export async function runMaintainerCommand( ): Promise { const allowed: Record> = { setup: ['artifacts', 'distribution', 'repository', 'pluginName', 'skill'], - adopt: ['artifacts', 'json', 'path', 'apply'], add: [ 'artifacts', 'package', @@ -258,7 +239,7 @@ export async function runMaintainerCommand( } if (!allowed[action]) fail( - `Unknown maintainer action: ${action}. Expected setup, adopt, add, remove, status, sync, review, or check.`, + `Unknown maintainer action: ${action}. Expected setup, add, remove, status, sync, review, or check.`, ) if (name !== undefined && action !== 'add' && action !== 'remove') fail(`maintainer ${action} does not take a skill name.`) @@ -290,71 +271,38 @@ export async function runMaintainerCommand( return } const project = resolveMaintainerProject(process.cwd(), options.artifacts) - if (action === 'adopt') { - let input: unknown - if (options.apply) { - if (options.json || options.path) - fail('--apply cannot be combined with --json or --path.') - input = JSON.parse(readFileSync(resolve(options.apply), 'utf8')) - } else { - const plan = createAdoptionPlan(project, options.path) - if (options.json) { - console.log(JSON.stringify(plan, null, 2)) - return - } - if ( - (runtime.isCI ?? isCI) || - !(runtime.isTTY ?? (process.stdin.isTTY && process.stdout.isTTY)) - ) - fail( - 'Use maintainer adopt --json to preview, then --apply with explicit choices in noninteractive sessions.', - ) - for (const skill of plan.skills) - console.log( - `${JSON.stringify(skill.id)}: ${skill.status}${skill.problems.length ? ` (${skill.problems.join('; ')})` : ''}`, - ) - const prompts = - runtime.adoptionPrompts ?? - ( - await import('../maintainer/adoption-prompts.js') - ).createAdoptionPrompts() - const chosen = await prompts.choose(plan) - if (chosen === null) { - console.log('Adoption canceled. No files changed.') - return - } - const preview = planAdoptionChanges(project, chosen) - const files = preview.changes.map((change) => - relative(project.root, change.path).replaceAll('\\', '/'), - ) - if (!(await prompts.confirm(chosen, files))) { - console.log('Adoption canceled. No files changed.') - return - } - input = chosen - } - await withMaintainerLock(project.root, () => { - const plan = planAdoptionChanges(project, input) - writeChanges(project.root, plan.changes) - writeIntentSkillsBlock({ - ...buildMaintainerGuidanceBlock( - detectIntentCommandPackageManager(project.root), - ), - root: project.root, - namespace: 'intent-maintainer', - skipWhenEmpty: false, - }) - console.log( - `Registered ${plan.paths.length} skill(s). Authored task coverage and source review remain required.`, - ) - }) - return - } if (['setup', 'add', 'remove', 'sync'].includes(action)) { await withMaintainerLock(project.root, () => { if (action === 'setup') { const created = setupRecords(project) configureDistribution(project, options) + const existing = findExistingSkills(project) + const candidates = existing.filter((skill) => !skill.problems.length) + const registered = planAddSkills( + project, + candidates.map((skill) => ({ + name: skill.name, + options: { + package: skill.package || undefined, + path: skill.path, + domain: skill.domain, + }, + })), + [], + (index, error) => { + const candidate = candidates[index]! + // A blank domain only comes from a domain_map.yaml entry, and + // setup has no --domain flag to point the maintainer at. + candidate.problems.push( + candidate.domain.trim() + ? error instanceof Error + ? error.message + : String(error) + : `Set a non-empty domain for ${candidate.name} in ${project.artifacts}/domain_map.yaml, then run intent maintainer setup again.`, + ) + }, + ) + writeChanges(project.root, registered.changes) runSetupGithubActions(project.root, getMetaDir()) writeIntentSkillsBlock({ ...buildMaintainerGuidanceBlock( @@ -370,9 +318,16 @@ export async function runMaintainerCommand( console.log( 'Next: intent maintainer add --domain --description --source . Use --package for a workspace package. Use intent meta generate-skill for the authoring procedure.', ) - console.log( - 'For existing skills, run intent maintainer adopt to review registrations.', - ) + for (const skill of existing) + console.log( + skill.problems.length + ? `Skipped ${skill.id}: ${skill.problems.join(' ')}` + : `Registered ${skill.id} (domain ${skill.domain}).`, + ) + if (existing.some((skill) => skill.domain === 'uncategorized')) + console.log( + `Set a domain for uncategorized skills in ${project.artifacts}/skill_tree.yaml and domain_map.yaml.`, + ) const distribution = readDistribution( readRecord(project, 'skill_tree.yaml'), ) diff --git a/packages/intent/src/maintainer/add.ts b/packages/intent/src/maintainer/add.ts index 2468b1e3..9fc9ab42 100644 --- a/packages/intent/src/maintainer/add.ts +++ b/packages/intent/src/maintainer/add.ts @@ -54,11 +54,20 @@ export function planAddSkills( project: MaintainerProject, additions: Array<{ name: string | undefined; options: AddSkillOptions }>, initialChanges: Array = [], + onInvalid?: (index: number, error: unknown) => void, ) { const changes = [...initialChanges] const tree = readRecord(project, 'skill_tree.yaml', changes) const entries = skillEntries(project, tree) + const names = new Set( + entries.map((entry) => String(entry.slug ?? entry.name)), + ) + const registeredPaths = new Set( + entries.map((entry) => skillPath(project, entry)), + ) const map = readRecord(project, 'domain_map.yaml', changes) + const mapSkills: Array> = map.document.toJS().skills + const mappedNames = new Set(mapSkills.map((skill) => skill.slug)) const specPath = recordPath(project, 'skill_spec.md') const specChange = changes.find((change) => change.path === specPath) const spec = existsSync(specPath) ? readFileSync(specPath, 'utf8') : null @@ -66,106 +75,28 @@ export function planAddSkills( if (nextSpec === null) throw new Error('Missing skill_spec.md. Run intent maintainer setup.') const paths: Array = [] - for (const { name, options } of additions) { - if (!name || name.length > 64 || !/^[a-z0-9]+(?:-[a-z0-9]+)*$/.test(name)) - throw new Error( - 'Choose a skill name of at most 64 lowercase letters, numbers, and hyphens.', - ) - if (!options.domain?.trim()) - throw new Error('Choose the task domain with --domain .') - if (entries.some((entry) => (entry.slug ?? entry.name) === name)) - throw new Error( - `Skill ${name} is already registered. Edit its SKILL.md, then run intent maintainer sync.`, - ) - const packageDir = options.package === '.' ? undefined : options.package - const entry: SkillEntry = { - name, - slug: name, - domain: options.domain, - ...(packageDir ? { package: packageDir } : {}), - path: options.path ?? `skills/${name}/SKILL.md`, + for (const [index, { name, options }] of additions.entries()) { + let addition: ReturnType + try { + addition = prepareAddition(project, name, options, names, registeredPaths) + } catch (error) { + if (!onInvalid) throw error + onInvalid(index, error) + continue } - const path = skillPath(project, entry) - if (basename(dirname(path)) !== name) - throw new Error('The skill name must match its parent directory.') - if (entries.some((existing) => skillPath(project, existing) === path)) - throw new Error(`Skill path is already registered: ${entry.path}`) - const packageRoot = packageDir - ? projectPath(project.root, packageDir) - : project.root - const manifestPath = projectPath( - project.root, - packageDir ? `${packageDir}/package.json` : 'package.json', - ) - const manifest = JSON.parse(readFileSync(manifestPath, 'utf8')) - if ( - resolveProjectContext({ cwd: project.root, targetPath: path }) - .packageRoot !== packageRoot - ) - throw new Error('The skill path must belong to the selected package.') - let frontmatter: Record - if (existsSync(path)) { - if (options.description || options.source || options.requires) - throw new Error( - 'To register an existing skill, supply its --path and --domain; edit its frontmatter directly before running sync.', - ) - const parsed = parseFrontmatter(path) - if (!isObject(parsed) || parsed.name !== name) - throw new Error( - 'Existing skill has invalid frontmatter or a different name.', - ) - frontmatter = parsed - } else { - if (!options.description?.trim()) - throw new Error('A new skill needs --description .') - const sources = stringList( - options.source === undefined ? [] : [options.source].flat(), - 'sources', - ) - if (!sources.length) - throw new Error('A new skill needs at least one --source .') - frontmatter = { - name, - description: options.description, - metadata: { library: manifest.name }, - sources, - ...(options.requires - ? { requires: stringList([options.requires].flat(), 'requires') } - : {}), - } - changes.push({ - path, - source: null, - content: `---\n${stringify(frontmatter)}---\n\n${authoringMarker}\n\nWrite the task procedure, working examples, source-backed pitfalls, and completion checks. Add metadata.purpose in your own words. Remove the authoring marker after writing and checking the guidance.\n`, - }) - } - if ( - typeof frontmatter.description !== 'string' || - !frontmatter.description.trim() - ) - throw new Error('A skill needs a non-empty description.') - entry.description = frontmatter.description - entry.sources = stringList(frontmatter.sources ?? [], 'sources') - entry.requires = stringList(frontmatter.requires ?? [], 'requires') - if ( - isObject(frontmatter.metadata) && - typeof frontmatter.metadata.purpose === 'string' - ) - entry.purpose = frontmatter.metadata.purpose + const { entry, path, packageDir, manifestName, tasks, change } = addition + if (change) changes.push(change) + names.add(String(entry.slug ?? entry.name)) + registeredPaths.add(path) tree.document.addIn(['skills'], entry) - entries.push(entry) - const tasks = stringList( - options.task === undefined ? [] : [options.task].flat(), - 'tasks', - ) - const mapSkills: Array> = map.document.toJS().skills - if (!mapSkills.some((skill) => skill.slug === name)) { + if (!mappedNames.has(name)) { + mappedNames.add(name) map.document.addIn(['skills'], { name, slug: name, domain: options.domain, description: entry.purpose ?? entry.description, - ...(packageDir ? { packages: [manifest.name] } : {}), + ...(packageDir ? { packages: [manifestName] } : {}), tasks, covers: [], }) @@ -173,7 +104,7 @@ export function planAddSkills( nextSpec = `${nextSpec.trimEnd()}\n\n- Registered \`${name}\` in \`${packageDir ?? '.'}\` (domain \`${options.domain}\`).${tasks.length ? ` Developer tasks: ${tasks.join('; ')}.` : ''} ${tasks.length ? 'Decisions and checks' : 'Task coverage, decisions, and checks'} still need to be recorded.\n` paths.push(join(packageDir ?? '', entry.path).replaceAll('\\', '/')) } - if (additions.length) { + if (paths.length) { changes.push( { path: tree.path, @@ -191,3 +122,104 @@ export function planAddSkills( paths, } } + +// Validate one candidate completely before adding it to the shared batch. +function prepareAddition( + project: MaintainerProject, + name: string | undefined, + options: AddSkillOptions, + names: Set, + registeredPaths: Set, +) { + if (!name || name.length > 64 || !/^[a-z0-9]+(?:-[a-z0-9]+)*$/.test(name)) + throw new Error( + 'Choose a skill name of at most 64 lowercase letters, numbers, and hyphens.', + ) + if (!options.domain?.trim()) + throw new Error('Choose the task domain with --domain .') + if (names.has(name)) + throw new Error( + `Skill ${name} is already registered. Edit its SKILL.md, then run intent maintainer sync.`, + ) + const packageDir = options.package === '.' ? undefined : options.package + const entry: SkillEntry = { + name, + slug: name, + domain: options.domain, + ...(packageDir ? { package: packageDir } : {}), + path: options.path ?? `skills/${name}/SKILL.md`, + } + const path = skillPath(project, entry) + if (basename(dirname(path)) !== name) + throw new Error('The skill name must match its parent directory.') + if (registeredPaths.has(path)) + throw new Error(`Skill path is already registered: ${entry.path}`) + const packageRoot = packageDir + ? projectPath(project.root, packageDir) + : project.root + const manifestPath = projectPath( + project.root, + packageDir ? `${packageDir}/package.json` : 'package.json', + ) + const manifest = JSON.parse(readFileSync(manifestPath, 'utf8')) + if ( + resolveProjectContext({ cwd: project.root, targetPath: path }) + .packageRoot !== packageRoot + ) + throw new Error('The skill path must belong to the selected package.') + let change: FileChange | undefined + let frontmatter: Record + if (existsSync(path)) { + if (options.description || options.source || options.requires) + throw new Error( + 'To register an existing skill, supply its --path and --domain; edit its frontmatter directly before running sync.', + ) + const parsed = parseFrontmatter(path) + if (!isObject(parsed) || parsed.name !== name) + throw new Error( + 'Existing skill has invalid frontmatter or a different name.', + ) + frontmatter = parsed + } else { + if (!options.description?.trim()) + throw new Error('A new skill needs --description .') + const sources = stringList( + options.source === undefined ? [] : [options.source].flat(), + 'sources', + ) + if (!sources.length) + throw new Error('A new skill needs at least one --source .') + frontmatter = { + name, + description: options.description, + metadata: { library: manifest.name }, + sources, + ...(options.requires + ? { requires: stringList([options.requires].flat(), 'requires') } + : {}), + } + change = { + path, + source: null, + content: `---\n${stringify(frontmatter)}---\n\n${authoringMarker}\n\nWrite the task procedure, working examples, source-backed pitfalls, and completion checks. Add metadata.purpose in your own words. Remove the authoring marker after writing and checking the guidance.\n`, + } + } + if ( + typeof frontmatter.description !== 'string' || + !frontmatter.description.trim() + ) + throw new Error('A skill needs a non-empty description.') + entry.description = frontmatter.description + entry.sources = stringList(frontmatter.sources ?? [], 'sources') + entry.requires = stringList(frontmatter.requires ?? [], 'requires') + if ( + isObject(frontmatter.metadata) && + typeof frontmatter.metadata.purpose === 'string' + ) + entry.purpose = frontmatter.metadata.purpose + const tasks = stringList( + options.task === undefined ? [] : [options.task].flat(), + 'tasks', + ) + return { entry, path, packageDir, manifestName: manifest.name, tasks, change } +} diff --git a/packages/intent/src/maintainer/adopt.ts b/packages/intent/src/maintainer/adopt.ts deleted file mode 100644 index 7bf0a807..00000000 --- a/packages/intent/src/maintainer/adopt.ts +++ /dev/null @@ -1,357 +0,0 @@ -import { execFileSync } from 'node:child_process' -import { createHash } from 'node:crypto' -import { existsSync, readFileSync } from 'node:fs' -import { basename, dirname, relative } from 'node:path' -import { resolveProjectContext } from '../core/project-context.js' -import { resolveWorkspacePackages } from '../setup/workspace-patterns.js' -import { parseFrontmatter } from '../shared/utils.js' -import { planAddSkills, stringList } from './add.js' -import { - inferDistributionRepository, - planDistributionChoice, - readDistribution, -} from './distribution.js' -import { - isObject, - planSetupRecords, - projectPath, - readRecord, - recordPath, - skillEntries, - skillPath, -} from './project.js' -import type { MaintainerProject } from './project.js' - -export interface AdoptionSkill { - id: string - name: string - package: string - path: string - description: string - domain: string - status: - | 'unregistered' - | 'registered' - | 'missing' - | 'invalid' - | 'conflict' - | 'planned' - | 'retired' - problems: Array - selected: boolean -} - -export function createAdoptionPlan( - project: MaintainerProject, - directory?: string, -) { - if (directory) projectPath(project.root, directory) - const files = execFileSync( - 'git', - [ - '-c', - 'core.fsmonitor=false', - 'ls-files', - '--cached', - '--others', - '--exclude-standard', - '-z', - ], - { cwd: project.root, encoding: 'utf8', maxBuffer: 32 * 1024 * 1024 }, - ) - .split('\0') - .filter(Boolean) - const records = ['domain_map.yaml', 'skill_spec.md', 'skill_tree.yaml'].map( - (name) => { - const path = recordPath(project, name) - return [path, existsSync(path) ? readFileSync(path, 'utf8') : null] - }, - ) - const tree = existsSync(recordPath(project, 'skill_tree.yaml')) - ? readRecord(project, 'skill_tree.yaml') - : undefined - const entries = tree ? skillEntries(project, tree) : [] - const registered = new Map( - entries.map((entry) => [ - relative(project.root, skillPath(project, entry)).replaceAll('\\', '/'), - entry, - ]), - ) - const mapPath = recordPath(project, 'domain_map.yaml') - const mapped = existsSync(mapPath) - ? readRecord(project, 'domain_map.yaml').document.toJS().skills - : [] - const paths = [ - ...new Set([ - ...files.filter( - (path) => - basename(path) === 'SKILL.md' && - !path - .split('/') - .some((part) => part.startsWith('.') || part === 'node_modules') && - (/(^|\/)skills\//.test(path) || - (directory && path.startsWith(`${directory}/`))), - ), - ...registered.keys(), - ]), - ].sort() - const snapshot: Array = [ - records, - ...[ - 'package.json', - 'pnpm-workspace.yaml', - 'AGENTS.md', - 'CLAUDE.md', - '.cursorrules', - '.github/copilot-instructions.md', - '.claude-plugin/plugin.json', - '.cursor-plugin/plugin.json', - ].map((name) => { - const path = projectPath(project.root, name) - return [name, existsSync(path) ? readFileSync(path, 'utf8') : null] - }), - ] - const packageRoots = new Set([ - project.root, - ...resolveWorkspacePackages( - project.root, - resolveProjectContext({ cwd: project.root }).workspacePatterns, - ), - ]) - const skills = paths.flatMap((id) => { - const entry = registered.get(id) - const candidate: AdoptionSkill = { - id, - name: String(entry?.slug ?? entry?.name ?? basename(dirname(id))), - package: entry?.package ?? '', - path: entry?.path ?? id, - description: '', - domain: typeof entry?.domain === 'string' ? entry.domain : '', - status: entry ? 'registered' : 'unregistered', - problems: [], - selected: false, - } - if (entry?.status === 'planned' || entry?.status === 'retired') { - candidate.status = entry.status - return [candidate] - } - try { - const absolute = projectPath(project.root, id) - if (!existsSync(absolute)) { - candidate.status = 'missing' - candidate.problems.push('Registered skill file is missing.') - snapshot.push([id, null]) - return [candidate] - } - const context = resolveProjectContext({ - cwd: project.root, - targetPath: absolute, - }) - if (!context.packageRoot) - throw new Error('Skill has no owning package.json.') - if ( - !entry && - !packageRoots.has(context.packageRoot) && - !(directory && id.startsWith(`${directory}/`)) - ) - return [] - candidate.package = relative( - project.root, - context.packageRoot, - ).replaceAll('\\', '/') - candidate.path = relative(context.packageRoot, absolute).replaceAll( - '\\', - '/', - ) - const manifest = projectPath( - project.root, - candidate.package - ? `${candidate.package}/package.json` - : 'package.json', - ) - snapshot.push([ - id, - readFileSync(absolute, 'utf8'), - readFileSync(manifest, 'utf8'), - ]) - const frontmatter = parseFrontmatter(absolute) - if ( - !frontmatter || - typeof frontmatter.name !== 'string' || - !/^[a-z0-9]+(?:-[a-z0-9]+)*$/.test(frontmatter.name) || - frontmatter.name.length > 64 || - frontmatter.name !== basename(dirname(id)) || - (entry && frontmatter.name !== candidate.name) - ) - throw new Error( - 'Skill name must match its directory and registered identity.', - ) - candidate.name = frontmatter.name - if ( - typeof frontmatter.description !== 'string' || - !frontmatter.description.trim() - ) - throw new Error('Skill needs a non-empty description.') - candidate.description = frontmatter.description - stringList(frontmatter.sources ?? [], 'sources') - stringList(frontmatter.requires ?? [], 'requires') - const mapping = mapped.find( - (value: unknown) => - typeof value === 'object' && - value !== null && - 'slug' in value && - value.slug === candidate.name, - ) - if (!candidate.domain && typeof mapping?.domain === 'string') - candidate.domain = mapping.domain - if (entry && (entry.package ?? '') !== candidate.package) - throw new Error('Skill ownership differs from its registration.') - } catch (error) { - candidate.status = 'invalid' - candidate.problems.push( - error instanceof Error ? error.message : String(error), - ) - } - return [candidate] - }) - for (const candidate of skills) { - if ( - skills.some( - (other) => other.id !== candidate.id && other.name === candidate.name, - ) - ) { - candidate.status = 'conflict' - candidate.problems.push( - 'Another skill has the same name. Resolve the identity before adoption.', - ) - } - } - return { - schemaVersion: 1 as const, - root: project.root, - artifacts: project.artifacts, - directory, - fingerprint: createHash('sha256') - .update(JSON.stringify([project, directory, snapshot, skills])) - .digest('hex'), - distribution: (tree ? readDistribution(tree) : undefined) ?? { - mode: 'unconfigured' as const, - repository: inferDistributionRepository(project), - }, - skills, - } -} - -export type AdoptionPlan = ReturnType - -export interface AdoptionPrompts { - choose: (plan: AdoptionPlan) => Promise - confirm: (plan: AdoptionPlan, files: Array) => Promise -} - -export function planAdoptionChanges( - project: MaintainerProject, - input: unknown, -) { - if ( - !isObject(input) || - input.schemaVersion !== 1 || - input.root !== project.root || - input.artifacts !== project.artifacts || - !Array.isArray(input.skills) || - (input.directory !== undefined && typeof input.directory !== 'string') - ) - throw new Error( - 'Use an adoption plan from maintainer adopt --json in this repository.', - ) - const current = createAdoptionPlan(project, input.directory) - if (input.fingerprint !== current.fingerprint) - throw new Error( - 'Skills or planning records changed. Create a new adoption plan before applying.', - ) - if (input.skills.length !== current.skills.length) - throw new Error( - 'Keep every adoption plan entry; change selected and domain only.', - ) - const seen = new Set() - const additions = input.skills.flatMap((value: unknown) => { - if ( - !isObject(value) || - typeof value.id !== 'string' || - typeof value.selected !== 'boolean' || - typeof value.domain !== 'string' || - seen.has(value.id) - ) - throw new Error( - 'Each adoption choice needs a unique id, selected boolean, and domain string.', - ) - seen.add(value.id) - const candidate = current.skills.find((skill) => skill.id === value.id) - if ( - !candidate || - ['name', 'package', 'path', 'status'].some( - (key) => value[key] !== candidate[key as keyof AdoptionSkill], - ) - ) - throw new Error('Adoption identities changed. Create a new plan.') - if (!value.selected) return [] - if (candidate.status !== 'unregistered') - throw new Error(`Cannot adopt ${candidate.id}: ${candidate.status}.`) - if (!value.domain.trim()) - throw new Error(`Choose a domain for ${candidate.name}.`) - return [ - { - name: candidate.name, - options: { - package: candidate.package || undefined, - path: candidate.path, - domain: value.domain.trim(), - }, - }, - ] - }) - const plan = planAddSkills(project, additions, planSetupRecords(project)) - const choice = input.distribution - if ( - !isObject(choice) || - !['repo', 'none', 'unconfigured'].includes(String(choice.mode)) - ) - throw new Error( - 'Choose repository distribution, none, or leave the current choice unchanged.', - ) - if ( - choice.mode === 'unconfigured' && - current.distribution.mode !== 'unconfigured' - ) - throw new Error( - 'Preserve the distribution choice or select an explicit opt-out.', - ) - const distribution = planDistributionChoice( - project, - choice.mode === 'unconfigured' - ? {} - : { - distribution: String(choice.mode), - ...(choice.mode === 'repo' - ? { - repository: - typeof choice.repository === 'string' - ? choice.repository - : undefined, - pluginName: - typeof choice.name === 'string' ? choice.name : undefined, - skill: stringList(choice.skills, 'distribution.skills'), - } - : {}), - }, - plan.changes, - ) - if (distribution) { - const index = plan.changes.findIndex( - (change) => change.path === distribution.path, - ) - if (index < 0) plan.changes.push(distribution) - else plan.changes[index] = distribution - } - return plan -} diff --git a/packages/intent/src/maintainer/adoption-prompts.ts b/packages/intent/src/maintainer/adoption-prompts.ts deleted file mode 100644 index 590992b6..00000000 --- a/packages/intent/src/maintainer/adoption-prompts.ts +++ /dev/null @@ -1,124 +0,0 @@ -import { stdin, stdout } from 'node:process' -import { stripVTControlCharacters } from 'node:util' -import { - autocompleteMultiselect, - confirm, - isCancel, - select, - text, -} from '@clack/prompts' -import type { AdoptionPrompts } from './adopt.js' - -export function createAdoptionPrompts(): AdoptionPrompts { - const io = { input: stdin, output: stdout } - return { - async choose(plan) { - const available = plan.skills.filter( - (skill) => skill.status === 'unregistered', - ) - if (available.length) { - const selected = await autocompleteMultiselect({ - ...io, - message: 'Choose existing library skills to register', - options: available.map((skill) => ({ - value: skill.id, - label: skill.name, - hint: stripVTControlCharacters(skill.id), - })), - initialValues: [], - required: false, - maxItems: 6, - }) - if (isCancel(selected)) return null - for (const skill of available) { - skill.selected = selected.includes(skill.id) - if (!skill.selected || skill.domain.trim()) continue - const domain = await text({ - ...io, - message: `Domain for ${skill.name}`, - validate: (value) => - value?.trim() ? undefined : 'Enter the task domain.', - }) - if (isCancel(domain)) return null - skill.domain = domain.trim() - } - } - const mode = await select({ - ...io, - message: 'Repository distribution', - initialValue: 'keep', - options: [ - { - value: 'keep', - label: - plan.distribution.mode === 'unconfigured' - ? 'Decide later' - : `Keep current choice (${plan.distribution.mode})`, - }, - { value: 'repo', label: 'Choose skills for repository distribution' }, - { value: 'none', label: 'Package-only distribution' }, - ], - }) - if (isCancel(mode)) return null - if (mode === 'none') plan.distribution = { mode: 'none' } - if (mode === 'repo') { - const candidates = plan.skills.filter( - (skill) => skill.status === 'registered' || skill.selected, - ) - if (!candidates.length) - throw new Error( - 'Select skills to register before configuring exports.', - ) - const selected = await autocompleteMultiselect({ - ...io, - message: 'Select repository exports and their local prerequisites', - options: candidates.map((skill) => ({ - value: skill.name, - label: skill.name, - hint: stripVTControlCharacters(skill.id), - })), - initialValues: - 'skills' in plan.distribution ? plan.distribution.skills : [], - required: true, - maxItems: 6, - }) - if (isCancel(selected)) return null - let repository = plan.distribution.repository ?? '' - if (!repository) { - const answer = await text({ - ...io, - message: 'GitHub repository (owner/repo)', - validate: (value) => - value?.trim() ? undefined : 'Enter the GitHub repository.', - }) - if (isCancel(answer)) return null - repository = answer.trim() - } - plan.distribution = { - ...plan.distribution, - mode: 'repo', - repository, - skills: selected, - } - } - return plan - }, - async confirm(plan, files) { - console.log('\nProposed registrations:') - for (const skill of plan.skills.filter((entry) => entry.selected)) - console.log( - stripVTControlCharacters(` ${skill.id} -> ${skill.domain}`), - ) - console.log(`Distribution: ${plan.distribution.mode}`) - console.log('Planning files:') - for (const file of files) - console.log(` ${stripVTControlCharacters(file)}`) - const answer = await confirm({ - ...io, - message: 'Apply these choices and install maintainer guidance?', - initialValue: false, - }) - return !isCancel(answer) && answer - }, - } -} diff --git a/packages/intent/src/maintainer/distribution.ts b/packages/intent/src/maintainer/distribution.ts index 19910a76..7c3cc316 100644 --- a/packages/intent/src/maintainer/distribution.ts +++ b/packages/intent/src/maintainer/distribution.ts @@ -62,9 +62,7 @@ function readJson(path: string): Record { return value } -export function inferDistributionRepository( - project: MaintainerProject, -): string { +function inferDistributionRepository(project: MaintainerProject): string { const manifest = readJson(projectPath(project.root, 'package.json')) const declared = isObject(manifest.repository) ? manifest.repository.url @@ -86,7 +84,7 @@ export function configureDistribution( if (change) writeChanges(project.root, [change]) } -export function planDistributionChoice( +function planDistributionChoice( project: MaintainerProject, options: DistributionOptions, changes: ReadonlyArray = [], diff --git a/packages/intent/src/maintainer/existing.ts b/packages/intent/src/maintainer/existing.ts new file mode 100644 index 00000000..5f3178c6 --- /dev/null +++ b/packages/intent/src/maintainer/existing.ts @@ -0,0 +1,163 @@ +import { execFileSync } from 'node:child_process' +import { existsSync } from 'node:fs' +import { basename, dirname, relative } from 'node:path' +import { resolveProjectContext } from '../core/project-context.js' +import { resolveWorkspacePackages } from '../setup/workspace-patterns.js' +import { parseFrontmatter } from '../shared/utils.js' +import { stringList } from './add.js' +import { + isObject, + projectPath, + readRecord, + recordPath, + skillEntries, + skillPath, +} from './project.js' +import type { MaintainerProject } from './project.js' + +export interface ExistingSkill { + id: string + name: string + package: string + path: string + domain: string + problems: Array +} + +const defaultDomain = 'uncategorized' + +// Git-visible SKILL.md files under a skills/ directory of a workspace package +// that skill_tree.yaml does not register yet. Agent skill directories, +// dependencies, and packages outside the workspace are not library skills. +export function findExistingSkills( + project: MaintainerProject, +): Array { + const files = execFileSync( + 'git', + [ + '-c', + 'core.fsmonitor=false', + 'ls-files', + '--cached', + '--others', + '--exclude-standard', + '-z', + ], + { cwd: project.root, encoding: 'utf8', maxBuffer: 32 * 1024 * 1024 }, + ) + .split('\0') + .filter(Boolean) + const tree = readRecord(project, 'skill_tree.yaml') + const entries = skillEntries(project, tree) + const registered = new Set( + entries.map((entry) => + relative(project.root, skillPath(project, entry)).replaceAll('\\', '/'), + ), + ) + const registeredNames = new Set( + entries.map((entry) => String(entry.slug ?? entry.name)), + ) + const mapPath = recordPath(project, 'domain_map.yaml') + const mapped: Array = existsSync(mapPath) + ? readRecord(project, 'domain_map.yaml').document.toJS().skills + : [] + const packageRoots = new Set([ + project.root, + ...resolveWorkspacePackages( + project.root, + resolveProjectContext({ cwd: project.root }).workspacePatterns, + ), + ]) + const skills = files + .filter( + (path) => + basename(path) === 'SKILL.md' && + /(^|\/)skills\//.test(path) && + !path + .split('/') + .some((part) => part.startsWith('.') || part === 'node_modules') && + !registered.has(path), + ) + .sort() + .flatMap((id) => { + const skill: ExistingSkill = { + id, + name: basename(dirname(id)), + package: '', + path: id, + domain: '', + problems: [], + } + try { + const absolute = projectPath(project.root, id) + const context = resolveProjectContext({ + cwd: project.root, + targetPath: absolute, + }) + if (!context.packageRoot || !packageRoots.has(context.packageRoot)) + return [] + skill.package = relative(project.root, context.packageRoot).replaceAll( + '\\', + '/', + ) + skill.path = relative(context.packageRoot, absolute).replaceAll( + '\\', + '/', + ) + const frontmatter = parseFrontmatter(absolute) + if ( + !frontmatter || + typeof frontmatter.name !== 'string' || + !/^[a-z0-9]+(?:-[a-z0-9]+)*$/.test(frontmatter.name) || + frontmatter.name.length > 64 || + frontmatter.name !== skill.name + ) + throw new Error('Skill name must match its directory.') + if ( + typeof frontmatter.description !== 'string' || + !frontmatter.description.trim() + ) + throw new Error('Skill needs a non-empty description.') + stringList(frontmatter.sources ?? [], 'sources') + stringList(frontmatter.requires ?? [], 'requires') + skill.domain = inferDomain(frontmatter, mapped, id) + } catch (error) { + skill.problems.push( + error instanceof Error ? error.message : String(error), + ) + } + return [skill] + }) + for (const skill of skills) { + if ( + registeredNames.has(skill.name) || + skills.some((other) => other.id !== skill.id && other.name === skill.name) + ) + skill.problems.push( + 'Another skill has the same name. Rename one before it can be registered.', + ) + } + return skills +} + +// metadata.domain, then the domain map, then a parent directory between +// skills/ and the skill directory, then a placeholder the maintainer can edit. +function inferDomain( + frontmatter: Record, + mapped: Array, + id: string, +): string { + const metadata = frontmatter.metadata + if ( + isObject(metadata) && + typeof metadata.domain === 'string' && + metadata.domain.trim() + ) + return metadata.domain.trim() + const name = basename(dirname(id)) + const mapping = mapped.find((value) => isObject(value) && value.slug === name) + if (isObject(mapping) && typeof mapping.domain === 'string') + return mapping.domain + const parent = basename(dirname(dirname(id))) + return parent === 'skills' ? defaultDomain : parent +} diff --git a/packages/intent/src/maintainer/project.ts b/packages/intent/src/maintainer/project.ts index dda29cd3..fc01fd02 100644 --- a/packages/intent/src/maintainer/project.ts +++ b/packages/intent/src/maintainer/project.ts @@ -173,9 +173,7 @@ export function setupRecords(project: MaintainerProject): Array { ) } -export function planSetupRecords( - project: MaintainerProject, -): Array { +function planSetupRecords(project: MaintainerProject): Array { const { root } = project const context = resolveProjectContext({ cwd: root }) if (!context.packageRoot) diff --git a/packages/intent/tests/integration/packed-release.test.ts b/packages/intent/tests/integration/packed-release.test.ts index 0441ba2a..b9edf242 100644 --- a/packages/intent/tests/integration/packed-release.test.ts +++ b/packages/intent/tests/integration/packed-release.test.ts @@ -193,16 +193,9 @@ describe('packed release', () => { } expect(run(['scaffold']).status).toBe(1) const overview = run(['maintainer', '--help']).stdout - for (const action of [ - 'setup', - 'adopt', - 'add', - 'status', - 'sync', - 'review', - 'check', - ]) + for (const action of ['setup', 'add', 'status', 'sync', 'review', 'check']) expect(overview).toContain(`\n${action}: maintainer ${action}`) + expect(overview).not.toContain('maintainer adopt') }) it('keeps nested authoring references usable within the extracted package', () => { diff --git a/packages/intent/tests/maintainer.test.ts b/packages/intent/tests/maintainer.test.ts index 97bef13c..3f0b3e30 100644 --- a/packages/intent/tests/maintainer.test.ts +++ b/packages/intent/tests/maintainer.test.ts @@ -14,7 +14,6 @@ import { afterEach, beforeEach, expect, it, vi } from 'vitest' import { parse } from 'yaml' import { main } from '../src/cli.js' import { createReview } from '../src/review/review.js' -import type { AdoptionPlan } from '../src/maintainer/adopt.js' // These tests run maintainer commands against real Git repositories. vi.setConfig({ testTimeout: 30_000 }) @@ -246,15 +245,19 @@ it('preserves a planning record located directly at the repository root', async ) }) -it('previews existing library skills without writing or including agent and dependency skills', async () => { +it('registers existing library skills during setup and skips agent, dependency, and fixture skills', async () => { write('pnpm-workspace.yaml', 'packages: [packages/*]\n') write('packages/client/package.json', '{"name":"@library/client"}\n') + write('tests/fixtures/client/package.json', '{"name":"fixture"}\n') + const guidance = + '---\nname: query\ndescription: Use when querying.\nmetadata:\n purpose: Preserve this purpose.\nsources: [package.json]\n---\nAuthored guidance.\n' + write('packages/client/skills/query/SKILL.md', guidance) for (const directory of [ 'skills/root-task', - 'packages/client/skills/query', '.agents/skills/agent-only', '.github/skills/review', 'node_modules/dependency/skills/dependency', + 'tests/fixtures/client/skills/fixture-task', ]) { const name = directory.split('/').at(-1) write( @@ -262,277 +265,149 @@ it('previews existing library skills without writing or including agent and depe `---\nname: ${name}\ndescription: Use for ${name}.\nsources: [package.json]\n---\nExisting guidance.\n`, ) } - expect(await main(['maintainer', 'adopt', '--json'])).toBe(0) - const plan = JSON.parse(String(vi.mocked(console.log).mock.calls[0]![0])) - expect(plan.skills).toEqual([ + expect(await main(['maintainer', 'setup'])).toBe(0) + const tree = parse(read('_artifacts/skill_tree.yaml')) + expect(tree.skills).toEqual([ expect.objectContaining({ name: 'query', package: 'packages/client', path: 'skills/query/SKILL.md', - status: 'unregistered', - selected: false, + domain: 'uncategorized', + purpose: 'Preserve this purpose.', }), expect.objectContaining({ name: 'root-task', - package: '', path: 'skills/root-task/SKILL.md', - status: 'unregistered', - selected: false, + domain: 'uncategorized', }), ]) - expect(existsSync(join(root, '_artifacts'))).toBe(false) - expect(existsSync(join(root, '.intent'))).toBe(false) - expect(existsSync(join(root, 'AGENTS.md'))).toBe(false) -}) - -it('excludes nested fixture packages unless their directory is explicitly requested', async () => { - write('pnpm-workspace.yaml', 'packages: [packages/*]\n') - for (const directory of ['packages/client', 'tests/fixtures/client']) { - write(`${directory}/package.json`, '{"name":"client"}\n') - write( - `${directory}/skills/query/SKILL.md`, - '---\nname: query\ndescription: Query\n---\nGuidance.\n', - ) - } - expect(await main(['maintainer', 'adopt', '--json'])).toBe(0) - const plan = JSON.parse(String(vi.mocked(console.log).mock.calls[0]![0])) - expect(plan.skills.map((skill: { id: string }) => skill.id)).toEqual([ - 'packages/client/skills/query/SKILL.md', - ]) - vi.mocked(console.log).mockClear() - expect( - await main([ - 'maintainer', - 'adopt', - '--path', - 'tests/fixtures/client/skills', - '--json', - ]), - ).toBe(0) - const explicit = JSON.parse(String(vi.mocked(console.log).mock.calls[0]![0])) - expect(explicit.skills).toHaveLength(2) -}) - -it('adopts a confirmed batch without rewriting skills or approving their content', async () => { - const guidance = - '---\nname: query\ndescription: Use when querying.\nmetadata:\n purpose: Preserve this purpose.\nsources: [package.json]\n---\nAuthored guidance.\n' - write('skills/query/SKILL.md', guidance) - expect(await main(['maintainer', 'adopt', '--json'])).toBe(0) - const plan = JSON.parse(String(vi.mocked(console.log).mock.calls[0]![0])) - plan.skills[0].selected = true - plan.skills[0].domain = 'queries' - plan.distribution = { mode: 'none' } - write('adoption.json', JSON.stringify(plan)) - expect(await main(['maintainer', 'adopt', '--apply', 'adoption.json'])).toBe( - 0, - ) - expect(read('skills/query/SKILL.md')).toBe(guidance) - expect( - parse(read('skills/_artifacts/skill_tree.yaml')).skills[0], - ).toMatchObject({ - name: 'query', - domain: 'queries', - purpose: 'Preserve this purpose.', - }) - expect( - parse(read('skills/_artifacts/domain_map.yaml')).skills[0].tasks, - ).toEqual([]) - expect(read('skills/_artifacts/skill_spec.md')).toContain( - 'intent:needs-authoring', + expect(read('packages/client/skills/query/SKILL.md')).toBe(guidance) + expect(parse(read('_artifacts/domain_map.yaml')).skills[0].tasks).toEqual([]) + expect(vi.mocked(console.log).mock.calls.flat().join('\n')).toContain( + 'Set a domain for uncategorized skills', ) - expect(read('AGENTS.md')).toContain('maintainer check') - expect(existsSync(join(root, '.intent/review-state.json'))).toBe(false) - expect(await main(['maintainer', 'check'])).toBe(1) const before = ['skill_tree.yaml', 'domain_map.yaml', 'skill_spec.md'].map( - (name) => read(`skills/_artifacts/${name}`), - ) - vi.mocked(console.log).mockClear() - expect(await main(['maintainer', 'adopt', '--json'])).toBe(0) - const repeated = JSON.parse(String(vi.mocked(console.log).mock.calls[0]![0])) - expect(repeated.skills[0].status).toBe('registered') - write('adoption.json', JSON.stringify(repeated)) - expect(await main(['maintainer', 'adopt', '--apply', 'adoption.json'])).toBe( - 0, + (name) => read(`_artifacts/${name}`), ) + expect(await main(['maintainer', 'setup'])).toBe(0) expect( ['skill_tree.yaml', 'domain_map.yaml', 'skill_spec.md'].map((name) => - read(`skills/_artifacts/${name}`), + read(`_artifacts/${name}`), ), ).toEqual(before) + expect(existsSync(join(root, '.intent/review-state.json'))).toBe(false) }) -it('rejects invalid batch choices and stale adoption plans before creating records', async () => { - for (const name of ['query', 'cache']) - write( - `skills/${name}/SKILL.md`, - `---\nname: ${name}\ndescription: ${name}\n---\nGuidance.\n`, - ) - expect(await main(['maintainer', 'adopt', '--json'])).toBe(0) - const plan = JSON.parse(String(vi.mocked(console.log).mock.calls[0]![0])) - for (const candidate of plan.skills) candidate.selected = true - plan.skills[0].domain = 'queries' - write('adoption.json', JSON.stringify(plan)) - expect(await main(['maintainer', 'adopt', '--apply', 'adoption.json'])).toBe( - 1, - ) - expect(existsSync(join(root, 'skills/_artifacts'))).toBe(false) - plan.skills[1].domain = 'queries' - write('adoption.json', JSON.stringify(plan)) +it('infers the domain from frontmatter, the domain map, or the parent directory', async () => { write( 'skills/query/SKILL.md', - read('skills/query/SKILL.md') + 'Later change.\n', + '---\nname: query\ndescription: Query\nmetadata:\n domain: reads\n---\nGuidance.\n', + ) + write( + 'skills/writes/mutate/SKILL.md', + '---\nname: mutate\ndescription: Mutate\n---\nGuidance.\n', ) - expect(await main(['maintainer', 'adopt', '--apply', 'adoption.json'])).toBe( - 1, + write( + 'skills/cache/SKILL.md', + '---\nname: cache\ndescription: Cache\n---\nGuidance.\n', ) - expect(vi.mocked(console.error).mock.calls.flat().join('\n')).toContain( - 'changed', + write( + 'skills/_artifacts/domain_map.yaml', + 'skills:\n - name: cache\n slug: cache\n domain: storage\n', ) - expect(existsSync(join(root, 'skills/_artifacts'))).toBe(false) - expect(existsSync(join(root, 'AGENTS.md'))).toBe(false) + expect(await main(['maintainer', 'setup'])).toBe(0) + expect( + Object.fromEntries( + parse(read('skills/_artifacts/skill_tree.yaml')).skills.map( + (skill: { name: string; domain: string }) => [skill.name, skill.domain], + ), + ), + ).toEqual({ cache: 'storage', mutate: 'writes', query: 'reads' }) }) -it('requires interactive confirmation and keeps cancellation read-only', async () => { +it('reports invalid and conflicting existing skills without registering them', async () => { + write('pnpm-workspace.yaml', 'packages: [packages/*]\n') + write('packages/client/package.json', '{"name":"@library/client"}\n') write( 'skills/query/SKILL.md', '---\nname: query\ndescription: Query\n---\nGuidance.\n', ) - const choose = vi.fn((plan: AdoptionPlan) => { - plan.skills[0]!.selected = true - plan.skills[0]!.domain = 'queries' - plan.distribution = { mode: 'none' } - return Promise.resolve(plan) - }) - const confirm = vi.fn(() => Promise.resolve(false)) - const runtime = { - isTTY: true, - isCI: false, - adoptionPrompts: { choose, confirm }, - } - expect(await main(['maintainer', 'adopt'], runtime)).toBe(0) - expect(confirm).toHaveBeenCalledOnce() - expect(existsSync(join(root, 'skills/_artifacts'))).toBe(false) - expect(existsSync(join(root, 'AGENTS.md'))).toBe(false) - confirm.mockResolvedValue(true) - expect(await main(['maintainer', 'adopt'], runtime)).toBe(0) - expect(parse(read('skills/_artifacts/skill_tree.yaml')).skills[0].name).toBe( - 'query', + write( + 'packages/client/skills/query/SKILL.md', + '---\nname: query\ndescription: Query again\n---\nGuidance.\n', ) -}) - -it('requires explicit noninteractive adoption and reports custom-root conflicts', async () => { + write('skills/broken/SKILL.md', '---\nname: other\n---\nGuidance.\n') write( - 'guidance/query/SKILL.md', - '---\nname: query\ndescription: Query\n---\nGuidance.\n', + 'skills/blank/SKILL.md', + '---\nname: blank\ndescription: Blank domain\nmetadata:\n domain: " "\n---\nGuidance.\n', ) - expect(await main(['maintainer', 'adopt'], { isTTY: false })).toBe(1) - expect(existsSync(join(root, '.intent'))).toBe(false) - vi.mocked(console.log).mockClear() - expect( - await main(['maintainer', 'adopt', '--path', 'guidance', '--json']), - ).toBe(0) - const plan = JSON.parse(String(vi.mocked(console.log).mock.calls[0]![0])) - expect(plan.skills[0].id).toBe('guidance/query/SKILL.md') - write('skills/query/SKILL.md', read('guidance/query/SKILL.md')) - vi.mocked(console.log).mockClear() - expect( - await main(['maintainer', 'adopt', '--path', 'guidance', '--json']), - ).toBe(0) - const conflicts = JSON.parse(String(vi.mocked(console.log).mock.calls[0]![0])) - expect( - conflicts.skills.map((skill: { status: string }) => skill.status), - ).toEqual(['conflict', 'conflict']) -}) - -it('never prompts in CI even when a terminal is attached', async () => { - const choose = vi.fn(() => Promise.resolve(null)) - const confirm = vi.fn(() => Promise.resolve(false)) - const runtime = { - isTTY: true, - isCI: true, - adoptionPrompts: { choose, confirm }, - } - expect(await main(['maintainer', 'adopt'], runtime)).toBe(1) - expect(await main(['maintainer', 'adopt', '--json'], runtime)).toBe(0) - expect(choose).not.toHaveBeenCalled() - expect(confirm).not.toHaveBeenCalled() - expect(existsSync(join(root, '.intent'))).toBe(false) - expect(existsSync(join(root, 'skills/_artifacts'))).toBe(false) -}) - -it('keeps planned and retired entries separate from missing active skills', async () => { - expect(await main(['maintainer', 'setup'])).toBe(0) write( - 'skills/_artifacts/skill_tree.yaml', - 'skills:\n - name: future\n path: skills/future/SKILL.md\n status: planned\n - name: old\n path: skills/old/SKILL.md\n status: retired\n - name: missing\n path: skills/missing/SKILL.md\n', + '_artifacts/skill_tree.yaml', + 'skills:\n - name: taken\n slug: taken\n path: elsewhere/taken/SKILL.md\n domain: d\n', ) - vi.mocked(console.log).mockClear() - expect(await main(['maintainer', 'adopt', '--json'])).toBe(0) - const plan = JSON.parse(String(vi.mocked(console.log).mock.calls[0]![0])) - expect(plan.skills.map((skill: { status: string }) => skill.status)).toEqual([ - 'planned', - 'missing', - 'retired', - ]) -}) - -it('rejects adoption after repository instructions change', async () => { write( - 'skills/query/SKILL.md', - '---\nname: query\ndescription: Query\n---\nGuidance.\n', + 'skills/taken/SKILL.md', + '---\nname: taken\ndescription: Taken\n---\nGuidance.\n', + ) + expect(await main(['maintainer', 'setup'])).toBe(0) + expect(parse(read('_artifacts/skill_tree.yaml')).skills).toEqual([ + expect.objectContaining({ + name: 'taken', + path: 'elsewhere/taken/SKILL.md', + }), + expect.objectContaining({ name: 'blank', domain: 'uncategorized' }), + ]) + const output = vi.mocked(console.log).mock.calls.flat().join('\n') + expect(output).toContain( + 'Skipped skills/taken/SKILL.md: Another skill has the same name', + ) + expect(output).toContain( + 'Skipped skills/broken/SKILL.md: Skill name must match', + ) + expect(output).toContain( + 'Skipped skills/query/SKILL.md: Another skill has the same name', + ) + expect(output).toContain( + 'Skipped packages/client/skills/query/SKILL.md: Another skill has the same name', ) - expect(await main(['maintainer', 'adopt', '--json'])).toBe(0) - const plan = JSON.parse(String(vi.mocked(console.log).mock.calls[0]![0])) - plan.skills[0].selected = true - plan.skills[0].domain = 'queries' - write('adoption.json', JSON.stringify(plan)) - write('AGENTS.md', 'New maintainer instructions.\n') - expect(await main(['maintainer', 'adopt', '--apply', 'adoption.json'])).toBe( - 1, - ) - expect(read('AGENTS.md')).toBe('New maintainer instructions.\n') - expect(existsSync(join(root, 'skills/_artifacts'))).toBe(false) }) -it('adopts two packages and saves an explicit distribution selection', async () => { +it('keeps valid registrations when the planner rejects a candidate in the batch', async () => { + const contents = new Map( + ['alpha', 'broken', 'omega'].map((name) => [ + name, + `---\nname: ${name}\ndescription: Use ${name}.\n---\nAuthored guidance.\n`, + ]), + ) + for (const [name, content] of contents) + write(`skills/${name}/SKILL.md`, content) write( - 'package.json', - '{"name":"library","repository":"https://github.com/acme/library"}\n', + 'skills/_artifacts/domain_map.yaml', + 'skills: [{ slug: broken, domain: "" }]\n', ) - write('pnpm-workspace.yaml', 'packages: [packages/*]\n') - for (const name of ['query', 'cache']) { - write(`packages/${name}/package.json`, `{"name":"@library/${name}"}\n`) - write( - `packages/${name}/skills/${name}/SKILL.md`, - `---\nname: ${name}\ndescription: Use for ${name}.\nsources: [package.json]\n---\nAuthored ${name} guidance.\n`, - ) - } - expect(await main(['maintainer', 'adopt', '--json'])).toBe(0) - const plan = JSON.parse(String(vi.mocked(console.log).mock.calls[0]![0])) - for (const candidate of plan.skills) { - candidate.selected = true - candidate.domain = 'queries' - } - plan.distribution = { - mode: 'repo', - repository: 'acme/library', - skills: ['query'], - } - write('adoption.json', JSON.stringify(plan)) - expect(await main(['maintainer', 'adopt', '--apply', 'adoption.json'])).toBe( - 0, + expect(await main(['maintainer', 'setup'])).toBe(0) + expect( + parse(read('skills/_artifacts/skill_tree.yaml')).skills.map( + (entry: { name: string }) => entry.name, + ), + ).toEqual(['alpha', 'omega']) + expect(read('skills/_artifacts/skill_spec.md')).not.toContain( + 'Registered `broken`', ) - const tree = parse(read('_artifacts/skill_tree.yaml')) + expect(vi.mocked(console.log).mock.calls.flat().join('\n')).toContain( + 'Skipped skills/broken/SKILL.md: Set a non-empty domain for broken in skills/_artifacts/domain_map.yaml, then run intent maintainer setup again.', + ) + for (const [name, content] of contents) + expect(read(`skills/${name}/SKILL.md`)).toBe(content) + const records = ['skill_tree.yaml', 'domain_map.yaml', 'skill_spec.md'].map( + (file) => read(`skills/_artifacts/${file}`), + ) + expect(await main(['maintainer', 'setup'])).toBe(0) expect( - tree.skills.map((skill: { package: string }) => skill.package), - ).toEqual(['packages/cache', 'packages/query']) - expect(tree.distribution).toEqual({ - mode: 'repo', - repository: 'acme/library', - name: 'acme-library', - skills: ['query'], - }) - expect(existsSync(join(root, '.claude-plugin'))).toBe(false) + ['skill_tree.yaml', 'domain_map.yaml', 'skill_spec.md'].map((file) => + read(`skills/_artifacts/${file}`), + ), + ).toEqual(records) }) afterEach(() => {