Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions scripts/generate-skill-index.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,8 @@ def _process_skill_dir(skill_dir: Path, skill_file_override: Path | None = None)
old_ok = (repo_root / existing["file"]).is_file() if repo_root else False
if old_ok and not new_ok:
return
if old_ok and new_ok:
warnings.append(f" - {name}: duplicate skill name — {existing['file']} overwritten by {entry['file']}")
index[collection_key][name] = entry

for child in sorted(source_dir.iterdir()):
Expand All @@ -443,6 +445,16 @@ def _process_skill_dir(skill_dir: Path, skill_file_override: Path | None = None)
# Check if this directory directly contains a SKILL.md (flat layout)
if (child / "SKILL.md").exists():
_process_skill_dir(child)
# Hybrid dirs (e.g. skills/process/, skills/research/) have both a top-level SKILL.md and nested skill subdirs.
for nested in sorted(child.iterdir()):
if not nested.is_dir():
continue
if nested.is_symlink() and not include_private:
continue
if (nested / "SKILL.md").exists():
_process_skill_dir(nested)
elif (nested / "skill" / "SKILL.md").exists():
_process_skill_dir(nested, skill_file_override=nested / "skill" / "SKILL.md")
# Check for nested skill/SKILL.md layout (e.g., voice-example/skill/SKILL.md)
elif (child / "skill" / "SKILL.md").exists():
_process_skill_dir(child, skill_file_override=child / "skill" / "SKILL.md")
Expand Down
53 changes: 53 additions & 0 deletions scripts/tests/test_generate_skill_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -376,3 +376,56 @@ def test_include_private_run_writes_only_the_requested_output(self, tmp_path: Pa
data = json.loads(output.read_text(encoding="utf-8"))
# Floor tracks the post-consolidation catalog; raise it with new skills.
assert 35 <= len(data["skills"]) <= 80


class TestHybridDirIndexing:
"""Hybrid dirs (top-level SKILL.md + nested skill subdirs) appear fully in index."""

def test_hybrid_dir_top_and_nested_both_indexed(self, tmp_path: Path) -> None:
"""A hybrid dir's top-level skill and its nested skill both appear in the index."""
skills_dir = tmp_path / "skills"
# Top-level hybrid skill (e.g. skills/process/SKILL.md)
process_dir = skills_dir / "process"
process_dir.mkdir(parents=True)
(process_dir / "SKILL.md").write_text(_SKILL_FRONTMATTER.format(name="process"))
# Nested skill under the hybrid dir (e.g. skills/process/pr-workflow/SKILL.md)
nested_dir = process_dir / "pr-workflow"
nested_dir.mkdir()
(nested_dir / "SKILL.md").write_text(_SKILL_FRONTMATTER.format(name="pr-workflow"))

index, _warnings = gsi.generate_index(
source_dir=skills_dir,
dir_prefix="skills",
collection_key="skills",
)

assert "process" in index["skills"], "Top-level hybrid skill must be indexed"
assert "pr-workflow" in index["skills"], "Nested skill in hybrid dir must be indexed"


class TestOverwriteWarning:
"""A duplicate skill name emits an overwrite warning."""

def test_duplicate_name_emits_warning(self, tmp_path: Path) -> None:
"""Two skill dirs with the same name: in frontmatter triggers the overwrite-warning branch."""
skills_dir = tmp_path / "skills"
# Two different dir names, same name: in frontmatter so both map to "shared-name".
dir_a = skills_dir / "cat-a" / "shared-skill"
dir_a.mkdir(parents=True)
(dir_a / "SKILL.md").write_text(_SKILL_FRONTMATTER.format(name="shared-name"))

dir_b = skills_dir / "cat-b" / "shared-skill"
dir_b.mkdir(parents=True)
(dir_b / "SKILL.md").write_text(_SKILL_FRONTMATTER.format(name="shared-name"))

index, warnings = gsi.generate_index(
source_dir=skills_dir,
dir_prefix="skills",
collection_key="skills",
repo_root=tmp_path,
)

assert "shared-name" in index["skills"], "The skill must still be indexed"
assert any("shared-name" in w for w in warnings), (
f"Expected an overwrite warning for 'shared-name'; got warnings: {warnings}"
)
6 changes: 6 additions & 0 deletions scripts/validate-skill-names.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ def find_duplicates(repo: Path) -> dict[str, list[str]]:
continue
if (top / "SKILL.md").is_file():
seen.setdefault(top.name, []).append(top.relative_to(repo).as_posix())
# Hybrid dir: also scan immediate subdirs for nested skills.
# Intentionally scans only 1 level deep (vs. generate-skill-index.py's 2 levels):
# this is a fast-fail gate, not an exhaustive mirror of the indexer.
for child in sorted(top.iterdir()):
if child.is_dir() and (child / "SKILL.md").is_file():
seen.setdefault(child.name, []).append(child.relative_to(repo).as_posix())
continue
for child in sorted(top.iterdir()):
if child.is_dir() and (child / "SKILL.md").is_file():
Expand Down
2 changes: 1 addition & 1 deletion skills/meta/toolkit/references/toolkit-evolution.md
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ Win condition for each implementation:

**Step 1: Handle winners (WIN status)**

For each winning implementation, create a PR using the template from `references/evolve-scripts.md` § Step 1, then merge. After creating the PR, run pr-review to validate, then merge.
For each winning implementation, create a PR using the template from `references/evolve-scripts.md` § Step 1, then merge. After creating the PR, run /pr-review to validate, then merge.

The multi-persona critique + A/B testing gate is the review. Auto-merge is safe because the validation happened before this step.

Expand Down
4 changes: 2 additions & 2 deletions skills/process/pr-workflow/references/fix.md
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,8 @@ Solution:

### Related Skills
- `pr-pipeline` — For creating PRs from scratch
- `pr-review` — For reviewing code without fixing
- `systematic-debugging` — For general debugging unrelated to PR comments
- `review` — For reviewing code without fixing (skill); use `/pr-review` command for full multi-agent PR review
- `debugging` — For general debugging unrelated to PR comments

### PR Comment Best Practices
- Always validate before fixing (this prevents introducing bugs that reviewers caught)
Expand Down
2 changes: 1 addition & 1 deletion skills/process/pr-workflow/references/sync.md
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ Solution:

## References

- `/pr-review` -- Comprehensive PR review (used in the review-fix loop)
- `/pr-review` -- Comprehensive PR review slash command (not the `review` skill; used in the review-fix loop)
- `/pr-cleanup` -- Post-merge branch cleanup
- `scripts/classify-repo.py` -- Repo classification for workflow gating
- `scripts/adr-decision-coverage.py` -- ADR decision coverage checker
Loading