From fc3026efd3770b09bb875bb9b21f1acfb4eeec92 Mon Sep 17 00:00:00 2001 From: Abhishek Sharma Date: Thu, 24 Sep 2026 18:31:59 -0700 Subject: [PATCH] fix(skill-lint): strip every negated trigger clause, not just the first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The negated-trigger check removed only the first "Do not use when …" clause before testing whether a positive trigger remained, because the replace used the non-global DESCRIPTION_TRIGGER_NEGATE. A description carrying two or more negated clauses and no positive trigger left a second "use when" substring behind, which DESCRIPTION_TRIGGER then matched, so the skill passed validation with no trigger at all. One negated clause was correctly rejected; two were accepted. Adds a global form of the pattern for the strip and keeps the non-global one for .test(), since a /g regex carries lastIndex across test() calls. --- scripts/lib/skill-lint-test.js | 14 ++++++++++++++ scripts/lib/skill-lint.js | 8 +++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/scripts/lib/skill-lint-test.js b/scripts/lib/skill-lint-test.js index d2ebf936..c06c2548 100644 --- a/scripts/lib/skill-lint-test.js +++ b/scripts/lib/skill-lint-test.js @@ -100,6 +100,20 @@ test('reports a description with no trigger clause', () => { assert.match(errors[0], /no 'when to use' trigger/); }); +test('a description whose only triggers are negated is rejected regardless of how many there are', () => { + const content = withAllSections( + [ + '---', + 'name: alpha', + 'description: Designs alphas. Do not use when building betas. Never use when the input is JSON.', + '---', + ].join('\n') + ); + const { errors } = lintSkillContent('alpha', content, KNOWN); + assert.equal(errors.length, 1); + assert.match(errors[0], /no 'when to use' trigger/); +}); + test('reports frontmatter name that disagrees with the directory', () => { const content = withAllSections( ['---', 'name: beta', 'description: Designs alphas. Use when building one.', '---'].join('\n') diff --git a/scripts/lib/skill-lint.js b/scripts/lib/skill-lint.js index be4a455d..1b2e3667 100644 --- a/scripts/lib/skill-lint.js +++ b/scripts/lib/skill-lint.js @@ -38,6 +38,12 @@ const KEBAB_CASE = /^[a-z0-9]+(-[a-z0-9]+)*$/; // describe exclusions, not trigger conditions. const DESCRIPTION_TRIGGER = /\buse (this )?when\b|\buse (before|after|during)\b/i; const DESCRIPTION_TRIGGER_NEGATE = /\b(do not|don't|never) use (this )?(when|before|after|during)\b/i; +// Same pattern, global — used to strip *every* negated clause before checking +// whether a positive trigger remains. Without /g only the first is removed, so +// a description carrying two or more negated clauses and no positive one would +// still match DESCRIPTION_TRIGGER on the leftovers and pass. Kept separate from +// the non-global form above because a /g regex carries lastIndex across .test(). +const DESCRIPTION_TRIGGER_NEGATE_ALL = new RegExp(DESCRIPTION_TRIGGER_NEGATE.source, 'gi'); // Sections every standard SKILL.md must contain. // Each entry is an array of acceptable heading strings — the first @@ -287,7 +293,7 @@ function lintSkillContent(dirName, content, knownSkills) { } const hasTrigger = DESCRIPTION_TRIGGER.test(fm.description); const onlyNegated = hasTrigger && DESCRIPTION_TRIGGER_NEGATE.test(fm.description) - && !fm.description.replace(DESCRIPTION_TRIGGER_NEGATE, '').match(DESCRIPTION_TRIGGER); + && !fm.description.replace(DESCRIPTION_TRIGGER_NEGATE_ALL, '').match(DESCRIPTION_TRIGGER); if (!hasTrigger || onlyNegated) { errors.push( `Description has no 'when to use' trigger — add a "Use when …" clause ` +