fix: leave skills that review.ignore matches out of default discovery

.changeset/review-ignore-skill-discovery.md

@@ -0,0 +1,5 @@
+---
+'@tanstack/intent': patch
+---
+
+Leave a `SKILL.md` that `review.ignore` matches out of the skills that `review` and `maintainer setup` discover. A Claude Code plugin kept beside the library, such as `plugins/<name>/skills/`, was reviewed as a library skill and reported `No source paths declared`, and `maintainer setup` registered it. A skill that the tree declares is still reviewed.

docs/cli/intent-maintainer.md

@@ -43,7 +43,7 @@ The three records have separate jobs:
 
 Generated skeletons remain unfinished. Author their contents and remove the `intent:needs-authoring` marker after completing that work. A successful setup command does not mean the skills are ready to publish.
 
-Setup automatically registers valid, Git-visible `skills/**/SKILL.md` files in the root package and workspace packages, preserving their content. It skips dependencies, hidden agent directories, invalid skills, and conflicting names, reporting each skipped candidate. Domains come from `metadata.domain`, the existing domain map, a parent directory under `skills/`, or `uncategorized`; review that placeholder and complete task coverage. For custom locations outside `skills/`, use `maintainer add --path`. Repeating setup preserves existing registrations and workflow files. Reviewers can use [interactive review](./intent-review#interactive-review) in a human terminal; CI uses the noninteractive checks.
+Setup automatically registers valid, Git-visible `skills/**/SKILL.md` files in the root package and workspace packages, preserving their content. It skips dependencies, hidden agent directories, and paths that [`review.ignore`](./intent-review#ignored-paths) matches. It also skips invalid skills and conflicting names, and reports each one. Domains come from `metadata.domain`, the existing domain map, a parent directory under `skills/`, or `uncategorized`; review that placeholder and complete task coverage. For custom locations outside `skills/`, use `maintainer add --path`. Repeating setup preserves existing registrations and workflow files. Reviewers can use [interactive review](./intent-review#interactive-review) in a human terminal; CI uses the noninteractive checks.
 
 ## Add a skill
 

docs/cli/intent-review.md

@@ -102,7 +102,7 @@ When the previous stored baseline is unavailable, recording this fully resolved
 
 ### Source mappings
 
-Discovery covers first-party `skills/**/SKILL.md` files in the repository and its packages, custom roots containing `_artifacts/`, exact paths declared in the skill tree, and previously reviewed skill paths. An unrelated `SKILL.md` elsewhere does not automatically become a library skill. Review excludes `node_modules`, even without a Git ignore rule.
+Discovery covers first-party `skills/**/SKILL.md` files in the repository and its packages that [`review.ignore`](#ignored-paths) does not match, custom roots containing `_artifacts/`, exact paths declared in the skill tree, and previously reviewed skill paths. An unrelated `SKILL.md` elsewhere does not automatically become a library skill. Review excludes `node_modules`, even without a Git ignore rule.
 
 | Source entry | Resolution |
 | --- | --- |
@@ -129,6 +129,8 @@ review:
 
 Entries use the same Git glob syntax as source mappings. An entry that is not a non-empty string fails review with the path of the tree file.
 
+A `skills/**/SKILL.md` that matches `review.ignore` is not a library skill: `maintainer setup` does not register it, and review skips it unless the skill tree declares its path or the review state already records it. Use this for agent skills kept beside the library, such as the skills of a Claude Code plugin.
+
 ### Required planning documents
 
 The maintainer workflow keeps a cumulative record across batches:

packages/intent/src/maintainer/existing.ts

@@ -2,6 +2,7 @@ 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 { reviewIgnorePatterns } from '../review/review.js'
 import { resolveWorkspacePackages } from '../setup/workspace-patterns.js'
 import { isDefaultSkillPath, parseFrontmatter } from '../shared/utils.js'
 import { stringList } from './add.js'
@@ -35,22 +36,33 @@ export function findExistingSkills(
   project: MaintainerProject,
   changes: ReadonlyArray<FileChange> = [],
 ): Array<ExistingSkill> {
-  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 listFiles = (patterns: Array<string> = []) =>
+    execFileSync(
+      'git',
+      [
+        '-c',
+        'core.fsmonitor=false',
+        'ls-files',
+        '--cached',
+        '--others',
+        '--exclude-standard',
+        '-z',
+        '--',
+        ...patterns,
+      ],
+      { cwd: project.root, encoding: 'utf8', maxBuffer: 32 * 1024 * 1024 },
+    )
+      .split('\0')
+      .filter(Boolean)
+  const files = listFiles()
   const tree = readRecord(project, 'skill_tree.yaml', changes)
+  const ignorePatterns = reviewIgnorePatterns(
+    tree.document.toJS(),
+    relative(project.root, tree.path).replaceAll('\\', '/'),
+  )
+  const ignored = new Set(
+    ignorePatterns.length > 0 ? listFiles(ignorePatterns) : [],
+  )
   const entries = skillEntries(project, tree)
   const registered = new Set(
     entries.map((entry) =>
@@ -80,6 +92,7 @@ export function findExistingSkills(
         basename(path) === 'SKILL.md' &&
         /(^|\/)skills\//.test(path) &&
         isDefaultSkillPath(path) &&
+        !ignored.has(path) &&
         !registered.has(path),
     )
     .sort()

packages/intent/src/review/review.ts

@@ -362,7 +362,10 @@ function globPattern(path: string, label: string, kind: string): string {
   return `:(top,glob)${path}`
 }
 
-function reviewIgnorePatterns(tree: Record<string, unknown>, path: string) {
+export function reviewIgnorePatterns(
+  tree: Record<string, unknown>,
+  path: string,
+) {
   if (tree.review === undefined) return []
   const ignore = isObject(tree.review) ? tree.review.ignore : undefined
   if (
@@ -531,9 +534,7 @@ export function createReview(cwd: string, baseRef?: string): ReviewReport {
     .map((dir) => dirname(dir))
     .filter((dir) => dir !== '.' && !files.includes(`${dir}/package.json`))
   const declaredSkills = new Set<string>()
-  const ignorePatterns = defaultReviewIgnore.map((pattern) =>
-    globPattern(pattern, pattern, 'review.ignore'),
-  )
+  const treeIgnorePatterns: Array<string> = []
   for (const dir of existingArtifactDirs) {
     const treePath = join(dir, 'skill_tree.yaml').replaceAll('\\', '/')
     let tree: unknown
@@ -544,7 +545,7 @@ export function createReview(cwd: string, baseRef?: string): ReviewReport {
       continue
     }
     if (!isObject(tree)) continue
-    ignorePatterns.push(...reviewIgnorePatterns(tree, treePath))
+    treeIgnorePatterns.push(...reviewIgnorePatterns(tree, treePath))
     if (Array.isArray(tree.skills)) {
       for (const entry of tree.skills) {
         if (!isObject(entry) || typeof entry.path !== 'string') continue
@@ -556,17 +557,27 @@ export function createReview(cwd: string, baseRef?: string): ReviewReport {
       }
     }
   }
+  const ignorePatterns = [
+    ...defaultReviewIgnore.map((pattern) =>
+      globPattern(pattern, pattern, 'review.ignore'),
+    ),
+    ...treeIgnorePatterns,
+  ]
   // Query Git for ignored paths only when an uncovered change needs classifying.
   let ignored: Set<string> | undefined
   const isIgnored = (path: string) => {
     ignored ??= new Set([...list(ignorePatterns), ...diff(ignorePatterns)])
     return ignored.has(path)
   }
+  const treeIgnored = new Set(
+    treeIgnorePatterns.length > 0 ? list(treeIgnorePatterns) : [],
+  )
   const skillFiles = files.filter(
     (path) =>
       basename(path) === 'SKILL.md' &&
-      ((/(^|\/)skills\//.test(path) && isDefaultSkillPath(path)) ||
-        customRoots.some((dir) => path.startsWith(`${dir}/`)) ||
+      ((!treeIgnored.has(path) &&
+        ((/(^|\/)skills\//.test(path) && isDefaultSkillPath(path)) ||
+          customRoots.some((dir) => path.startsWith(`${dir}/`)))) ||
         declaredSkills.has(path) ||
         state?.items[`skill:${path}`]),
   )

packages/intent/tests/maintainer.test.ts

@@ -655,6 +655,27 @@ it('reports invalid and conflicting existing skills without registering them', a
   expect(output).not.toContain('skills/query/SKILL.md: Another skill')
 })
 
+it('does not register existing skills that review.ignore matches', async () => {
+  write(
+    'skills/query/SKILL.md',
+    '---\nname: query\ndescription: Query\n---\nGuidance.\n',
+  )
+  write(
+    'plugins/review/skills/review-code/SKILL.md',
+    '---\nname: review-code\ndescription: Review code\n---\nGuidance.\n',
+  )
+  write(
+    'skills/_artifacts/skill_tree.yaml',
+    'library: { name: library }\nreview:\n  ignore: [plugins/**]\nskills: []\n',
+  )
+  expect(await main(['maintainer', 'setup'])).toBe(0)
+  expect(
+    parse(read('skills/_artifacts/skill_tree.yaml')).skills.map(
+      (entry: { name: string }) => entry.name,
+    ),
+  ).toEqual(['query'])
+})
+
 it('keeps valid registrations when the planner rejects a candidate in the batch', async () => {
   const contents = new Map(
     ['alpha', 'broken', 'omega'].map((name) => [

packages/intent/tests/review.test.ts

@@ -168,6 +168,48 @@ it('ignores hidden agent directories during default skill discovery', () => {
     'skill:skills/request/SKILL.md',
   ])
 })
+it('leaves skills that review.ignore matches out of default skill discovery', () => {
+  const pluginSkill = 'plugins/review/skills/review-code/SKILL.md'
+  write('plugins/review/.claude-plugin/plugin.json', '{"name":"review"}\n')
+  write(
+    pluginSkill,
+    '---\nname: review-code\ndescription: Review code\n---\nReview guidance.\n',
+  )
+  planningRecords('_artifacts')
+  git('add', '.')
+  git('commit', '-qm', 'plugin skill')
+  expect(createReview(root).items.map((item) => item.id)).toContain(
+    `skill:${pluginSkill}`,
+  )
+
+  write(
+    '_artifacts/skill_tree.yaml',
+    'library: { name: library }\nreview:\n  ignore: [plugins/**]\nskills: []\n',
+  )
+  expect(createReview(root).items.map((item) => item.id)).not.toContain(
+    `skill:${pluginSkill}`,
+  )
+
+  write(
+    '_artifacts/skill_tree.yaml',
+    `library: { name: library }\nreview:\n  ignore: [plugins/**]\nskills: [{path: ${pluginSkill}}]\n`,
+  )
+  expect(createReview(root).items.map((item) => item.id)).toContain(
+    `skill:${pluginSkill}`,
+  )
+})
+it('leaves an ignored skill under the root skills directory out of review', () => {
+  planningRecords()
+  write(
+    'skills/_artifacts/skill_tree.yaml',
+    "library: { name: library }\nreview:\n  ignore: ['**/*.md']\nskills: []\n",
+  )
+  git('add', '.')
+  git('commit', '-qm', 'ignore markdown')
+  expect(createReview(root).items.map((item) => item.id)).not.toContain(
+    'skill:skills/request/SKILL.md',
+  )
+})
 it('retains a hidden skill through review state without explicit declaration or custom root', () => {
   const skillPath = '.agents/skills/hidden/SKILL.md'