diff --git a/.changeset/clean-parks-itch.md b/.changeset/clean-parks-itch.md new file mode 100644 index 0000000..7c1c847 --- /dev/null +++ b/.changeset/clean-parks-itch.md @@ -0,0 +1,7 @@ +--- +'@tanstack/intent': minor +--- + +Add `intent validate --check` and `intent validate --fix` for mechanical `SKILL.md` frontmatter migrations. + +`--check` reports pending migrations without writing files. `--fix` rewrites fixable `name` and metadata scalar migrations, then re-runs validation. diff --git a/docs/cli/intent-validate.md b/docs/cli/intent-validate.md index 6e6e8b9..b54b4d2 100644 --- a/docs/cli/intent-validate.md +++ b/docs/cli/intent-validate.md @@ -6,7 +6,7 @@ id: intent-validate `intent validate` checks `SKILL.md` files and artifacts for structural problems. ```bash -npx @tanstack/intent@latest validate [] +npx @tanstack/intent@latest validate [] [--github-summary] [--fix] [--check] ``` ## Arguments @@ -14,6 +14,39 @@ npx @tanstack/intent@latest validate [] - ``: directory containing skills; default is `skills` - Relative paths are resolved from the current working directory +## Options + +- `--github-summary`: write a GitHub Actions step summary when `GITHUB_STEP_SUMMARY` is set +- `--check`: fail if any `SKILL.md` has fixable frontmatter migrations pending, without writing files +- `--fix`: rewrite fixable `SKILL.md` frontmatter migrations, then validate the result + +## Frontmatter migration fixes + +Use `--check` in CI to detect mechanical frontmatter migrations that have not been applied: + +```bash +npx @tanstack/intent@latest validate --check +``` + +Use `--fix` locally to apply the mechanical frontmatter migrations: + +```bash +npx @tanstack/intent@latest validate --fix +``` + +`--fix` only rewrites unambiguous frontmatter migrations: + +- `name` values are rewritten to the parent directory leaf when the parent directory is already a legal skill name +- Top-level string fields `type`, `library`, `library_version`, and `framework` are moved under `metadata` + +`--fix` does not rewrite authoring-judgment validation errors: + +- Missing or invalid `description` +- Length-limit failures +- Invalid `metadata` shape or non-string `metadata` values +- Missing `requires` for framework skills +- Artifact validation failures + ## Validation checks For each discovered `SKILL.md`: diff --git a/package.json b/package.json index e3fb573..360be17 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,7 @@ "test:knip": "knip", "test:lib": "nx affected --targets=test:lib --exclude=examples/**", "test:lib:dev": "pnpm test:lib && nx watch --all -- pnpm test:lib", - "test:pr": "tsc --noEmit && nx affected --targets=test:eslint,test:sherif,test:knip,test:docs,test:lib,test:integration,test:types,build", + "test:pr": "tsc --noEmit && nx affected --targets=test:eslint,test:sherif,test:knip,test:docs,test:lib,test:types,build", "test:sherif": "sherif", "test:types": "nx affected --targets=test:types --exclude=examples/**", "watch": "pnpm run build:all && nx watch --all -- pnpm run build:all" diff --git a/packages/intent/src/cli.ts b/packages/intent/src/cli.ts index 2ccd7f2..481590e 100644 --- a/packages/intent/src/cli.ts +++ b/packages/intent/src/cli.ts @@ -87,8 +87,13 @@ function createCli(): CAC { cli .command('validate [dir]', 'Validate skill files') - .usage('validate [dir] [--github-summary]') + .usage('validate [dir] [--github-summary] [--fix] [--check]') .option('--github-summary', 'Write a GitHub Actions step summary') + .option('--fix', 'Rewrite fixable SKILL.md frontmatter issues') + .option( + '--check', + 'Fail if fixable SKILL.md frontmatter issues would be rewritten', + ) .example('validate') .example('validate packages/query/skills') .action( diff --git a/packages/intent/src/commands/validate.ts b/packages/intent/src/commands/validate.ts index c6981c0..e582700 100644 --- a/packages/intent/src/commands/validate.ts +++ b/packages/intent/src/commands/validate.ts @@ -1,4 +1,9 @@ -import { appendFileSync, existsSync, readFileSync } from 'node:fs' +import { + appendFileSync, + existsSync, + readFileSync, + writeFileSync, +} from 'node:fs' import { basename, dirname, join, relative, resolve } from 'node:path' import { fail, isCliFailure } from '../cli-error.js' import { printWarnings } from '../cli-support.js' @@ -16,7 +21,15 @@ interface ValidationWarning { message: string } +interface FrontmatterFixPlan { + file: string + filePath: string + changes: Array +} + export interface ValidateCommandOptions { + check?: boolean + fix?: boolean githubSummary?: boolean } @@ -36,6 +49,13 @@ const specTopLevelKeys = new Set([ // structured surface is tracked separately (#161), so they are not flagged here. const intentArrayKeys = new Set(['sources', 'requires']) +const metadataScalarKeys = [ + 'type', + 'library', + 'library_version', + 'framework', +] as const + function isScalarValue(value: unknown): boolean { return ( typeof value === 'string' || @@ -118,6 +138,115 @@ function isRecord(value: unknown): value is Record { return !!value && typeof value === 'object' && !Array.isArray(value) } +function collectFrontmatterFixPlan({ + filePath, + fm, + rel, +}: { + filePath: string + fm: Record + rel: string +}): FrontmatterFixPlan | null { + const changes: Array = [] + const parentDir = basename(dirname(filePath)) + + if ( + typeof fm.name === 'string' && + (fm.name.includes('/') || fm.name !== parentDir) && + agentSkillNamePattern.test(parentDir) + ) { + changes.push(`rewrite name to "${parentDir}"`) + } + + const metadata = fm.metadata + const canMoveMetadata = metadata === undefined || isRecord(metadata) + if (canMoveMetadata) { + const metadataRecord = isRecord(metadata) ? metadata : undefined + for (const key of metadataScalarKeys) { + if (typeof fm[key] !== 'string') continue + + if (metadataRecord && key in metadataRecord) { + changes.push( + `remove top-level "${key}"; metadata.${key} already exists`, + ) + } else { + changes.push(`move top-level "${key}" under metadata.${key}`) + } + } + } + + return changes.length > 0 ? { file: rel, filePath, changes } : null +} + +function normalizeLineEndings(value: string, lineEnding: string): string { + return lineEnding === '\r\n' ? value.replace(/\r?\n/g, '\r\n') : value +} + +async function applyFrontmatterFixes( + fixPlans: Array, +): Promise { + const { parseDocument } = await import('yaml') + + for (const plan of fixPlans) { + const content = readFileSync(plan.filePath, 'utf8') + const match = content.match( + /^---(\r?\n)([\s\S]*?)(\r?\n)---(\r?\n?)([\s\S]*)/, + ) + if (!match) continue + + const openingLineEnding = match[1] + const frontmatter = match[2] + const closingLineEnding = match[3] + const afterClose = match[4] + const body = match[5] + if ( + openingLineEnding === undefined || + frontmatter === undefined || + closingLineEnding === undefined || + afterClose === undefined || + body === undefined + ) { + continue + } + + const doc = parseDocument(frontmatter) + if (doc.errors.length > 0) continue + + const fm = doc.toJS() as Record + const parentDir = basename(dirname(plan.filePath)) + + if ( + typeof fm.name === 'string' && + (fm.name.includes('/') || fm.name !== parentDir) && + agentSkillNamePattern.test(parentDir) + ) { + doc.set('name', parentDir) + } + + const metadata = fm.metadata + const canMoveMetadata = metadata === undefined || isRecord(metadata) + if (canMoveMetadata) { + for (const key of metadataScalarKeys) { + const value = fm[key] + if (typeof value !== 'string') continue + + if (!doc.hasIn(['metadata', key])) { + const valueNode = doc.get(key, true) + doc.setIn(['metadata', key], valueNode ?? value) + } + doc.delete(key) + } + } + + const nextFrontmatter = normalizeLineEndings( + doc.toString().replace(/\r?\n$/, ''), + openingLineEnding, + ) + const nextContent = `---${openingLineEnding}${nextFrontmatter}${closingLineEnding}---${afterClose}${body}` + writeFileSync(plan.filePath, nextContent) + } +} + function collectAgentSkillSpecWarnings({ fm, rel, @@ -174,13 +303,17 @@ export async function runValidateCommand( dir?: string, options: ValidateCommandOptions = {}, ): Promise { + if (options.fix && options.check) { + fail('Cannot combine --fix and --check') + } + if (!options.githubSummary) { - await runValidateCommandInternal(dir) + await runValidateCommandInternal(dir, options) return } try { - await runValidateCommandInternal(dir) + await runValidateCommandInternal(dir, options) writeGithubValidationSummary({ ok: true }) } catch (err) { writeGithubValidationSummary({ @@ -191,7 +324,10 @@ export async function runValidateCommand( } } -async function runValidateCommandInternal(dir?: string): Promise { +async function runValidateCommandInternal( + dir?: string, + options: ValidateCommandOptions = {}, +): Promise { const [{ parse: parseYaml }, { findSkillFiles, readScalarField }] = await Promise.all([import('yaml'), import('../utils.js')]) const context = resolveProjectContext({ @@ -209,6 +345,7 @@ async function runValidateCommandInternal(dir?: string): Promise { const errors: Array = [] const warnings: Array = [] + const fixPlans: Array = [] let validatedCount = 0 if (explicitDir && findSkillFiles(skillsDirs[0]!).length === 0) { @@ -254,6 +391,9 @@ async function runValidateCommandInternal(dir?: string): Promise { continue } + const fixPlan = collectFrontmatterFixPlan({ filePath, fm, rel }) + if (fixPlan) fixPlans.push(fixPlan) + if (!fm.name) { errors.push({ file: rel, message: 'Missing required field: name' }) } @@ -398,6 +538,22 @@ async function runValidateCommandInternal(dir?: string): Promise { warnings.push(...collectPackagingWarnings(validateContext)) } + if (options.check) { + for (const plan of fixPlans) { + errors.push({ + file: plan.file, + message: `fixable frontmatter migration pending: ${plan.changes.join('; ')}`, + }) + } + } + + if (options.fix && fixPlans.length > 0) { + await applyFrontmatterFixes(fixPlans) + console.log(`✅ Fixed ${fixPlans.length} skill files`) + await runValidateCommandInternal(dir, { ...options, fix: false }) + return + } + if (errors.length > 0) { fail(buildValidationFailure(errors, warnings)) } diff --git a/packages/intent/tests/cli.test.ts b/packages/intent/tests/cli.test.ts index 8d858fc..baba931 100644 --- a/packages/intent/tests/cli.test.ts +++ b/packages/intent/tests/cli.test.ts @@ -1766,6 +1766,285 @@ describe('cli commands', () => { ) }) + it('reports fixable frontmatter migrations in check mode without writing', async () => { + const root = mkdtempSync(join(realTmpdir, 'intent-cli-validate-check-')) + tempDirs.push(root) + + const skillPath = join(root, 'skills', 'core', 'setup', 'SKILL.md') + mkdirSync(dirname(skillPath), { recursive: true }) + const original = [ + '---', + 'name: core/setup', + 'description: Core setup concepts', + 'type: framework', + 'library: core', + '---', + '', + 'Skill content here.', + '', + ].join('\n') + writeFileSync(skillPath, original) + + process.chdir(root) + + const exitCode = await main(['validate', '--check']) + const output = errorSpy.mock.calls.flat().join('\n') + + expect(exitCode).toBe(1) + expect(output).toContain('fixable frontmatter migration pending') + expect(output).toContain('rewrite name to "setup"') + expect(output).toContain('move top-level "type" under metadata.type') + expect(readFileSync(skillPath, 'utf8')).toBe(original) + }) + + it('passes check mode when skills are already compliant', async () => { + const root = mkdtempSync(join(realTmpdir, 'intent-cli-validate-check-ok-')) + tempDirs.push(root) + + writeSkillMd(join(root, 'skills', 'db-core'), { + name: 'db-core', + description: 'Core database concepts', + }) + + process.chdir(root) + + const exitCode = await main(['validate', '--check']) + const output = logSpy.mock.calls.flat().join('\n') + + expect(exitCode).toBe(0) + expect(output).toContain('✅ Validated 1 skill files — all passed') + }) + + it('fixes mechanical frontmatter migrations and validates the result', async () => { + const root = mkdtempSync(join(realTmpdir, 'intent-cli-validate-fix-')) + tempDirs.push(root) + + const skillPath = join(root, 'skills', 'core', 'setup', 'SKILL.md') + mkdirSync(dirname(skillPath), { recursive: true }) + writeFileSync( + skillPath, + [ + '---', + 'name: core/setup', + 'description: Core setup concepts', + 'type: core', + 'library: core', + '---', + '', + 'Skill content here.', + '', + ].join('\n'), + ) + + process.chdir(root) + + const exitCode = await main(['validate', '--fix']) + const output = logSpy.mock.calls.flat().join('\n') + const fixed = readFileSync(skillPath, 'utf8') + + expect(exitCode).toBe(0) + expect(output).toContain('✅ Fixed 1 skill files') + expect(output).toContain('✅ Validated 1 skill files — all passed') + expect(fixed).toContain('name: setup') + expect(fixed).toContain('metadata:\n type: core\n library: core') + expect(fixed).not.toContain('\ntype: core') + expect(fixed).not.toContain('\nlibrary: core') + expect(fixed).toContain('\nSkill content here.\n') + }) + + it('keeps existing metadata values when removing conflicting top-level scalars', async () => { + const root = mkdtempSync( + join(realTmpdir, 'intent-cli-validate-fix-conflict-'), + ) + tempDirs.push(root) + + const skillPath = join(root, 'skills', 'db-core', 'SKILL.md') + mkdirSync(dirname(skillPath), { recursive: true }) + writeFileSync( + skillPath, + [ + '---', + 'name: db-core', + 'description: Core database concepts', + 'metadata:', + ' library: nested', + 'library: top', + '---', + '', + 'Skill content here.', + '', + ].join('\n'), + ) + + process.chdir(root) + + const exitCode = await main(['validate', '--fix']) + const fixed = readFileSync(skillPath, 'utf8') + + expect(exitCode).toBe(0) + expect(fixed).toContain('metadata:\n library: nested') + expect(fixed).not.toContain('\nlibrary: top') + }) + + it('fixes names while leaving scalar migrations blocked by non-mapping metadata', async () => { + const root = mkdtempSync( + join(realTmpdir, 'intent-cli-validate-fix-meta-block-'), + ) + tempDirs.push(root) + + const skillPath = join(root, 'skills', 'core', 'setup', 'SKILL.md') + mkdirSync(dirname(skillPath), { recursive: true }) + writeFileSync( + skillPath, + [ + '---', + 'name: core/setup', + 'description: Core setup concepts', + 'metadata: nope', + 'type: core', + '---', + '', + 'Skill content here.', + '', + ].join('\n'), + ) + + process.chdir(root) + + const exitCode = await main(['validate', '--fix']) + const output = errorSpy.mock.calls.flat().join('\n') + const fixed = readFileSync(skillPath, 'utf8') + + expect(exitCode).toBe(1) + expect(output).toContain('metadata must be a mapping') + expect(fixed).toContain('name: setup') + expect(fixed).toContain('type: core') + }) + + it('preserves CRLF line endings and markdown body bytes when fixing frontmatter', async () => { + const root = mkdtempSync(join(realTmpdir, 'intent-cli-validate-fix-crlf-')) + tempDirs.push(root) + + const skillPath = join(root, 'skills', 'db-core', 'SKILL.md') + mkdirSync(dirname(skillPath), { recursive: true }) + const body = 'First body line.\r\n\r\nSecond body line.\r\n' + writeFileSync( + skillPath, + [ + '---', + 'name: wrong-name', + 'description: Core database concepts', + 'type: core', + '---', + '', + ].join('\r\n') + body, + ) + + process.chdir(root) + + const exitCode = await main(['validate', '--fix']) + const fixed = readFileSync(skillPath, 'utf8') + const fixedBody = fixed.slice(fixed.indexOf(body)) + + expect(exitCode).toBe(0) + expect(fixed).toContain('name: db-core\r\n') + expect(fixed).toContain('metadata:\r\n type: core\r\n') + expect(fixedBody).toBe(body) + }) + + it('preserves trailing comments on migrated scalar values', async () => { + const root = mkdtempSync( + join(realTmpdir, 'intent-cli-validate-fix-comments-'), + ) + tempDirs.push(root) + + const skillPath = join(root, 'skills', 'db-core', 'SKILL.md') + mkdirSync(dirname(skillPath), { recursive: true }) + writeFileSync( + skillPath, + [ + '---', + 'name: db-core', + 'description: Core database concepts', + 'library: core # keep this comment', + '---', + '', + 'Skill content here.', + '', + ].join('\n'), + ) + + process.chdir(root) + + const exitCode = await main(['validate', '--fix']) + const fixed = readFileSync(skillPath, 'utf8') + + expect(exitCode).toBe(0) + expect(fixed).toContain('metadata:\n library: core # keep this comment') + }) + + it('does not fix names when the parent directory is not a legal skill name', async () => { + const root = mkdtempSync( + join(realTmpdir, 'intent-cli-validate-fix-invalid-parent-'), + ) + tempDirs.push(root) + + writeSkillMd(join(root, 'skills', 'PDF-Processing'), { + name: 'wrong-name', + description: 'PDF processing concepts', + }) + + process.chdir(root) + + const exitCode = await main(['validate', '--fix']) + const output = errorSpy.mock.calls.flat().join('\n') + + expect(exitCode).toBe(1) + expect(output).toContain( + 'name "wrong-name" does not match parent directory "PDF-Processing"', + ) + }) + + it('is idempotent after fixing mechanical frontmatter migrations', async () => { + const root = mkdtempSync( + join(realTmpdir, 'intent-cli-validate-fix-idempotent-'), + ) + tempDirs.push(root) + + const skillPath = join(root, 'skills', 'db-core', 'SKILL.md') + mkdirSync(dirname(skillPath), { recursive: true }) + writeFileSync( + skillPath, + [ + '---', + 'name: wrong-name', + 'description: Core database concepts', + 'type: core', + '---', + '', + 'Skill content here.', + '', + ].join('\n'), + ) + + process.chdir(root) + + const firstExitCode = await main(['validate', '--fix']) + const fixed = readFileSync(skillPath, 'utf8') + const secondExitCode = await main(['validate', '--fix']) + + expect(firstExitCode).toBe(0) + expect(secondExitCode).toBe(0) + expect(readFileSync(skillPath, 'utf8')).toBe(fixed) + }) + + it('fails cleanly when fix and check are combined', async () => { + const exitCode = await main(['validate', '--fix', '--check']) + + expect(exitCode).toBe(1) + expect(errorSpy).toHaveBeenCalledWith('Cannot combine --fix and --check') + }) + it('fails when a non-spec scalar field is emitted at the top level', async () => { const root = mkdtempSync(join(realTmpdir, 'intent-cli-validate-scalar-')) tempDirs.push(root) diff --git a/packages/intent/tests/integration/pnp-berry-corepack.test.ts b/packages/intent/tests/integration/pnp-berry-corepack.test.ts index 31965bc..193cc64 100644 --- a/packages/intent/tests/integration/pnp-berry-corepack.test.ts +++ b/packages/intent/tests/integration/pnp-berry-corepack.test.ts @@ -108,7 +108,8 @@ function scaffoldBerryProject(): string { dependencies: { '@repro/skills-leaf': `file:./${tarball}` }, }) - execFileSync('corepack', ['yarn', 'install'], { + // CI makes Berry installs immutable by default; this fixture creates lockfile fresh. + execFileSync('corepack', ['yarn', 'install', '--no-immutable'], { cwd: dir, stdio: 'pipe', env: corepackEnv,