From 5a64da32d336b015713082f139511af50d059987 Mon Sep 17 00:00:00 2001 From: mastertyko <11311479+mastertyko@users.noreply.github.com> Date: Mon, 23 Mar 2026 20:17:01 +0100 Subject: [PATCH] review: clarify comment wording, add special-character test Address review feedback: - Update comment to clarify that the function-call-like syntax led LLMs to infer a positional parameter name (not 'positional-looking') - Add test documenting current behavior when skill names contain special characters (quotes, apostrophes) --- src/resources/extensions/gsd/auto-prompts.ts | 3 ++- .../gsd/tests/skill-activation.test.ts | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/resources/extensions/gsd/auto-prompts.ts b/src/resources/extensions/gsd/auto-prompts.ts index 102aebb63..f06bca4da 100644 --- a/src/resources/extensions/gsd/auto-prompts.ts +++ b/src/resources/extensions/gsd/auto-prompts.ts @@ -422,7 +422,8 @@ function resolvePreferredSkillNames( function formatSkillActivationBlock(skillNames: string[]): string { if (skillNames.length === 0) return ""; // Use explicit parameter syntax so LLMs pass { skill: "..." } instead of { name: "..." }. - // Positional-looking `Skill('name')` caused validation failures — see #2224. + // The function-call-like syntax `Skill('name')` led LLMs to infer a positional + // parameter name, causing tool validation failures — see #2224. const calls = skillNames.map(name => `Call Skill({ skill: '${name}' })`).join('. '); return `${calls}.`; } diff --git a/src/resources/extensions/gsd/tests/skill-activation.test.ts b/src/resources/extensions/gsd/tests/skill-activation.test.ts index 312c078bf..064b68f5c 100644 --- a/src/resources/extensions/gsd/tests/skill-activation.test.ts +++ b/src/resources/extensions/gsd/tests/skill-activation.test.ts @@ -191,3 +191,23 @@ test("buildSkillActivationBlock does not activate skills from extraContext or ta cleanup(base); } }); + +test("buildSkillActivationBlock handles skill names with special characters safely", () => { + const base = makeTempBase(); + try { + // Skill names come from directory names — test that quotes/braces don't break the template + writeSkill(base, "my-skill's", "Skill with apostrophe in name."); + loadOnlyTestSkills(base); + + const result = buildBlock(base, {}, { + always_use_skills: ["my-skill's"], + }); + + // The skill name is interpolated as-is — this documents current behavior. + // A future guard (e.g. /^[a-z0-9-]+$/) could reject such names. + assert.match(result, /skill_activation/); + assert.match(result, /my-skill's/); + } finally { + cleanup(base); + } +});