From 626e6918cfe4ec3be48c1a2a778e8a916306cfce Mon Sep 17 00:00:00 2001 From: Jesper Schulz-Wedde Date: Thu, 25 Jun 2026 12:53:52 +0200 Subject: [PATCH] Wiring-fix: single source of truth for review leaf list + CI enforcement De-enumerate the AL review leaf list so it lives in exactly one place: the al-code-review super-skill's frontmatter sub-skills list. - al-code-review.md: drop the enumerated parenthetical from description and remove the redundant bullet re-list in the Source section. - README.md: remove the hardcoded count word and the enumerated domain list from the leaf-skill sentence. - validate_frontmatter.py: add cross-file rule R26 asserting a super-skill's declared sub-skills exactly match the sibling al-*-review.md leaf files on disk (missing, stale, and unregistered leaves all fail CI). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/scripts/validate_frontmatter.py | 45 +++++++++++++++++++++++ README.md | 2 +- microsoft/skills/review/al-code-review.md | 18 +-------- 3 files changed, 48 insertions(+), 17 deletions(-) diff --git a/.github/scripts/validate_frontmatter.py b/.github/scripts/validate_frontmatter.py index 4d9bdc8..682a801 100644 --- a/.github/scripts/validate_frontmatter.py +++ b/.github/scripts/validate_frontmatter.py @@ -504,9 +504,48 @@ class SkillRecord: skill_id: str | None +def validate_sub_skills_registry(path: Path, fm: dict[str, Any], root: Path, report: Report) -> None: + """R26: a super-skill's declared `sub-skills` must exactly match the + `al-*-review.md` leaf files present in the same directory (set equality, + ordering-agnostic). This keeps the registered leaf list the single source + of truth and fails CI on a forgotten, stale, or missing registration. + + Only applies to action-skill files declaring a non-empty list-of-str + `sub-skills`. Files whose `sub-skills` is malformed are handled by R20. + """ + ss = fm.get("sub-skills") + if not is_non_empty_list_of_str(ss): + return + + declared = {s.lstrip("./") for s in ss} + + # Sibling leaves on disk, excluding the super-skill file itself. + leaves = { + p.relative_to(root).as_posix() + for p in path.parent.glob("al-*-review.md") + if p.resolve() != path.resolve() + } + + # Declared entries that are not real sibling leaves on disk (missing/stale). + for entry in sorted(declared - leaves): + entry_path = root / entry + if not entry_path.exists(): + report.error(path, "R26", f"declared sub-skill does not exist on disk: {entry}", 1) + else: + report.error( + path, "R26", + f"sub-skills entry is not a sibling 'al-*-review.md' leaf: {entry}", 1, + ) + + # Sibling leaves on disk that were never registered ('forgot to wire it up'). + for leaf in sorted(leaves - declared): + report.error(path, "R26", f"leaf not registered in sub-skills: {leaf}", 1) + + def run(root: Path) -> Report: report = Report() skill_records: list[SkillRecord] = [] + action_skill_fms: list[tuple[Path, dict[str, Any]]] = [] # Walk declared top-level folders only; avoid wandering into .git, etc. walk_roots = [root / "skills"] + [root / layer for layer in LAYERS] @@ -533,6 +572,8 @@ def run(root: Path) -> Report: validate_knowledge(path, parsed, report) elif kind == "action-skill": validate_action_skill(path, parsed, report) + if parsed.frontmatter: + action_skill_fms.append((path, parsed.frontmatter)) if parsed.frontmatter and isinstance(parsed.frontmatter.get("id"), str): skill_records.append(SkillRecord(path, "action-skill", parsed.frontmatter["id"])) elif kind == "meta": @@ -566,6 +607,10 @@ def run(root: Path) -> Report: others = [q.relative_to(root).as_posix() for q in paths if q != p] report.error(p, "R24", f"skill id '{sid}' ({kind}) is not unique; also defined in: {others}") + # Fourth pass: R26 sub-skills registry matches leaf files on disk + for path, fm in action_skill_fms: + validate_sub_skills_registry(path, fm, root, report) + return report diff --git a/README.md b/README.md index 5cdc715..ddab523 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,7 @@ Skills define how agents consume knowledge. They come in three flavors: READ and DO are read on demand — typically when the first dispatched action skill runs. They are not prerequisites for invoking Entry. WRITE is only used when scaffolding new content. -- **Action skills** — concrete skills that follow the Action Skill template to do real work (review code, audit telemetry, etc.). Action skills live inside the layers that own them (`/microsoft/skills/`, `/community/skills/`, `/custom/skills/`). An action skill is either a **leaf** that evaluates knowledge files directly, or a **super-skill** that composes other action skills (declared via `sub-skills` in frontmatter). The canonical reference is [`microsoft/skills/review/al-code-review.md`](microsoft/skills/review/al-code-review.md) (super-skill), which composes eleven leaf skills under [`microsoft/skills/review/`](microsoft/skills/review/) — one per knowledge domain (performance, security, privacy, upgrade, style, UI, error handling, events, interfaces, breaking changes, web services). +- **Action skills** — concrete skills that follow the Action Skill template to do real work (review code, audit telemetry, etc.). Action skills live inside the layers that own them (`/microsoft/skills/`, `/community/skills/`, `/custom/skills/`). An action skill is either a **leaf** that evaluates knowledge files directly, or a **super-skill** that composes other action skills (declared via `sub-skills` in frontmatter). The canonical reference is [`microsoft/skills/review/al-code-review.md`](microsoft/skills/review/al-code-review.md) (super-skill), which composes the AL review leaf skills under [`microsoft/skills/review/`](microsoft/skills/review/) — one per knowledge domain. ### Agent bootstrapping diff --git a/microsoft/skills/review/al-code-review.md b/microsoft/skills/review/al-code-review.md index a6ad4b8..ba58d70 100644 --- a/microsoft/skills/review/al-code-review.md +++ b/microsoft/skills/review/al-code-review.md @@ -3,7 +3,7 @@ kind: action-skill id: al-code-review version: 1 title: AL code review -description: Reviews AL source changes by composing the AL review leaf skills (performance, security, privacy, upgrade, style, UI, error handling, events, interfaces, breaking changes, web services). +description: Reviews AL source changes by composing the AL review leaf skills, one per knowledge domain. inputs: [pr-diff, file-path] outputs: [findings-report] bc-version: [all] @@ -34,21 +34,7 @@ An orchestrator invokes this skill with either a `pr-diff` (the standard PR-revi ## Source -The sub-skills invoked by this skill are those listed in frontmatter `sub-skills`: - -- `microsoft/skills/review/al-performance-review.md` -- `microsoft/skills/review/al-security-review.md` -- `microsoft/skills/review/al-privacy-review.md` -- `microsoft/skills/review/al-upgrade-review.md` -- `microsoft/skills/review/al-style-review.md` -- `microsoft/skills/review/al-ui-review.md` -- `microsoft/skills/review/al-error-handling-review.md` -- `microsoft/skills/review/al-events-review.md` -- `microsoft/skills/review/al-interfaces-review.md` -- `microsoft/skills/review/al-breaking-changes-review.md` -- `microsoft/skills/review/al-web-services-review.md` - -Additional leaf skills (for example, telemetry, testing) are added by updating the `sub-skills` list. The skill does not discover sub-skills implicitly. +The sub-skills invoked by this skill are those listed in frontmatter `sub-skills`. Additional leaf skills (for example, telemetry, testing) are added by updating the `sub-skills` list. The skill does not discover sub-skills implicitly. ## Relevance