mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 17:36:53 +01:00
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>
This commit is contained in:
parent
13f47f65a8
commit
626e6918cf
3 changed files with 48 additions and 17 deletions
45
.github/scripts/validate_frontmatter.py
vendored
45
.github/scripts/validate_frontmatter.py
vendored
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue