From 92a425a849f9eca67538ac92aea2e4fed4576d29 Mon Sep 17 00:00:00 2001 From: viclafouch Date: Mon, 5 Oct 2026 16:35:57 +0200 Subject: [PATCH 1/3] fix: leave skills that review.ignore matches out of default discovery --- .changeset/review-ignore-skill-discovery.md | 5 +++ docs/cli/intent-maintainer.md | 2 +- docs/cli/intent-review.md | 4 +- packages/intent/src/maintainer/existing.ts | 43 ++++++++++++++------- packages/intent/src/review/review.ts | 24 +++++++++--- packages/intent/tests/maintainer.test.ts | 21 ++++++++++ packages/intent/tests/review.test.ts | 30 ++++++++++++++ 7 files changed, 106 insertions(+), 23 deletions(-) create mode 100644 .changeset/review-ignore-skill-discovery.md diff --git a/.changeset/review-ignore-skill-discovery.md b/.changeset/review-ignore-skill-discovery.md new file mode 100644 index 0000000..5322600 --- /dev/null +++ b/.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//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. diff --git a/docs/cli/intent-maintainer.md b/docs/cli/intent-maintainer.md index 62e01c2..c05e48e 100644 --- a/docs/cli/intent-maintainer.md +++ b/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, paths that [`review.ignore`](./intent-review#ignored-paths) matches, 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. ## Add a skill diff --git a/docs/cli/intent-review.md b/docs/cli/intent-review.md index 26c578e..4ba1bbb 100644 --- a/docs/cli/intent-review.md +++ b/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: review and `maintainer setup` skip it unless the skill tree declares its path. 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: diff --git a/packages/intent/src/maintainer/existing.ts b/packages/intent/src/maintainer/existing.ts index 407b0fb..a462352 100644 --- a/packages/intent/src/maintainer/existing.ts +++ b/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 = [], ): 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 listFiles = (patterns: Array = []) => + 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() diff --git a/packages/intent/src/review/review.ts b/packages/intent/src/review/review.ts index 218a512..7292d0e 100644 --- a/packages/intent/src/review/review.ts +++ b/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, path: string) { +export function reviewIgnorePatterns( + tree: Record, + 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() - const ignorePatterns = defaultReviewIgnore.map((pattern) => - globPattern(pattern, pattern, 'review.ignore'), - ) + const treeIgnorePatterns: Array = [] 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,16 +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 | 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)) || + ((/(^|\/)skills\//.test(path) && + isDefaultSkillPath(path) && + !treeIgnored.has(path)) || customRoots.some((dir) => path.startsWith(`${dir}/`)) || declaredSkills.has(path) || state?.items[`skill:${path}`]), diff --git a/packages/intent/tests/maintainer.test.ts b/packages/intent/tests/maintainer.test.ts index 762d587..5bbfaab 100644 --- a/packages/intent/tests/maintainer.test.ts +++ b/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) => [ diff --git a/packages/intent/tests/review.test.ts b/packages/intent/tests/review.test.ts index 4c177b0..c2f7912 100644 --- a/packages/intent/tests/review.test.ts +++ b/packages/intent/tests/review.test.ts @@ -168,6 +168,36 @@ 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('retains a hidden skill through review state without explicit declaration or custom root', () => { const skillPath = '.agents/skills/hidden/SKILL.md' From c47b7229e899613ea712b90d13ae1c9c9f3bfcce Mon Sep 17 00:00:00 2001 From: viclafouch Date: Mon, 5 Oct 2026 17:15:48 +0200 Subject: [PATCH 2/3] docs: state every exception to ignored-skill discovery --- docs/cli/intent-review.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/cli/intent-review.md b/docs/cli/intent-review.md index 4ba1bbb..603558e 100644 --- a/docs/cli/intent-review.md +++ b/docs/cli/intent-review.md @@ -129,7 +129,7 @@ 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: review and `maintainer setup` skip it unless the skill tree declares its path. Use this for agent skills kept beside the library, such as the skills of a Claude Code plugin. +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, a custom root holds it, 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 From a724d516902806f58e32a49177750b8094a228f9 Mon Sep 17 00:00:00 2001 From: viclafouch Date: Tue, 6 Oct 2026 09:23:02 +0200 Subject: [PATCH 3/3] fix: apply review.ignore to custom-root skill discovery --- docs/cli/intent-maintainer.md | 2 +- docs/cli/intent-review.md | 2 +- packages/intent/src/review/review.ts | 7 +++---- packages/intent/tests/review.test.ts | 12 ++++++++++++ 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/docs/cli/intent-maintainer.md b/docs/cli/intent-maintainer.md index c05e48e..44e1c3e 100644 --- a/docs/cli/intent-maintainer.md +++ b/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, paths that [`review.ignore`](./intent-review#ignored-paths) matches, 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 diff --git a/docs/cli/intent-review.md b/docs/cli/intent-review.md index 603558e..ab3db2e 100644 --- a/docs/cli/intent-review.md +++ b/docs/cli/intent-review.md @@ -129,7 +129,7 @@ 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, a custom root holds it, 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. +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 diff --git a/packages/intent/src/review/review.ts b/packages/intent/src/review/review.ts index 7292d0e..7d4a91a 100644 --- a/packages/intent/src/review/review.ts +++ b/packages/intent/src/review/review.ts @@ -575,10 +575,9 @@ export function createReview(cwd: string, baseRef?: string): ReviewReport { const skillFiles = files.filter( (path) => basename(path) === 'SKILL.md' && - ((/(^|\/)skills\//.test(path) && - isDefaultSkillPath(path) && - !treeIgnored.has(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}`]), ) diff --git a/packages/intent/tests/review.test.ts b/packages/intent/tests/review.test.ts index c2f7912..c4c90e9 100644 --- a/packages/intent/tests/review.test.ts +++ b/packages/intent/tests/review.test.ts @@ -198,6 +198,18 @@ it('leaves skills that review.ignore matches out of default skill discovery', () `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'