Skip to content

Commit 0340c9b

Browse files
committed
refactor: install skills directly to skills folder instead of nested path
- Add source_directory field to Skill class to store full path in repo - Change directory field to store just the skill folder name for installation - Update _fetch_skills_from_repo to set both fields correctly - Update _download_and_install_skill to use source_directory for source location - Fix tests to match new behavior Before: ~/.factory/skills/devops/terraform-module-builder/skills/terraform-module-builder/ After: ~/.factory/skills/terraform-module-builder/
1 parent 2c952a7 commit 0340c9b

4 files changed

Lines changed: 36 additions & 15 deletions

File tree

code_assistant_manager/skill_repos.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
"owner": "jeremylongshore",
3131
"name": "claude-code-plugins-plus",
3232
"branch": "main",
33-
"enabled": true
33+
"enabled": true,
34+
"skillsPath": "plugins"
3435
}
3536
}

code_assistant_manager/skills.py

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ def __init__(
9494
repo_branch: Optional[str] = None,
9595
skills_path: Optional[str] = None,
9696
readme_url: Optional[str] = None,
97+
source_directory: Optional[str] = None,
9798
):
9899
self.key = key
99100
self.name = name
@@ -105,6 +106,8 @@ def __init__(
105106
self.repo_branch = repo_branch or "main"
106107
self.skills_path = skills_path
107108
self.readme_url = readme_url
109+
# source_directory stores full path in repo; directory is just the skill folder name for installation
110+
self.source_directory = source_directory
108111

109112
def to_dict(self) -> Dict:
110113
"""Convert to dictionary."""
@@ -125,6 +128,8 @@ def to_dict(self) -> Dict:
125128
data["skillsPath"] = self.skills_path
126129
if self.readme_url:
127130
data["readmeUrl"] = self.readme_url
131+
if self.source_directory:
132+
data["sourceDirectory"] = self.source_directory
128133
return data
129134

130135
@classmethod
@@ -141,6 +146,7 @@ def from_dict(cls, data: Dict) -> "Skill":
141146
repo_branch=data.get("repoBranch"),
142147
skills_path=data.get("skillsPath"),
143148
readme_url=data.get("readmeUrl"),
149+
source_directory=data.get("sourceDirectory"),
144150
)
145151

146152

@@ -395,17 +401,19 @@ def _download_and_install_skill(self, skill: Skill, install_dir: Path) -> None:
395401

396402
try:
397403
# Determine the source path within the downloaded repo
404+
# Use source_directory if available (full path in repo), otherwise fall back to directory
405+
source_dir = skill.source_directory or skill.directory
398406
if skill.skills_path:
399-
source_path = temp_dir / skill.skills_path.strip("/") / skill.directory
407+
source_path = temp_dir / skill.skills_path.strip("/") / source_dir
400408
else:
401-
source_path = temp_dir / skill.directory
409+
source_path = temp_dir / source_dir
402410

403411
if not source_path.exists():
404412
raise ValueError(
405413
f"Skill directory not found in repository: {source_path}"
406414
)
407415

408-
# Copy to install directory
416+
# Copy to install directory using just the skill folder name (directory field)
409417
dest_path = install_dir / skill.directory
410418
if dest_path.exists():
411419
shutil.rmtree(dest_path)
@@ -643,14 +651,17 @@ def _fetch_skills_from_repo(self, repo: SkillRepo) -> List[Skill]:
643651
meta = self._parse_skill_metadata(skill_md)
644652

645653
try:
646-
# Calculate relative path from scan_dir
654+
# Calculate relative path from scan_dir (full path in repo relative to skills_path)
647655
rel_path = skill_dir.relative_to(scan_dir)
648-
directory = str(rel_path).replace("\\", "/")
656+
source_directory = str(rel_path).replace("\\", "/")
649657

