From 916b65a9ba8247a31060a6505af0f0a7b7158601 Mon Sep 17 00:00:00 2001 From: ladybluenotes Date: Fri, 11 Sep 2026 22:33:45 -0700 Subject: [PATCH 1/8] feat: register existing skills during maintainer setup Replace `maintainer adopt` with a scan inside `maintainer setup`. Any Git-visible SKILL.md under a skills/ directory of a workspace package that the tree does not record yet is registered with its content 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. Remove the adoption plan, its JSON apply format, the interactive picker, and the second place the distribution question was asked. --- .changeset/guided-maintainer-adoption.md | 2 +- packages/intent/src/cli.ts | 16 +- packages/intent/src/commands/maintainer.ts | 121 ++---- packages/intent/src/maintainer/adopt.ts | 357 ------------------ .../intent/src/maintainer/adoption-prompts.ts | 124 ------ packages/intent/src/maintainer/existing.ts | 154 ++++++++ packages/intent/tests/maintainer.test.ts | 290 +++----------- 7 files changed, 243 insertions(+), 821 deletions(-) delete mode 100644 packages/intent/src/maintainer/adopt.ts delete mode 100644 packages/intent/src/maintainer/adoption-prompts.ts create mode 100644 packages/intent/src/maintainer/existing.ts diff --git a/.changeset/guided-maintainer-adoption.md b/.changeset/guided-maintainer-adoption.md index 76eb881e..984fecd3 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` 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/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..40443838 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,26 @@ 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 registered = planAddSkills( + project, + existing + .filter((skill) => skill.problems.length === 0) + .map((skill) => ({ + name: skill.name, + options: { + package: skill.package || undefined, + path: skill.path, + domain: skill.domain, + }, + })), + ) + writeChanges(project.root, registered.changes) runSetupGithubActions(project.root, getMetaDir()) writeIntentSkillsBlock({ ...buildMaintainerGuidanceBlock( @@ -370,9 +306,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/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/existing.ts b/packages/intent/src/maintainer/existing.ts new file mode 100644 index 00000000..970dbc2c --- /dev/null +++ b/packages/intent/src/maintainer/existing.ts @@ -0,0 +1,154 @@ +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 registered = new Set( + skillEntries(project, tree).map((entry) => + relative(project.root, skillPath(project, entry)).replaceAll('\\', '/'), + ), + ) + 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 ( + 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') + return metadata.domain + 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/tests/maintainer.test.ts b/packages/intent/tests/maintainer.test.ts index 97bef13c..1d449253 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,90 @@ 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', - ) - expect(await main(['maintainer', 'adopt', '--apply', 'adoption.json'])).toBe( - 1, + '---\nname: query\ndescription: Query\nmetadata:\n domain: reads\n---\nGuidance.\n', ) - expect(vi.mocked(console.error).mock.calls.flat().join('\n')).toContain( - 'changed', - ) - expect(existsSync(join(root, 'skills/_artifacts'))).toBe(false) - expect(existsSync(join(root, 'AGENTS.md'))).toBe(false) -}) - -it('requires interactive confirmation and keeps cancellation read-only', async () => { write( - 'skills/query/SKILL.md', - '---\nname: query\ndescription: Query\n---\nGuidance.\n', + 'skills/writes/mutate/SKILL.md', + '---\nname: mutate\ndescription: Mutate\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', - ) -}) - -it('requires explicit noninteractive adoption and reports custom-root conflicts', async () => { write( - 'guidance/query/SKILL.md', - '---\nname: query\ndescription: Query\n---\nGuidance.\n', + 'skills/cache/SKILL.md', + '---\nname: cache\ndescription: Cache\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', + 'skills/_artifacts/domain_map.yaml', + 'skills:\n - name: cache\n slug: cache\n domain: storage\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', - ]) + 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('rejects adoption after repository instructions change', 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', ) - 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 () => { write( - 'package.json', - '{"name":"library","repository":"https://github.com/acme/library"}\n', + 'packages/client/skills/query/SKILL.md', + '---\nname: query\ndescription: Query again\n---\nGuidance.\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, + write('skills/broken/SKILL.md', '---\nname: other\n---\nGuidance.\n') + expect(await main(['maintainer', 'setup'])).toBe(0) + expect(parse(read('_artifacts/skill_tree.yaml')).skills).toEqual([]) + const output = vi.mocked(console.log).mock.calls.flat().join('\n') + 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', ) - const tree = parse(read('_artifacts/skill_tree.yaml')) - 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) }) afterEach(() => { From 46247118dfd55068f7805a1b79392459fafd60f7 Mon Sep 17 00:00:00 2001 From: ladybluenotes Date: Sat, 12 Sep 2026 09:18:41 -0700 Subject: [PATCH 2/8] fix: keep setup registering valid skills when one candidate is rejected Register existing skills one at a time so a planner rejection is reported as skipped instead of aborting the batch. Treat a name that is already registered at another path, and a blank metadata.domain, as problems up front. Drop the packed-release expectation for the removed adopt action and state the replacement in the changeset. --- .changeset/guided-maintainer-adoption.md | 2 +- packages/intent/src/commands/maintainer.ts | 42 +++++++++++++------ packages/intent/src/maintainer/existing.ts | 15 +++++-- .../tests/integration/packed-release.test.ts | 10 +---- packages/intent/tests/maintainer.test.ts | 23 +++++++++- 5 files changed, 65 insertions(+), 27 deletions(-) diff --git a/.changeset/guided-maintainer-adoption.md b/.changeset/guided-maintainer-adoption.md index 984fecd3..6dc26c02 100644 --- a/.changeset/guided-maintainer-adoption.md +++ b/.changeset/guided-maintainer-adoption.md @@ -2,4 +2,4 @@ '@tanstack/intent': minor --- -`maintainer 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. +`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/packages/intent/src/commands/maintainer.ts b/packages/intent/src/commands/maintainer.ts index 40443838..47da0f53 100644 --- a/packages/intent/src/commands/maintainer.ts +++ b/packages/intent/src/commands/maintainer.ts @@ -277,19 +277,35 @@ export async function runMaintainerCommand( const created = setupRecords(project) configureDistribution(project, options) const existing = findExistingSkills(project) - const registered = planAddSkills( - project, - existing - .filter((skill) => skill.problems.length === 0) - .map((skill) => ({ - name: skill.name, - options: { - package: skill.package || undefined, - path: skill.path, - domain: skill.domain, - }, - })), - ) + // Register one skill at a time so a candidate the planner rejects is + // reported as skipped instead of aborting the others. + let registered: ReturnType = { + changes: [], + paths: [], + } + for (const skill of existing) { + if (skill.problems.length) continue + try { + registered = planAddSkills( + project, + [ + { + name: skill.name, + options: { + package: skill.package || undefined, + path: skill.path, + domain: skill.domain, + }, + }, + ], + registered.changes, + ) + } catch (error) { + skill.problems.push( + error instanceof Error ? error.message : String(error), + ) + } + } writeChanges(project.root, registered.changes) runSetupGithubActions(project.root, getMetaDir()) writeIntentSkillsBlock({ diff --git a/packages/intent/src/maintainer/existing.ts b/packages/intent/src/maintainer/existing.ts index 970dbc2c..5f3178c6 100644 --- a/packages/intent/src/maintainer/existing.ts +++ b/packages/intent/src/maintainer/existing.ts @@ -48,11 +48,15 @@ export function findExistingSkills( .split('\0') .filter(Boolean) const tree = readRecord(project, 'skill_tree.yaml') + const entries = skillEntries(project, tree) const registered = new Set( - skillEntries(project, tree).map((entry) => + 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 @@ -126,6 +130,7 @@ export function findExistingSkills( }) for (const skill of skills) { if ( + registeredNames.has(skill.name) || skills.some((other) => other.id !== skill.id && other.name === skill.name) ) skill.problems.push( @@ -143,8 +148,12 @@ function inferDomain( id: string, ): string { const metadata = frontmatter.metadata - if (isObject(metadata) && typeof metadata.domain === 'string') - return metadata.domain + 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') diff --git a/packages/intent/tests/integration/packed-release.test.ts b/packages/intent/tests/integration/packed-release.test.ts index 0441ba2a..620383d8 100644 --- a/packages/intent/tests/integration/packed-release.test.ts +++ b/packages/intent/tests/integration/packed-release.test.ts @@ -193,15 +193,7 @@ 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}`) }) diff --git a/packages/intent/tests/maintainer.test.ts b/packages/intent/tests/maintainer.test.ts index 1d449253..458a6558 100644 --- a/packages/intent/tests/maintainer.test.ts +++ b/packages/intent/tests/maintainer.test.ts @@ -337,9 +337,30 @@ it('reports invalid and conflicting existing skills without registering them', a '---\nname: query\ndescription: Query again\n---\nGuidance.\n', ) write('skills/broken/SKILL.md', '---\nname: other\n---\nGuidance.\n') + write( + 'skills/blank/SKILL.md', + '---\nname: blank\ndescription: Blank domain\nmetadata:\n domain: " "\n---\nGuidance.\n', + ) + write( + '_artifacts/skill_tree.yaml', + 'skills:\n - name: taken\n slug: taken\n path: elsewhere/taken/SKILL.md\n domain: d\n', + ) + write( + '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(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', ) From b0d87895cd7dcc3fdb028bc9e2f60ae81e09bb42 Mon Sep 17 00:00:00 2001 From: ladybluenotes Date: Sat, 12 Sep 2026 09:26:30 -0700 Subject: [PATCH 3/8] chore: keep distribution and record planners module-private for knip --- packages/intent/src/maintainer/distribution.ts | 4 ++-- packages/intent/src/maintainer/project.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/intent/src/maintainer/distribution.ts b/packages/intent/src/maintainer/distribution.ts index 19910a76..1ed70ed7 100644 --- a/packages/intent/src/maintainer/distribution.ts +++ b/packages/intent/src/maintainer/distribution.ts @@ -62,7 +62,7 @@ function readJson(path: string): Record { return value } -export function inferDistributionRepository( +function inferDistributionRepository( project: MaintainerProject, ): string { const manifest = readJson(projectPath(project.root, 'package.json')) @@ -86,7 +86,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/project.ts b/packages/intent/src/maintainer/project.ts index dda29cd3..ebcd78ad 100644 --- a/packages/intent/src/maintainer/project.ts +++ b/packages/intent/src/maintainer/project.ts @@ -173,7 +173,7 @@ export function setupRecords(project: MaintainerProject): Array { ) } -export function planSetupRecords( +function planSetupRecords( project: MaintainerProject, ): Array { const { root } = project From fb739c3268bcbc401bf177071a016e6f75da7407 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sat, 12 Sep 2026 16:27:05 +0000 Subject: [PATCH 4/8] ci: apply automated fixes --- packages/intent/src/maintainer/distribution.ts | 4 +--- packages/intent/src/maintainer/project.ts | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/intent/src/maintainer/distribution.ts b/packages/intent/src/maintainer/distribution.ts index 1ed70ed7..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 } -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 diff --git a/packages/intent/src/maintainer/project.ts b/packages/intent/src/maintainer/project.ts index ebcd78ad..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 { ) } -function planSetupRecords( - project: MaintainerProject, -): Array { +function planSetupRecords(project: MaintainerProject): Array { const { root } = project const context = resolveProjectContext({ cwd: root }) if (!context.packageRoot) From 214505a401f46508f192f1e7b50dd2f7ec587a72 Mon Sep 17 00:00:00 2001 From: Sarah Gerrard Date: Sat, 12 Sep 2026 09:58:41 -0700 Subject: [PATCH 5/8] test: assert the packed maintainer help no longer lists adopt --- packages/intent/tests/integration/packed-release.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/intent/tests/integration/packed-release.test.ts b/packages/intent/tests/integration/packed-release.test.ts index 620383d8..b9edf242 100644 --- a/packages/intent/tests/integration/packed-release.test.ts +++ b/packages/intent/tests/integration/packed-release.test.ts @@ -195,6 +195,7 @@ describe('packed release', () => { const overview = run(['maintainer', '--help']).stdout 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', () => { From 4982e1e4d1c327cc4b7dbf372c486037c3920feb Mon Sep 17 00:00:00 2001 From: ladybluenotes Date: Sat, 12 Sep 2026 20:45:11 -0700 Subject: [PATCH 6/8] perf: register existing skills in one validated batch --- benchmarks/intent/maintainer.bench.ts | 65 ++++++ packages/intent/src/commands/maintainer.ts | 43 ++-- packages/intent/src/maintainer/add.ts | 220 ++++++++++++--------- packages/intent/tests/maintainer.test.ts | 38 ++++ 4 files changed, 245 insertions(+), 121 deletions(-) create mode 100644 benchmarks/intent/maintainer.bench.ts diff --git a/benchmarks/intent/maintainer.bench.ts b/benchmarks/intent/maintainer.bench.ts new file mode 100644 index 00000000..2e168821 --- /dev/null +++ b/benchmarks/intent/maintainer.bench.ts @@ -0,0 +1,65 @@ +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 + beforeAll(() => { + 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, + }) + }) + afterAll(() => rmSync(root, { recursive: true, force: true })) + + bench( + 'registers the complete batch', + () => { + // 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 }, + ) + }) +} diff --git a/packages/intent/src/commands/maintainer.ts b/packages/intent/src/commands/maintainer.ts index 47da0f53..8b304a25 100644 --- a/packages/intent/src/commands/maintainer.ts +++ b/packages/intent/src/commands/maintainer.ts @@ -277,35 +277,24 @@ export async function runMaintainerCommand( const created = setupRecords(project) configureDistribution(project, options) const existing = findExistingSkills(project) - // Register one skill at a time so a candidate the planner rejects is - // reported as skipped instead of aborting the others. - let registered: ReturnType = { - changes: [], - paths: [], - } - for (const skill of existing) { - if (skill.problems.length) continue - try { - registered = planAddSkills( - project, - [ - { - name: skill.name, - options: { - package: skill.package || undefined, - path: skill.path, - domain: skill.domain, - }, - }, - ], - registered.changes, - ) - } catch (error) { - skill.problems.push( + 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) => { + candidates[index]!.problems.push( error instanceof Error ? error.message : String(error), ) - } - } + }, + ) writeChanges(project.root, registered.changes) runSetupGithubActions(project.root, getMetaDir()) writeIntentSkillsBlock({ 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/tests/maintainer.test.ts b/packages/intent/tests/maintainer.test.ts index 458a6558..091e245f 100644 --- a/packages/intent/tests/maintainer.test.ts +++ b/packages/intent/tests/maintainer.test.ts @@ -372,6 +372,44 @@ it('reports invalid and conflicting existing skills without registering them', a ) }) +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( + 'skills/_artifacts/domain_map.yaml', + 'skills: [{ slug: broken, domain: "" }]\n', + ) + 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`', + ) + expect(vi.mocked(console.log).mock.calls.flat().join('\n')).toContain( + 'Skipped skills/broken/SKILL.md: Choose the task domain', + ) + 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( + ['skill_tree.yaml', 'domain_map.yaml', 'skill_spec.md'].map((file) => + read(`skills/_artifacts/${file}`), + ), + ).toEqual(records) +}) + afterEach(() => { process.chdir(previousCwd) vi.restoreAllMocks() From 272217faaca5bffc7dea935e7c7e8387b14ff4b2 Mon Sep 17 00:00:00 2001 From: ladybluenotes Date: Sat, 12 Sep 2026 20:48:28 -0700 Subject: [PATCH 7/8] test: initialize maintainer benchmark fixtures in the measured runner --- benchmarks/intent/maintainer.bench.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/benchmarks/intent/maintainer.bench.ts b/benchmarks/intent/maintainer.bench.ts index 2e168821..853353cf 100644 --- a/benchmarks/intent/maintainer.bench.ts +++ b/benchmarks/intent/maintainer.bench.ts @@ -11,8 +11,9 @@ const cliPath = fileURLToPath( for (const count of [20, 200]) { describe(`maintainer setup with ${count} existing skills`, () => { - let root: string - beforeAll(() => { + let root: string | undefined + function setup() { + if (root) return root = createTempDir('maintainer-setup') writeJson(join(root, 'package.json'), { name: '@bench/library', @@ -31,15 +32,21 @@ for (const count of [20, 200]) { execFileSync('git', ['-c', 'core.fsmonitor=false', 'init', '-q'], { cwd: root, }) - }) - afterAll(() => rmSync(root, { recursive: true, force: true })) + } + 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 }) + rmSync(join(root!, path), { recursive: true, force: true }) } const result = spawnSync( process.execPath, @@ -59,7 +66,7 @@ for (const count of [20, 200]) { ) } }, - { warmupIterations: 3, time: 3_000 }, + { warmupIterations: 3, time: 3_000, setup, teardown }, ) }) } From 7ca9700119973774afd7b31b6b0013e65b05aaf2 Mon Sep 17 00:00:00 2001 From: Sarah Gerrard Date: Sun, 13 Sep 2026 12:17:19 -0700 Subject: [PATCH 8/8] fix: point setup at domain_map.yaml when a mapped domain is blank --- packages/intent/src/commands/maintainer.ts | 11 +++++++++-- packages/intent/tests/maintainer.test.ts | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/intent/src/commands/maintainer.ts b/packages/intent/src/commands/maintainer.ts index 8b304a25..353451ff 100644 --- a/packages/intent/src/commands/maintainer.ts +++ b/packages/intent/src/commands/maintainer.ts @@ -290,8 +290,15 @@ export async function runMaintainerCommand( })), [], (index, error) => { - candidates[index]!.problems.push( - error instanceof Error ? error.message : String(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.`, ) }, ) diff --git a/packages/intent/tests/maintainer.test.ts b/packages/intent/tests/maintainer.test.ts index 091e245f..3f0b3e30 100644 --- a/packages/intent/tests/maintainer.test.ts +++ b/packages/intent/tests/maintainer.test.ts @@ -395,7 +395,7 @@ it('keeps valid registrations when the planner rejects a candidate in the batch' 'Registered `broken`', ) expect(vi.mocked(console.log).mock.calls.flat().join('\n')).toContain( - 'Skipped skills/broken/SKILL.md: Choose the task domain', + '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)