From 2c2b74ac8d141666941e024d3baf08dfabebabd1 Mon Sep 17 00:00:00 2001 From: Mohamed Khaled Date: Tue, 28 Jul 2026 12:39:53 +0300 Subject: [PATCH] fix(build): look up example skip patterns by directory name Keys in the examples: block of skip-patterns.yaml are example directory names, but a skill with a single example path looked its overrides up by skill.id. The lookup missed, mergeSkipPatterns read it as no overrides, and the build carried on with no error, so rules that were written on purpose never ran. Extract the lookup so it always keys on the directory name and can be tested. isSingle still selects the output filename and display title, which the build generates; config keys are typed by hand and are always directories. Rebuilt: integration-laravel drops 20,896 bytes (bootstrap/cache/services.php, Laravel's generated package cache) and integration-android drops 16,141. integration-swift already took the dirName branch and is unchanged. Fixes #284 --- scripts/lib/example-processor.js | 9 ++++++ scripts/lib/skill-generator.js | 4 +-- scripts/lib/tests/skip-patterns.test.js | 41 ++++++++++++++++++++++++- 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/scripts/lib/example-processor.js b/scripts/lib/example-processor.js index adbf6a65..619c5e70 100644 --- a/scripts/lib/example-processor.js +++ b/scripts/lib/example-processor.js @@ -74,6 +74,14 @@ function mergeSkipPatterns(globalPatterns, examplePatterns = {}) { }; } +/** + * Global patterns plus any override for this example. The `examples:` block in + * skip-patterns.yaml is keyed by directory name, not skill id. + */ +function skipPatternsForExample(skipPatterns, dirName) { + return mergeSkipPatterns(skipPatterns.global, skipPatterns.examples?.[dirName]); +} + /** * Recursively collect all files in a directory */ @@ -199,6 +207,7 @@ const defaultPlugins = [ignoreFilePlugin, ignoreBlockPlugin, ignoreLinePlugin]; export { loadSkipPatterns, mergeSkipPatterns, + skipPatternsForExample, shouldSkip, processExample, defaultPlugins, diff --git a/scripts/lib/skill-generator.js b/scripts/lib/skill-generator.js index 71dd2357..21cae1ad 100644 --- a/scripts/lib/skill-generator.js +++ b/scripts/lib/skill-generator.js @@ -41,7 +41,7 @@ import path from 'path'; import crypto from 'crypto'; import yaml from 'js-yaml'; import matter from 'gray-matter'; -import { processExample, loadSkipPatterns, mergeSkipPatterns, defaultPlugins } from './example-processor.js'; +import { processExample, loadSkipPatterns, skipPatternsForExample, defaultPlugins } from './example-processor.js'; import { CLI_ROLES, validateCommandName } from './cli-block-validation.js'; /** @@ -641,7 +641,7 @@ async function generateSkill({ displayName: isSingle ? skill.display_name : dirName, id: skill.id, repoRoot, - skipPatterns: mergeSkipPatterns(skipPatterns.global, skipPatterns.examples[isSingle ? skill.id : dirName]), + skipPatterns: skipPatternsForExample(skipPatterns, dirName), plugins: defaultPlugins, }); diff --git a/scripts/lib/tests/skip-patterns.test.js b/scripts/lib/tests/skip-patterns.test.js index 55a02869..882adb76 100644 --- a/scripts/lib/tests/skip-patterns.test.js +++ b/scripts/lib/tests/skip-patterns.test.js @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { mergeSkipPatterns, shouldSkip } from '../example-processor.js'; +import { mergeSkipPatterns, shouldSkip, skipPatternsForExample } from '../example-processor.js'; const globalPatterns = { includes: ['.yml', '.gitignore', 'node_modules'], @@ -42,6 +42,45 @@ describe('shouldSkip', () => { }); }); +describe('skipPatternsForExample', () => { + const config = { + global: globalPatterns, + examples: { + laravel: { includes: ['bootstrap/cache'] }, + 'swift-xcodegen': { allow: ['project.yml'] }, + }, + }; + + it('finds an example override by directory name', () => { + const patterns = skipPatternsForExample(config, 'laravel'); + expect(shouldSkip('bootstrap/cache/services.php', patterns)).toBe(true); + }); + + // The lookup key is the example directory, never the skill id — a skill + // with a single example used to be looked up as `integration-laravel`, + // silently missing every override written for `laravel`. + it('does not look overrides up by skill id', () => { + const patterns = skipPatternsForExample(config, 'integration-laravel'); + expect(shouldSkip('bootstrap/cache/services.php', patterns)).toBe(false); + }); + + it('applies allow overrides by directory name', () => { + const patterns = skipPatternsForExample(config, 'swift-xcodegen'); + expect(shouldSkip('project.yml', patterns)).toBe(false); + }); + + it('falls back to global patterns for an example with no overrides', () => { + const patterns = skipPatternsForExample(config, 'php'); + expect(shouldSkip('node_modules/foo.js', patterns)).toBe(true); + expect(shouldSkip('bootstrap/cache/services.php', patterns)).toBe(false); + }); + + it('tolerates a config with no examples block', () => { + const patterns = skipPatternsForExample({ global: globalPatterns }, 'laravel'); + expect(shouldSkip('node_modules/foo.js', patterns)).toBe(true); + }); +}); + describe('mergeSkipPatterns', () => { it('concatenates example includes and allow onto global patterns', () => { const merged = mergeSkipPatterns(globalPatterns, {