650658
# Skip if SKILL.md is at the root of scan_dir (directory == ".")
651659
# to avoid conflicts when installing to the root of skills directory
652-
if directory == ".":
660+
if source_directory == ".":
653661
continue
662+
663+
# The install directory is just the skill folder name (last part of path)
664+
directory = skill_dir.name
654665
except ValueError:
655666
continue
656667

@@ -659,8 +670,8 @@ def _fetch_skills_from_repo(self, repo: SkillRepo) -> List[Skill]:
659670
readme_path = str(path_from_repo_root).replace("\\", "/")
660671

661672
skill = Skill(
662-
key=f"{repo.owner}/{repo.name}:{directory}",
663-
name=meta.get("name", directory.split("/")[-1]),
673+
key=f"{repo.owner}/{repo.name}:{source_directory}",
674+
name=meta.get("name", directory),
664675
description=meta.get("description", ""),
665676
directory=directory,
666677
installed=False,
@@ -669,6 +680,7 @@ def _fetch_skills_from_repo(self, repo: SkillRepo) -> List[Skill]:
669680
repo_branch=actual_branch,
670681
skills_path=repo.skills_path,
671682
readme_url=f"https://github.com/{repo.owner}/{repo.name}/tree/{actual_branch}/{readme_path}",
683+
source_directory=source_directory,
672684
)
673685
skills.append(skill)
674686
logger.debug(f"Found skill: {skill.key}")

tests/test_skills_recursive.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,21 +53,24 @@ def test_fetch_skills_recursive(self, mock_download, skill_manager):
5353
# Verify results
5454
assert len(skills) == 2
5555

56+
# directory is now just the skill folder name for installation
5657
skill_map = {s.directory: s for s in skills}
5758

5859
# Check Skill 1
5960
assert "skill1" in skill_map
6061
s1 = skill_map["skill1"]
6162
assert s1.name == "Skill One"
6263
assert s1.key == "owner/repo:skill1"
64+
assert s1.source_directory == "skill1"
6365
assert s1.readme_url.endswith("/skills/skill1")
6466

6567
# Check Skill 2
66-
# directory should use forward slashes
67-
assert "category/skill2" in skill_map
68-
s2 = skill_map["category/skill2"]
68+
# directory is just the folder name, source_directory has full path
69+
assert "skill2" in skill_map
70+
s2 = skill_map["skill2"]
6971
assert s2.name == "Skill Two"
7072
assert s2.key == "owner/repo:category/skill2"
73+
assert s2.source_directory == "category/skill2"
7174
assert s2.readme_url.endswith("/skills/category/skill2")
7275

7376
@patch("code_assistant_manager.skills.SkillManager._download_repo")
@@ -95,5 +98,8 @@ def test_fetch_skills_root_structure(self, mock_download, skill_manager):
9598

9699
# Should find nested skill but skip root skill
97100
assert len(skills) == 1
98-
assert skills[0].directory == "nested/skill"
101+
# directory is now just the folder name for installation
102+
assert skills[0].directory == "skill"
103+
# source_directory has the full path
104+
assert skills[0].source_directory == "nested/skill"
99105
assert skills[0].name == "Nested"

tests/unit/test_skills.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -321,8 +321,10 @@ def test_manager_sync_installed_status(self, temp_config_dir, temp_install_dir):
321321
"code_assistant_manager.skills.SKILL_INSTALL_DIRS",
322322
{"test_app": temp_install_dir},
323323
):
324-
# Create skill1 directory in install location
325-
(temp_install_dir / "skill1").mkdir(parents=True)
324+
# Create skill1 directory in install location with SKILL.md
325+
skill1_dir = temp_install_dir / "skill1"
326+
skill1_dir.mkdir(parents=True)
327+
(skill1_dir / "SKILL.md").write_text("---\nname: Skill 1\n---\n# Skill 1")
326328

327329
manager.sync_installed_status("test_app")
328330

0 commit comments

Comments
 (0)