diff --git a/.changeset/reuse-maintainer-reads.md b/.changeset/reuse-maintainer-reads.md new file mode 100644 index 00000000..7e9450b9 --- /dev/null +++ b/.changeset/reuse-maintainer-reads.md @@ -0,0 +1,5 @@ +--- +'@tanstack/intent': patch +--- + +Reuse parsed planning records, source hashes, and Git source matches within maintainer operations. Keep fresh checks for later reports and writes. diff --git a/packages/intent/src/commands/maintainer.ts b/packages/intent/src/commands/maintainer.ts index 5e5e5b30..2859f7d6 100644 --- a/packages/intent/src/commands/maintainer.ts +++ b/packages/intent/src/commands/maintainer.ts @@ -3,6 +3,7 @@ import { dirname, relative, resolve } from 'node:path' import { isCI } from 'std-env' import { fail } from '../shared/cli-error.js' import { + readRecord, resolveMaintainerProject, setupRecords, } from '../maintainer/project.js' @@ -186,7 +187,9 @@ export async function runMaintainerCommand( console.log( 'For existing skills, run intent maintainer adopt to review registrations.', ) - const distribution = readDistribution(project) + const distribution = readDistribution( + readRecord(project, 'skill_tree.yaml'), + ) if (!distribution) console.log(distributionChoice) console.log( `Repository distribution: ${distribution?.mode ?? 'unconfigured'}. Run maintainer sync after authoring to update export metadata.`, diff --git a/packages/intent/src/maintainer/add.ts b/packages/intent/src/maintainer/add.ts index dc5aedd3..ff98659a 100644 --- a/packages/intent/src/maintainer/add.ts +++ b/packages/intent/src/maintainer/add.ts @@ -50,8 +50,8 @@ export function planAddSkills( initialChanges: Array = [], ) { const changes = [...initialChanges] - const entries = skillEntries(project, changes) const tree = readRecord(project, 'skill_tree.yaml', changes) + const entries = skillEntries(project, tree) const map = readRecord(project, 'domain_map.yaml', changes) const specPath = recordPath(project, 'skill_spec.md') const specChange = changes.find((change) => change.path === specPath) diff --git a/packages/intent/src/maintainer/adopt.ts b/packages/intent/src/maintainer/adopt.ts index 7ee62c87..7bf0a807 100644 --- a/packages/intent/src/maintainer/adopt.ts +++ b/packages/intent/src/maintainer/adopt.ts @@ -67,8 +67,10 @@ export function createAdoptionPlan( return [path, existsSync(path) ? readFileSync(path, 'utf8') : null] }, ) - const hasTree = existsSync(recordPath(project, 'skill_tree.yaml')) - const entries = hasTree ? skillEntries(project) : [] + 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('\\', '/'), @@ -232,7 +234,7 @@ export function createAdoptionPlan( fingerprint: createHash('sha256') .update(JSON.stringify([project, directory, snapshot, skills])) .digest('hex'), - distribution: (hasTree ? readDistribution(project) : undefined) ?? { + distribution: (tree ? readDistribution(tree) : undefined) ?? { mode: 'unconfigured' as const, repository: inferDistributionRepository(project), }, diff --git a/packages/intent/src/maintainer/distribution.ts b/packages/intent/src/maintainer/distribution.ts index 32dfaf3d..53fe0e39 100644 --- a/packages/intent/src/maintainer/distribution.ts +++ b/packages/intent/src/maintainer/distribution.ts @@ -12,7 +12,7 @@ import { skillPath, } from './project.js' import { writeChanges } from './files.js' -import type { MaintainerProject } from './project.js' +import type { MaintainerProject, SkillEntry } from './project.js' import type { FileChange } from './files.js' export interface DistributionOptions { @@ -34,14 +34,9 @@ const repositoryPattern = const namePattern = /^[a-z0-9]+(?:-[a-z0-9]+)*$/ export function readDistribution( - project: MaintainerProject, - changes: ReadonlyArray = [], + tree: ReturnType, ): Distribution | undefined { - const value: unknown = readRecord( - project, - 'skill_tree.yaml', - changes, - ).document.toJS().distribution + const value: unknown = tree.document.toJS().distribution if (value === undefined) return undefined if (!isObject(value) || !['repo', 'none'].includes(String(value.mode))) throw new Error('skill_tree.yaml distribution.mode must be repo or none.') @@ -108,7 +103,8 @@ export function planDistributionChoice( } if (!['repo', 'none'].includes(options.distribution)) throw new Error('--distribution must be repo or none.') - const previous = readDistribution(project, changes) + const tree = readRecord(project, 'skill_tree.yaml', changes) + const previous = readDistribution(tree) let distribution: Distribution if (options.distribution === 'none') { if (options.repository || options.pluginName || options.skill) @@ -154,7 +150,7 @@ export function planDistributionChoice( throw new Error( 'Select public skills explicitly with --skill (repeat for multiple skills).', ) - const entries = skillEntries(project, changes) + const entries = skillEntries(project, tree) for (const selected of skills) { const entry = entries.find( (skill) => (skill.slug ?? skill.name) === selected, @@ -168,7 +164,6 @@ export function planDistributionChoice( } distribution = { mode: 'repo', repository, name, skills } } - const tree = readRecord(project, 'skill_tree.yaml', changes) if (JSON.stringify(previous) === JSON.stringify(distribution)) return tree.document.set('distribution', distribution) return { @@ -213,8 +208,12 @@ function shellCommand(args: Array): string { .join(' ') } -export function planDistribution(project: MaintainerProject) { - const config = readDistribution(project) +export function planDistribution( + project: MaintainerProject, + tree: ReturnType, + entries: ReadonlyArray, +) { + const config = readDistribution(tree) const changes: Array = [] const problems: Array = [] const commands: Array = [] @@ -228,7 +227,6 @@ export function planDistribution(project: MaintainerProject) { if (config.mode === 'none' && !config.name) return { changes, problems, commands, mode: config.mode } const { name, repository } = config - const entries = skillEntries(project) const selected = config.mode === 'repo' ? config.skills! : [] const exported: Array<{ name: string; path: string }> = [] for (const selectedName of selected) { diff --git a/packages/intent/src/maintainer/project.ts b/packages/intent/src/maintainer/project.ts index 044868a3..3b6a5bf4 100644 --- a/packages/intent/src/maintainer/project.ts +++ b/packages/intent/src/maintainer/project.ts @@ -121,13 +121,9 @@ export interface SkillEntry extends Record { export function skillEntries( project: MaintainerProject, - changes: ReadonlyArray = [], + tree: ReturnType, ): Array { - const entries: Array = readRecord( - project, - 'skill_tree.yaml', - changes, - ).document.toJS().skills + const entries: Array = tree.document.toJS().skills const names = new Set() const paths = new Set() return entries.map((entry) => { diff --git a/packages/intent/src/maintainer/sync.ts b/packages/intent/src/maintainer/sync.ts index d9a65b7d..04ca2ac3 100644 --- a/packages/intent/src/maintainer/sync.ts +++ b/packages/intent/src/maintainer/sync.ts @@ -20,8 +20,9 @@ import type { FileChange } from './files.js' export function planMaintainerSync(project: MaintainerProject) { const tree = readRecord(project, 'skill_tree.yaml') + const originalTree = JSON.stringify(tree.document.toJS()) const map = readRecord(project, 'domain_map.yaml').document.toJS() - const entries = skillEntries(project) + const entries = skillEntries(project, tree) const changes: Array = [] const problems: Array = [] const packages = new Map>() @@ -195,15 +196,12 @@ export function planMaintainerSync(project: MaintainerProject) { if (source !== content) changes.push({ path, source, content }) } const nextTree = tree.document.toString() - if ( - JSON.stringify(tree.document.toJS()) !== - JSON.stringify(readRecord(project, 'skill_tree.yaml').document.toJS()) - ) + if (JSON.stringify(tree.document.toJS()) !== originalTree) changes.push({ path: tree.path, source: tree.source, content: nextTree }) const spec = readFileSync(recordPath(project, 'skill_spec.md'), 'utf8') if (!spec.trim() || spec.includes(authoringMarker)) problems.push('skill_spec.md still needs authored coverage and decisions.') - const distribution = planDistribution(project) + const distribution = planDistribution(project, tree, entries) changes.push(...distribution.changes) problems.push(...distribution.problems) return { changes, problems, skills, distribution } diff --git a/packages/intent/src/review/review.ts b/packages/intent/src/review/review.ts index 62067ed8..99d7b2b7 100644 --- a/packages/intent/src/review/review.ts +++ b/packages/intent/src/review/review.ts @@ -146,18 +146,6 @@ function safePath(root: string, path: string): string { return current } -function fileHash(root: string, path: string): string | null { - const absolute = safePath(root, path) - try { - if (!lstatSync(absolute).isFile()) - throw new Error(`Cannot review non-file: ${JSON.stringify(path)}`) - return digest(readFileSync(absolute)) - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') return null - throw error - } -} - function isObject(value: unknown): value is Record { return !!value && typeof value === 'object' && !Array.isArray(value) } @@ -307,23 +295,6 @@ function sourcePattern( return `:(top,glob)${path}` } -function snapshot( - root: string, - paths: Array, - problems: Array, -): Snapshot { - return Object.fromEntries( - sorted(paths).map((path) => { - try { - return [path, fileHash(root, path)] - } catch (error) { - problems.push(error instanceof Error ? error.message : String(error)) - return [path, null] - } - }), - ) -} - export function createReview(cwd: string, baseRef?: string): ReviewReport { let root: string try { @@ -383,6 +354,23 @@ export function createReview(cwd: string, baseRef?: string): ReviewReport { const names = repositoryNames(root) const covered = new Set() const items: Array = [] + const hashes = new Map() + const sourceMatches = new Map>() + function fileHash(path: string): string | null { + const cached = hashes.get(path) + if (cached !== undefined) return cached + const absolute = safePath(root, path) + let hash: string | null = null + try { + if (!lstatSync(absolute).isFile()) + throw new Error(`Cannot review non-file: ${JSON.stringify(path)}`) + hash = digest(readFileSync(absolute)) + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error + } + hashes.set(path, hash) + return hash + } function add( kind: ReviewItem['kind'], path: string, @@ -390,7 +378,16 @@ export function createReview(cwd: string, baseRef?: string): ReviewReport { problems: Array, ) { const id = `${kind}:${path}` - const current = snapshot(root, paths, problems) + const current: Snapshot = Object.fromEntries( + sorted(paths).map((file) => { + try { + return [file, fileHash(file)] + } catch (error) { + problems.push(error instanceof Error ? error.message : String(error)) + return [file, null] + } + }), + ) const previous = state?.items[id] const fingerprint = reviewFingerprint(id, current, problems) if ( @@ -471,7 +468,7 @@ export function createReview(cwd: string, baseRef?: string): ReviewReport { let frontmatter: Record | null let skillHash: string | null try { - skillHash = fileHash(root, file) + skillHash = fileHash(file) if (skillHash === null) continue frontmatter = parseFrontmatter(safePath(root, file)) } catch (error) { @@ -505,7 +502,11 @@ export function createReview(cwd: string, baseRef?: string): ReviewReport { if (typeof source !== 'string') throw new Error('Source entries must be strings.') const pattern = sourcePattern(source, packageDir, names) - const matches = sorted([...list([pattern]), ...diff([pattern])]) + let matches = sourceMatches.get(pattern) + if (matches === undefined) { + matches = sorted([...list([pattern]), ...diff([pattern])]) + sourceMatches.set(pattern, matches) + } if (matches.length === 0 && !sourceMappingWasRecorded) throw new Error(`Source matched no available files: ${source}`) sourceFiles.push(...matches) @@ -525,7 +526,7 @@ export function createReview(cwd: string, baseRef?: string): ReviewReport { ].some( (path) => files.includes(path) && - fileHash(root, path) !== null && + fileHash(path) !== null && readFileSync(safePath(root, path), 'utf8').includes( '', ), @@ -553,7 +554,7 @@ export function createReview(cwd: string, baseRef?: string): ReviewReport { ) continue } - if (fileHash(root, path) === null) { + if (fileHash(path) === null) { problems.push(`Missing required planning record: ${path}`) continue } @@ -595,7 +596,7 @@ export function createReview(cwd: string, baseRef?: string): ReviewReport { if (!id.startsWith('skill:')) continue const path = id.slice('skill:'.length) if (files.includes(path)) continue - if (fileHash(root, path) === null && !changed.includes(path)) { + if (fileHash(path) === null && !changed.includes(path)) { add('source', path, [path], []) } } diff --git a/packages/intent/tests/distribution.test.ts b/packages/intent/tests/distribution.test.ts index 62b50a75..d95828b0 100644 --- a/packages/intent/tests/distribution.test.ts +++ b/packages/intent/tests/distribution.test.ts @@ -10,8 +10,14 @@ import { import { tmpdir } from 'node:os' import { dirname, join } from 'node:path' import { afterEach, beforeEach, expect, it, vi } from 'vitest' -import { parse, stringify } from 'yaml' +import { parse, parseDocument, stringify } from 'yaml' import { main } from '../src/cli.js' +import type * as Yaml from 'yaml' + +vi.mock('yaml', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, parseDocument: vi.fn(actual.parseDocument) } +}) let root: string let previousCwd: string @@ -85,6 +91,29 @@ afterEach(() => { rmSync(root, { recursive: true, force: true }) }) +it('plans selected distribution from one parsed skill tree', async () => { + expect( + await main([ + 'maintainer', + 'setup', + '--distribution', + 'repo', + '--skill', + 'query', + ]), + ).toBe(0) + const tree = read('_artifacts/skill_tree.yaml') + vi.mocked(parseDocument).mockClear() + + expect(await main(['maintainer', 'sync'])).toBe(0) + expect( + vi.mocked(parseDocument).mock.calls.filter(([source]) => source === tree), + ).toHaveLength(1) + expect(readJson('.claude-plugin/plugin.json').skills).toEqual([ + './packages/client/skills/query', + ]) +}) + it('explains the missing choice, remembers an opt-out, and does not ask again', async () => { expect(vi.mocked(console.log).mock.calls.flat().join('\n')).toContain( '--distribution', diff --git a/packages/intent/tests/review.test.ts b/packages/intent/tests/review.test.ts index a6bf8747..5d679026 100644 --- a/packages/intent/tests/review.test.ts +++ b/packages/intent/tests/review.test.ts @@ -14,6 +14,7 @@ import { dirname, join } from 'node:path' import { afterEach, beforeEach, expect, it, vi } from 'vitest' import { main } from '../src/cli.js' import { createReview, recordReview } from '../src/review/review.js' +import type * as NodeChildProcess from 'node:child_process' import type * as NodeFs from 'node:fs' // These tests run complete review lifecycles against real Git repositories. @@ -21,7 +22,16 @@ vi.setConfig({ testTimeout: 30_000 }) vi.mock('node:fs', async (importOriginal) => { const actual = await importOriginal() - return { ...actual, renameSync: vi.fn(actual.renameSync) } + return { + ...actual, + readFileSync: vi.fn(actual.readFileSync), + renameSync: vi.fn(actual.renameSync), + } +}) + +vi.mock('node:child_process', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, execFileSync: vi.fn(actual.execFileSync) } }) let root: string @@ -75,6 +85,45 @@ it('reviews an initial skill and remembers a justified no-op', () => { expect(createReview(root).items).toEqual([]) }) +it('reuses shared source matches and hashes within each review report', () => { + write( + 'skills/retry/SKILL.md', + '---\nname: retry\ndescription: Retry safely\nsources: [acme/library:src/**/*.ts]\n---\nRetry the request.\n', + ) + planningRecords('_artifacts') + vi.mocked(readFileSync).mockClear() + vi.mocked(execFileSync).mockClear() + + const report = createReview(root) + expect(report.items.map((item) => item.id)).toEqual([ + 'skill:skills/request/SKILL.md', + 'skill:skills/retry/SKILL.md', + 'planning:_artifacts', + ]) + expect({ + sourceReads: vi + .mocked(readFileSync) + .mock.calls.filter( + ([file]) => file === join(report.root, 'src/request.ts'), + ).length, + sourceQueries: vi + .mocked(execFileSync) + .mock.calls.filter( + ([, args]) => + Array.isArray(args) && args.includes(':(top,glob)src/**/*.ts'), + ).length, + }).toEqual({ sourceReads: 1, sourceQueries: 2 }) + + const updated = 'export const attempts = 4\n' + write('src/request.ts', updated) + const next = createReview(root) + const hash = createHash('sha256').update(updated).digest('hex') + for (const item of next.items) { + expect(item.snapshot['src/request.ts']).toBe(hash) + expect(item.changedFiles).toContain('src/request.ts') + } +}) + it('does not classify shipped meta skills or unrelated agent instructions as library skills', () => { write( 'packages/intent/meta/generate-skill/SKILL.md',