diff --git a/.changeset/maintainer-authoring-commands.md b/.changeset/maintainer-authoring-commands.md new file mode 100644 index 00000000..e9a938ea --- /dev/null +++ b/.changeset/maintainer-authoring-commands.md @@ -0,0 +1,5 @@ +--- +'@tanstack/intent': patch +--- + +Record developer tasks at registration with `maintainer add --task`, retire a registered skill with `maintainer remove ` without deleting its guidance, and include the recording contract (allowed outcomes and required fields) in `review --json` reports. diff --git a/packages/intent/meta/generate-skill/references/maintainer-commands.md b/packages/intent/meta/generate-skill/references/maintainer-commands.md index d07418a7..e1d0d95f 100644 --- a/packages/intent/meta/generate-skill/references/maintainer-commands.md +++ b/packages/intent/meta/generate-skill/references/maintainer-commands.md @@ -1,13 +1,13 @@ # Run the maintainer workflow -Use the repository's Intent command for all six actions. These commands perform bookkeeping; the maintainer or coding agent still supplies task knowledge, source-backed guidance, and review conclusions. +Use the repository's Intent command for these actions. `intent maintainer --help` lists them in order with what each one writes; `intent maintainer --help` lists one action's options. These commands perform bookkeeping; the maintainer or coding agent still supplies task knowledge, source-backed guidance, and review conclusions. 1. Run `intent maintainer setup` once. It installs repository guidance and creates missing planning records, preserving existing documents. A monorepo uses one shared record and package-owned skill directories. If several record locations exist, select the established one with `--artifacts `; do not merge them by guessing. Read [repository distribution](distribution.md) and explain the option to the maintainer during setup. Save the selected skills or opt-out with the setup command; do not infer a public selection from directory placement or repeat a recorded decision. -2. For a new task, run `intent maintainer add --domain --description --source `. In a monorepo, pass `--package packages/`. Repeat `--source` or `--requires` for multiple entries. Source paths are relative to the owning package; `owner/repo:path` is relative to the repository. Use `--path /SKILL.md` for an established custom layout. To register an existing skill, supply its name, domain, package, and path; its frontmatter supplies the other fields. -3. Author the skill and reconcile all three records using the procedures in this skill. The command creates a skeleton and a domain-map entry with unassessed task coverage. Write that coverage and the spec's decisions/history. Remove `` only after authoring the corresponding document. Do not remove it simply to make a check pass. +2. For a new task, run `intent maintainer add --domain --description --source --task `. Repeat `--source`, `--requires`, and `--task` for multiple entries; each `--task` becomes an assessed developer task in the domain map. In a monorepo, run the command from the owning package directory or pass `--package packages/` relative to the repository root. Source paths are relative to the owning package; `owner/repo:path` is relative to the repository. Use `--path /SKILL.md` for an established custom layout. To register an existing skill, supply its name, domain, package, and path; its frontmatter supplies the other fields. The command prints every file it wrote. +3. Author the skill and reconcile all three records using the procedures in this skill. The command creates a skeleton and a domain-map entry; add any tasks not supplied on the command line. Write the spec's decisions/history. Remove `` only after authoring the corresponding document. Do not remove it simply to make a check pass. To retire a registered skill, run `intent maintainer remove `; it marks the tree entry `retired` and notes it in the spec without deleting the file, and refuses while the skill is selected for distribution or required by another skill. 4. Run `intent maintainer status` to see missing work, stale metadata, and pending reviews. `--json` includes the full source-review report. For a supplied PR base, use `--base `. 5. Run `intent maintainer sync` after edits. It copies descriptions, purpose, sources, and prerequisites from registered skills into the tree, repairs the tree's record links, and includes the skill directories in existing package `files` allowlists. It preserves authored map/spec content and other manifest fields. It does not change version claims or run a package release. An absent `files` allowlist stays absent so npm's default contents are preserved; check the actual packed archive as part of the package's release checks. -6. Follow [source review](source-review.md) with `intent maintainer review --json`, supply justified outcomes, and record them with `intent maintainer review --record .intent/review.json`. The command retains the existing revision and content-fingerprint checks. Run `intent maintainer check` after recording; it exits nonzero for incomplete authoring, stale generated metadata, invalid skills, missing local prerequisites, or pending reviews. Use the same check in CI, passing the actual PR base. +6. Follow [source review](source-review.md) with `intent maintainer review --json`, supply justified outcomes, and record them with `intent maintainer review --record .intent/review.json`. A maintainer working in a terminal can do the same with `intent maintainer review --interactive`. The command retains the existing revision and content-fingerprint checks. Run `intent maintainer check` after recording; it exits nonzero for incomplete authoring, stale generated metadata, invalid skills, missing local prerequisites, or pending reviews. Use the same check in CI, passing the actual PR base. Keep unimplemented future skills in the tree with `status: planned` and retired entries with `status: retired`. They remain part of the cumulative record but do not count as implemented skills or enter package publishing configuration. An active entry with a missing file is an error to resolve, not an invitation to delete the entry. Local prerequisite slugs are checked against implemented tree entries; verify external package prerequisites and the developer task through the task-quality procedure. diff --git a/packages/intent/meta/generate-skill/references/planning-records.md b/packages/intent/meta/generate-skill/references/planning-records.md index 3b20df6a..89a8133e 100644 --- a/packages/intent/meta/generate-skill/references/planning-records.md +++ b/packages/intent/meta/generate-skill/references/planning-records.md @@ -14,7 +14,28 @@ Run `intent maintainer setup` to create missing record skeletons without replaci ## Use the existing formats -Read [the domain map and skill spec formats](../../domain-discovery/references/artifacts.md) for those two documents and [the skill tree format](../../tree-generator/SKILL.md#scaffold-flow-output) for the third. Use the formats without entering full-library discovery or its interviews. The batch’s existing source research supplies discoverable facts; ask only for unresolved maintainer decisions. +Read [the domain map and skill spec formats](../../domain-discovery/references/artifacts.md) for those two documents. The skill tree uses the entry format below; `maintainer add` writes the required fields and `maintainer sync` keeps `description`, `purpose`, `sources`, and `requires` aligned with the skill's frontmatter. Read [the full tree format](../../tree-generator/SKILL.md#scaffold-flow-output) only for a full-library scaffold. Use the formats without entering full-library discovery or its interviews. The batch’s existing source research supplies discoverable facts; ask only for unresolved maintainer decisions. + +```yaml +library: { name: '[package-name]', version: '[version]' } +generated_from: + domain_map: skills/_artifacts/domain_map.yaml + skill_spec: skills/_artifacts/skill_spec.md +review: + ignore: ['[git glob]'] # optional: paths never reported as unmapped changes +distribution: { mode: none } # or { mode: repo, repository: owner/repo, name: plugin, skills: [slug] } +skills: + - name: '[task-focused skill name]' + slug: '[kebab-case]' + domain: '[domain slug]' + package: '[package directory]' # monorepo only; path is then relative to it + path: skills/[slug]/SKILL.md + status: planned | retired # omit for an implemented skill + description: '[activation guidance; synchronized from SKILL.md]' + purpose: '[descriptive explanation; synchronized from metadata.purpose]' + requires: ['[other skill slugs]'] # omit if none + sources: ['src/[path].ts', 'owner/repo:docs/[path].md'] +``` - `domain_map.yaml` owns the domain/task relationships, supported failure modes, cross-references, tensions, and knowledge gaps. Keep task slugs and package ownership aligned with the skills. - `skill_spec.md` is the human-readable coverage and decision record. Retain the existing inventories and add a **Coverage and batch history** section recording the assessed scope, each completed batch or behavior change, its source revision/version, consequential decisions and reasons, check outcomes, and remaining work. Keep entries concise; do not store transcripts. Distinguish implemented guidance from planned work and unassessed areas. diff --git a/packages/intent/meta/generate-skill/references/source-review.md b/packages/intent/meta/generate-skill/references/source-review.md index 4f16335d..61d0254f 100644 --- a/packages/intent/meta/generate-skill/references/source-review.md +++ b/packages/intent/meta/generate-skill/references/source-review.md @@ -12,17 +12,17 @@ For a new batch, review the selected skills. Other items remain pending unless t Read [the planning record procedure](planning-records.md), then review the actual diff, affected source, skill, references, relevant tests, and all three planning documents. Create missing records and reconcile changed coverage, recommendations, dependencies, and maintainer decisions before handoff. Preserve unrelated record entries and future work. `changedFiles` can be empty for an initial review. A recorded content snapshot detects edits even if the earlier review happened before a commit; it does not retain the earlier source text. Use an available Git diff or explain when historical text is unavailable. Missing sources, foreign repositories, conflicts, or unavailable history remain unknown until the evidence is resolved. -Plain `sources` paths are relative to the skill's nearest owning package, or the Git root when no package exists. `owner/repository:path` paths are relative to the Git root and must match the local repository's origin or root package repository metadata. Git glob syntax supports `*`, `?`, character classes and `**`; brace expansion and extglobs are unsupported. Keep evidence paths accurate. The command reviews Git-visible `SKILL.md` files under `skills/`, custom skill directories containing `_artifacts/`, and paths declared in an existing `skill_tree.yaml`. Previously recorded skills remain covered. Shared repository/package planning directories do not select every agent instruction in the package; list custom skill paths in the tree, using its `package` field for monorepo ownership. Installed dependencies under `node_modules` are excluded. Ignored files and external repositories still require explicit manual review. +Agent instruction files, generated plugin metadata, the generated CI workflow, `.intent/`, package manifests, and lockfiles are never reported as unmapped changes; a skill that lists one of them in `sources` still tracks it. Add repository-specific patterns under `review.ignore` in `skill_tree.yaml`. Plain `sources` paths are relative to the skill's nearest owning package, or the Git root when no package exists. `owner/repository:path` paths are relative to the Git root and must match the local repository's origin or root package repository metadata. Git glob syntax supports `*`, `?`, character classes and `**`; brace expansion and extglobs are unsupported. Keep evidence paths accurate. The command reviews Git-visible `SKILL.md` files under `skills/`, custom skill directories containing `_artifacts/`, and paths declared in an existing `skill_tree.yaml`. Previously recorded skills remain covered. Shared repository/package planning directories do not select every agent instruction in the package; list custom skill paths in the tree, using its `package` field for monorepo ownership. Installed dependencies under `node_modules` are excluded. Ignored files and external repositories still require explicit manual review. ## Record only completed reviews -After guidance edits and task checks, regenerate the JSON report. Save it outside tracked source paths, such as `.intent/review.json` after creating `.intent/`, so it cannot become its own review input. Include the `planning` item when the batch changes source or guidance, even when all three documents remain accurate. Annotate the selected items with: +After guidance edits and task checks, regenerate the JSON report. Save it outside tracked source paths, such as `.intent/review.json`, so it cannot become its own review input. The report's `recording` block lists the allowed outcomes and required fields. Include the `planning` item when the batch changes source or guidance, even when all three documents remain accurate. Annotate the selected items with: - `outcome`: `updated`, `no-change`, `out-of-scope`, or `unresolved`. - `reason`: the concrete behavior comparison and why that outcome follows. - `evidence`: source paths/revisions and actual check results. For behavior-changing guidance, include structural validation, executable task checks, and fresh-consumer evidence or its explicit limitation. -Preserve the report's identity, base and fingerprints. Run `intent maintainer review --record .intent/review.json`. For planning items, use `updated` or an evidence-backed `no-change` covering all three documents. The command rejects stale fingerprints and unresolved source mappings or planning files. It writes completed outcomes to `.intent/review-state.json`; unresolved or unannotated items stay pending. Do not invent passing checks, use a generic reason, or mark unrelated items complete just to empty the report. +Preserve the report's identity, base and fingerprints. Run `intent maintainer review --record .intent/review.json`. For planning items, use `updated` or an evidence-backed `no-change` covering all three documents. The command rejects a report that annotates nothing, stale fingerprints, and unresolved source mappings or planning files. It writes completed outcomes to `.intent/review-state.json`; unresolved or unannotated items stay pending. Do not invent passing checks, use a generic reason, or mark unrelated items complete just to empty the report. Keep the state file with the source/skill change for maintainer review. It contains content hashes, revisions, outcomes and evidence, not source contents. Record operations do not commit or publish. An identical content snapshot suppresses repeated reminders, including a justified no-op; another source or guidance change reopens review. This records an evidence-backed decision, not independent proof that the decision is correct. diff --git a/packages/intent/src/cli.ts b/packages/intent/src/cli.ts index a799d8df..8f48be28 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 ', @@ -238,6 +238,10 @@ function createCli( '--requires ', 'Prerequisite skill; repeat for multiple skills', ) + .option( + '--task ', + 'Developer task a new skill covers; repeat for multiple tasks', + ) .option('--base ', 'Git revision to review against') .option( '--interactive', @@ -255,6 +259,7 @@ function createCli( .example( 'maintainer add caching --domain queries --description "Use when caching queries." --source "src/**"', ) + .example('maintainer remove caching') .example('maintainer status --json') .example('maintainer sync') .example('maintainer review --json') diff --git a/packages/intent/src/commands/maintainer.ts b/packages/intent/src/commands/maintainer.ts index da365bcc..ed4302b0 100644 --- a/packages/intent/src/commands/maintainer.ts +++ b/packages/intent/src/commands/maintainer.ts @@ -9,6 +9,7 @@ import { setupRecords, } from '../maintainer/project.js' import { addSkill } from '../maintainer/add.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' @@ -75,6 +76,7 @@ const optionHelp: Record = { ], source: ['--source ', 'Source evidence path; repeat for more'], requires: ['--requires ', 'Prerequisite skill; repeat for more'], + task: ['--task ', 'Developer task the skill covers; repeat for more'], base: ['--base ', 'Git revision to review against'], interactive: ['--interactive', 'Inspect and record outcomes in a terminal'], json: ['--json', 'Print JSON instead of text'], @@ -121,8 +123,16 @@ export const maintainerActions: Record = { 'description', 'source', 'requires', + 'task', ].map((key) => optionHelp[key]!), }, + remove: { + usage: 'maintainer remove ', + summary: 'Retire a registered skill without deleting its guidance.', + writes: + 'The entry status in skill_tree.yaml and a note in skill_spec.md. Delete the SKILL.md yourself once its guidance is no longer needed.', + options: ['artifacts'].map((key) => optionHelp[key]!), + }, status: { usage: 'maintainer status [--json] [--base ]', summary: 'Report authoring gaps, files to sync, and pending reviews.', @@ -197,6 +207,7 @@ export interface MaintainerCommandOptions extends DistributionOptions { description?: string source?: string | Array requires?: string | Array + task?: string | Array base?: string json?: boolean record?: string @@ -236,7 +247,9 @@ export async function runMaintainerCommand( 'description', 'source', 'requires', + 'task', ], + remove: ['artifacts'], status: ['artifacts', 'base', 'json'], sync: ['artifacts'], review: ['base', 'json', 'record', 'interactive'], @@ -244,9 +257,9 @@ export async function runMaintainerCommand( } if (!allowed[action]) fail( - `Unknown maintainer action: ${action}. Expected setup, adopt, add, status, sync, review, or check.`, + `Unknown maintainer action: ${action}. Expected setup, adopt, add, remove, status, sync, review, or check.`, ) - if (name !== undefined && action !== 'add') + if (name !== undefined && action !== 'add' && action !== 'remove') fail(`maintainer ${action} does not take a skill name.`) for (const key of Object.keys(options)) { if (key !== '--' && !allowed[action].includes(key)) @@ -336,7 +349,7 @@ export async function runMaintainerCommand( }) return } - if (['setup', 'add', 'sync'].includes(action)) { + if (['setup', 'add', 'remove', 'sync'].includes(action)) { await withMaintainerLock(project.root, () => { if (action === 'setup') { const created = setupRecords(project) @@ -373,6 +386,15 @@ export async function runMaintainerCommand( console.log( `Next: author the guidance with intent meta generate-skill, record its developer tasks in ${project.artifacts}/domain_map.yaml, then run intent maintainer sync, intent maintainer review, and intent maintainer check.`, ) + } else if (action === 'remove') { + const retired = retireSkill(project, name) + console.log(`Retired ${name}.`) + console.log(`Updated: ${retired.files.join(', ')}`) + console.log( + retired.exists + ? `Delete ${retired.path} when its guidance is no longer needed, then run intent maintainer sync and intent maintainer review.` + : 'Run intent maintainer sync and intent maintainer review.', + ) } else { const plan = planMaintainerSync(project) writeChanges(project.root, plan.changes) diff --git a/packages/intent/src/commands/validate.ts b/packages/intent/src/commands/validate.ts index 8fd50030..a8f5ba04 100644 --- a/packages/intent/src/commands/validate.ts +++ b/packages/intent/src/commands/validate.ts @@ -91,14 +91,26 @@ function buildValidationFailure( return lines.join('\n') } -function filesEntryCovers(entry: string, directory: string): boolean { - if (entry.startsWith('!')) return false - const prefix = entry.replace(/\/(?:\*\*|\*)?$/, '') - return directory === prefix || directory.startsWith(`${prefix}/`) +// Positive `files` entries as directory prefixes: `skills`, `skills/`, and +// `skills/**` all publish the whole directory. +function filesPrefixes(files: ReadonlyArray): Array { + const prefixes: Array = [] + for (const entry of files) { + if (entry.startsWith('!')) continue + prefixes.push(entry.replace(/\/(?:\*\*|\*)?$/, '')) + } + return prefixes +} + +function covered(prefixes: ReadonlyArray, directory: string): boolean { + return prefixes.some( + (prefix) => directory === prefix || directory.startsWith(`${prefix}/`), + ) } function collectPackagingWarnings( context: ProjectContext, + skillsDir: string, skillFiles: ReadonlyArray, ): Array { if (!context.packageRoot || !context.targetPackageJsonPath) return [] @@ -144,20 +156,20 @@ function collectPackagingWarnings( const files = pkgJson.files as Array | undefined if (Array.isArray(files)) { const packageRoot = context.packageRoot - const skillDirs = [ - ...new Set( - skillFiles.map((file) => - relative(packageRoot, dirname(file)).replaceAll('\\', '/'), - ), - ), - ] + const prefixes = filesPrefixes(files) + const skillsRoot = relative(packageRoot, skillsDir).replaceAll('\\', '/') // Either the whole skills directory or each skill directory (as written // by `intent maintainer sync`) publishes the guidance. - for (const directory of skillDirs) { - if (!files.some((entry) => filesEntryCovers(entry, directory))) { - warnings.push( - `"${directory}" is not covered by the "files" array — this skill won't be published`, - ) + if (!covered(prefixes, skillsRoot)) { + const seen = new Set() + for (const file of skillFiles) { + const directory = `${skillsRoot}/${relative(skillsDir, dirname(file)).replaceAll('\\', '/')}` + if (seen.has(directory)) continue + seen.add(directory) + if (!covered(prefixes, directory)) + warnings.push( + `"${directory}" is not covered by the "files" array — this skill won't be published`, + ) } } @@ -165,9 +177,9 @@ function collectPackagingWarnings( // the negation pattern is a no-op and shouldn't be added. if ( !context.isMonorepo && - existsSync(join(packageRoot, 'skills', '_artifacts')) && - files.some((entry) => filesEntryCovers(entry, 'skills/_artifacts')) && - !files.includes('!skills/_artifacts') + covered(prefixes, 'skills/_artifacts') && + !files.includes('!skills/_artifacts') && + existsSync(join(packageRoot, 'skills', '_artifacts')) ) { warnings.push( '"!skills/_artifacts" is not in the "files" array — artifacts will be published unnecessarily', @@ -651,7 +663,9 @@ async function runValidateCommandInternal( } validatedCount += skillFiles.length - warnings.push(...collectPackagingWarnings(validateContext, skillFiles)) + warnings.push( + ...collectPackagingWarnings(validateContext, skillsDir, skillFiles), + ) } if (options.check) { diff --git a/packages/intent/src/maintainer/add.ts b/packages/intent/src/maintainer/add.ts index 236ef41d..d5c22ea0 100644 --- a/packages/intent/src/maintainer/add.ts +++ b/packages/intent/src/maintainer/add.ts @@ -23,6 +23,7 @@ export interface AddSkillOptions { description?: string source?: string | Array requires?: string | Array + task?: string | Array } export function stringList(value: unknown, label: string): Array { @@ -153,6 +154,10 @@ export function planAddSkills( entry.purpose = frontmatter.metadata.purpose 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)) { map.document.addIn(['skills'], { @@ -161,11 +166,11 @@ export function planAddSkills( domain: options.domain, description: entry.purpose ?? entry.description, ...(packageDir ? { packages: [manifest.name] } : {}), - tasks: [], + tasks, covers: [], }) } - nextSpec = `${nextSpec.trimEnd()}\n\n- Registered \`${name}\` in \`${packageDir ?? '.'}\` (domain \`${options.domain}\`). Task coverage, decisions, and checks still need to be recorded.\n` + 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)) } if (additions.length) { diff --git a/packages/intent/src/maintainer/remove.ts b/packages/intent/src/maintainer/remove.ts new file mode 100644 index 00000000..3e556349 --- /dev/null +++ b/packages/intent/src/maintainer/remove.ts @@ -0,0 +1,67 @@ +import { relative } from 'node:path' +import { existsSync, readFileSync } from 'node:fs' +import { readDistribution } from './distribution.js' +import { readRecord, recordPath, skillEntries, skillPath } from './project.js' +import { writeChanges } from './files.js' +import type { MaintainerProject } from './project.js' +import type { FileChange } from './files.js' + +// Retiring keeps the tree entry and the domain map history; only the status +// changes. The authored file stays on disk for the maintainer to delete. +export function retireSkill( + project: MaintainerProject, + name: string | undefined, +): { path: string; files: Array; exists: boolean } { + if (!name) + throw new Error('Name the skill to remove: maintainer remove .') + const tree = readRecord(project, 'skill_tree.yaml') + const entries = skillEntries(project, tree) + const index = entries.findIndex( + (entry) => (entry.slug ?? entry.name) === name, + ) + const entry = entries[index] + if (!entry) throw new Error(`Skill ${name} is not registered.`) + if (entry.status === 'retired') + throw new Error(`Skill ${name} is already retired.`) + const distribution = readDistribution(tree) + if (distribution?.mode === 'repo' && distribution.skills?.includes(name)) + throw new Error( + `Skill ${name} is selected for repository distribution. Reselect the remaining skills with intent maintainer setup --distribution repo --skill , or opt out with --distribution none, before removing it.`, + ) + const dependents = entries.filter( + (other) => + other !== entry && + !['planned', 'retired'].includes(String(other.status)) && + Array.isArray(other.requires) && + other.requires.includes(name), + ) + if (dependents.length) + throw new Error( + `Skill ${name} is required by ${dependents + .map((other) => other.slug ?? other.name) + .join(', ')}. Update those prerequisites before removing it.`, + ) + tree.document.setIn(['skills', index, 'status'], 'retired') + const specPath = recordPath(project, 'skill_spec.md') + const spec = existsSync(specPath) ? readFileSync(specPath, 'utf8') : null + if (spec === null) + throw new Error('Missing skill_spec.md. Run intent maintainer setup.') + const path = skillPath(project, entry) + const relativePath = relative(project.root, path).replaceAll('\\', '/') + const changes: Array = [ + { path: tree.path, source: tree.source, content: tree.document.toString() }, + { + path: specPath, + source: spec, + content: `${spec.trimEnd()}\n\n- Retired \`${name}\` (${relativePath}). Record why its guidance is no longer needed.\n`, + }, + ] + writeChanges(project.root, changes) + return { + path: relativePath, + files: changes.map((change) => + relative(project.root, change.path).replaceAll('\\', '/'), + ), + exists: existsSync(path), + } +} diff --git a/packages/intent/src/review/review.ts b/packages/intent/src/review/review.ts index e307a0a1..8216c404 100644 --- a/packages/intent/src/review/review.ts +++ b/packages/intent/src/review/review.ts @@ -37,9 +37,24 @@ export interface ReviewReport { root: string head: string base: string + recording: { + outcomes: Array> + planningOutcomes: Array> + required: ['outcome', 'reason', 'evidence'] + command: string + } items: Array } +// Tells an agent reading the JSON what a completed item needs, so the +// vocabulary does not live only in the packaged authoring procedure. +const recording: ReviewReport['recording'] = { + outcomes: ['updated', 'no-change', 'out-of-scope'], + planningOutcomes: ['updated', 'no-change'], + required: ['outcome', 'reason', 'evidence'], + command: 'intent maintainer review --record ', +} + interface ReviewRecord { fingerprint: string snapshot: Snapshot @@ -494,7 +509,12 @@ export function createReview(cwd: string, baseRef?: string): ReviewReport { } } } - const ignored = new Set([...list(ignorePatterns), ...diff(ignorePatterns)]) + // Query Git for ignored paths only when an uncovered change needs classifying. + let ignored: Set | undefined + const isIgnored = (path: string) => { + ignored ??= new Set([...list(ignorePatterns), ...diff(ignorePatterns)]) + return ignored.has(path) + } const skillFiles = files.filter( (path) => basename(path) === 'SKILL.md' && @@ -635,7 +655,7 @@ export function createReview(cwd: string, baseRef?: string): ReviewReport { } } for (const path of changed) { - if (covered.has(path) || ignored.has(path)) continue + if (covered.has(path) || isIgnored(path)) continue add('source', path, [path], []) } for (const id of Object.keys(state?.items ?? {})) { @@ -646,7 +666,7 @@ export function createReview(cwd: string, baseRef?: string): ReviewReport { add('source', path, [path], []) } } - return { schemaVersion: 1, root, head, base, items } + return { schemaVersion: 1, root, head, base, recording, items } } export function recordReview(cwd: string, input: unknown): number { diff --git a/packages/intent/tests/maintainer.test.ts b/packages/intent/tests/maintainer.test.ts index b9fb6faa..d85e1c3d 100644 --- a/packages/intent/tests/maintainer.test.ts +++ b/packages/intent/tests/maintainer.test.ts @@ -768,3 +768,107 @@ it('validates each skills root once during check', async () => { .filter((line) => line.includes('Validated 2 skill files')), ).toHaveLength(1) }) + +it('records developer tasks at registration and retires a skill without deleting it', async () => { + const logs = () => vi.mocked(console.log).mock.calls.flat().map(String) + const errors = () => vi.mocked(console.error).mock.calls.flat().map(String) + write('src/query.ts', 'export const query = () => 1\n') + expect(await main(['maintainer', 'setup', '--distribution', 'none'])).toBe(0) + expect( + await main([ + 'maintainer', + 'add', + 'query', + '--domain', + 'queries', + '--description', + 'Use when querying with Library.', + '--source', + 'src/query.ts', + '--task', + 'Read the current value', + '--task', + 'Subscribe to changes', + ]), + ).toBe(0) + expect( + parse(read('skills/_artifacts/domain_map.yaml')).skills[0].tasks, + ).toEqual(['Read the current value', 'Subscribe to changes']) + expect(read('skills/_artifacts/skill_spec.md')).toContain( + 'Developer tasks: Read the current value; Subscribe to changes.', + ) + vi.mocked(console.log).mockClear() + expect(await main(['maintainer', 'status'])).toBe(0) + expect(logs().join('\n')).not.toContain('record the assessed developer tasks') + + expect( + await main([ + 'maintainer', + 'add', + 'dependent', + '--domain', + 'queries', + '--description', + 'Use after query.', + '--source', + 'src/query.ts', + '--requires', + 'query', + ]), + ).toBe(0) + expect(await main(['maintainer', 'remove', 'query'])).toBe(1) + expect(errors().at(-1)).toContain('is required by dependent') + expect(await main(['maintainer', 'remove', 'missing'])).toBe(1) + expect(errors().at(-1)).toBe('Skill missing is not registered.') + + vi.mocked(console.log).mockClear() + expect(await main(['maintainer', 'remove', 'dependent'])).toBe(0) + expect(logs()).toEqual([ + 'Retired dependent.', + 'Updated: skills/_artifacts/skill_tree.yaml, skills/_artifacts/skill_spec.md', + expect.stringContaining('Delete skills/dependent/SKILL.md'), + ]) + const tree = parse(read('skills/_artifacts/skill_tree.yaml')) + expect(tree.skills[1]).toMatchObject({ name: 'dependent', status: 'retired' }) + expect(existsSync(join(root, 'skills/dependent/SKILL.md'))).toBe(true) + expect(read('skills/_artifacts/skill_spec.md')).toContain( + 'Retired `dependent`', + ) + expect(await main(['maintainer', 'remove', 'dependent'])).toBe(1) + expect(errors().at(-1)).toBe('Skill dependent is already retired.') + expect(await main(['maintainer', 'sync'])).toBe(0) + expect(JSON.parse(read('package.json')).files).toBeUndefined() +}) + +it('refuses to retire a skill selected for repository distribution', async () => { + write('src/query.ts', 'export const query = () => 1\n') + write( + 'package.json', + '{"name":"library","version":"1.0.0","repository":"https://github.com/acme/library"}\n', + ) + expect(await main(['maintainer', 'setup'])).toBe(0) + write( + 'skills/query/SKILL.md', + '---\nname: query\ndescription: Use when querying with Library.\nsources: [src/query.ts]\n---\nCall query().\n', + ) + expect( + await main(['maintainer', 'add', 'query', '--domain', 'queries']), + ).toBe(0) + expect( + await main([ + 'maintainer', + 'setup', + '--distribution', + 'repo', + '--skill', + 'query', + ]), + ).toBe(0) + expect(await main(['maintainer', 'remove', 'query'])).toBe(1) + expect( + vi.mocked(console.error).mock.calls.flat().map(String).at(-1), + ).toContain('selected for repository distribution') + expect( + parse(read('skills/_artifacts/skill_tree.yaml')).skills[0].status, + ).toBeUndefined() +}) diff --git a/packages/intent/tests/review.test.ts b/packages/intent/tests/review.test.ts index 42169646..813abe0c 100644 --- a/packages/intent/tests/review.test.ts +++ b/packages/intent/tests/review.test.ts @@ -743,3 +743,12 @@ it('rejects a review record that annotates nothing instead of silently recording expect(existsSync(join(root, '.intent/review-state.json'))).toBe(false) errorSpy.mockRestore() }) + +it('tells report readers which outcomes and fields a recording needs', () => { + expect(createReview(root).recording).toEqual({ + outcomes: ['updated', 'no-change', 'out-of-scope'], + planningOutcomes: ['updated', 'no-change'], + required: ['outcome', 'reason', 'evidence'], + command: 'intent maintainer review --record ', + }) +})