fix(skill-lint): strip every negated trigger clause, not just the first

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.
This commit is contained in:
Abhishek Sharma committed 2026-09-24 18:31:59 -07:00
1 parent bcab6a1b85
commit fc3026efd3
2 files changed
+21 -1

No files matched your search

+14
View File
@@ -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')
+7 -1
View File
@@ -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 ` +