fix(skills): refuse renaming a skill onto a name the owner already uses (#6941)

update_skill only checked whether the target directory existed. A skill
with that name in another category lives elsewhere on disk, so the
rename went through and left two skills with one name. Update and
delete look skills up by name, so the next delete removed whichever
file os.walk reached first.

Check the owner's skills in every category before renaming, and answer
409 from PUT /api/skills/{id} instead of the misleading 404.
This commit is contained in:
Robert Vind-Gardoș
2026-10-31 13:34:05 +00:00
committed by GitHub
parent 239f09cbc4
commit 9835c1a7f9
3 changed files with 104 additions and 0 deletions
+16
View File
@@ -500,6 +500,19 @@ class SkillsManager:
sk.path = self._skill_file(cat, nm)
return sk.to_dict()
def has_skill(self, name: str, owner: Optional[str] = None) -> bool:
"""Whether `owner` already has a skill called `name`, in any category.
Update and delete find a skill by name within its owner, so two of
one owner's skills sharing a name would make them act on whichever
file is walked first.
"""
for path in self._iter_skill_files():
sk = self._read_skill(path)
if sk and sk.name == name and (sk.owner or "") == (owner or ""):
return True
return False
def update_skill(self, skill_id: str, updates: Dict, owner: Optional[str] = None) -> bool:
"""`skill_id` is the slug name. Allows updating any field plus
renames if `name` changes (file is moved on disk).
@@ -551,6 +564,9 @@ class SkillsManager:
# Rename
new_name = slugify(updates.get("name") or sk.name)
if new_name != sk.name:
if self.has_skill(new_name, owner=owner):
logger.warning(f"Skill rename target name in use: {new_name}")
return False
sk.name = new_name
# Write to potentially new path