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)
This commit is contained in:
mastertyko 2026-03-23 20:17:01 +01:00
parent 006184456a
commit 5a64da32d3
2 changed files with 22 additions and 1 deletions

View file

@ -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 `<skill_activation>${calls}.</skill_activation>`;
}

View file

@ -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);
}
});