fix(ci): make root-skill validation refuse to recommend destroying edits

validate_root_skills.py compared source and mirror with a symmetric `!=`,
so it detected divergence but never direction, and printed the same advice
either way: "Run sync_root_skills.py and commit."

That advice is only correct when the harness source moved ahead. When the
edit is in the generated skills/ mirror -- as in #360 -- sync regenerates
the mirror from the source and silently deletes the change. The validator
then passes, so CI turns green and certifies the loss.

Detect the direction. When the mirror holds lines the source lacks, name
them, point at the source file, and never suggest the destructive command.
Regenerable drift keeps the old advice. Also make sync_root_skills.py
report each file it creates or overwrites instead of writing in silence.

Verified: in-sync, mirror-edited, source-edited, missing-mirror, and
idempotent no-op.
This commit is contained in:
yuhao
2026-07-09 13:05:56 +00:00
parent ff419ea10c
commit bc536c9beb
2 changed files with 79 additions and 11 deletions
+15 -1
View File
@@ -66,12 +66,26 @@ def main() -> int:
sources = _discover_sources()
ROOT_SKILLS_DIR.mkdir(parents=True, exist_ok=True)
written = 0
for source in sources:
skill_id = _canonical_skill_id(source)
target = ROOT_SKILLS_DIR / skill_id / "SKILL.md"
target.parent.mkdir(parents=True, exist_ok=True)
content = source.read_text(encoding="utf-8")
target.write_text(_rewrite_name_frontmatter(content, skill_id), encoding="utf-8")
expected = _rewrite_name_frontmatter(content, skill_id)
if target.is_file() and target.read_text(encoding="utf-8") == expected:
continue
# Overwriting is destructive: the mirror may hold edits the source lacks.
# Say so, so the loss is never silent. validate_root_skills.py refuses
# in that direction before CI ever reaches here.
action = "overwrite" if target.is_file() else "create"
print(f"{action}: {target.relative_to(REPO_ROOT)} <- {source.relative_to(REPO_ROOT)}")
target.write_text(expected, encoding="utf-8")
written += 1
print(f"Synced {written} root skill(s); {len(sources) - written} already up to date.")
return 0
+64 -10
View File
@@ -3,6 +3,7 @@
from __future__ import annotations
import difflib
import sys
from pathlib import Path
@@ -10,6 +11,22 @@ from pathlib import Path
REPO_ROOT = Path(__file__).resolve().parents[2]
def _mirror_only_lines(expected: str, actual: str) -> list[str]:
"""Lines present in the root mirror but absent from the regenerated source.
These are exactly what `sync_root_skills.py` would overwrite and destroy,
because it regenerates the mirror from the harness source unconditionally.
"""
diff = difflib.unified_diff(
expected.splitlines(), actual.splitlines(), lineterm="", n=0
)
return [
line[1:].strip()
for line in diff
if line.startswith("+") and not line.startswith("+++") and line[1:].strip()
]
def _load_sync_helpers():
namespace: dict[str, object] = {"__file__": str(REPO_ROOT / ".github" / "scripts" / "sync_root_skills.py")}
sync_script = REPO_ROOT / ".github" / "scripts" / "sync_root_skills.py"
@@ -24,12 +41,17 @@ def main() -> int:
rewrite_name_frontmatter = sync["_rewrite_name_frontmatter"]
root_skills_dir = sync["ROOT_SKILLS_DIR"]
errors: list[str] = []
# Drift where the mirror holds content the source lacks. Regenerating would
# delete it, so these must be resolved by editing the source instead.
clobber: list[tuple[Path, Path, list[str]]] = []
# Drift the sync script can safely repair: the source moved ahead.
stale: list[str] = []
for source in discover_sources():
skill_id = canonical_skill_id(source)
target = root_skills_dir / skill_id / "SKILL.md"
if not target.is_file():
errors.append(
stale.append(
f"Missing root skill for {source.relative_to(REPO_ROOT)}: expected {target.relative_to(REPO_ROOT)}"
)
continue
@@ -37,23 +59,55 @@ def main() -> int:
source_content = source.read_text(encoding="utf-8")
expected = rewrite_name_frontmatter(source_content, skill_id)
actual = target.read_text(encoding="utf-8")
if actual != expected:
errors.append(
if actual == expected:
continue
mirror_only = _mirror_only_lines(expected, actual)
if mirror_only:
clobber.append((source, target, mirror_only))
else:
stale.append(
f"Out-of-sync root skill for {source.relative_to(REPO_ROOT)}: {target.relative_to(REPO_ROOT)}"
)
if errors:
print("Root skills validation failed:", file=sys.stderr)
for error in errors:
if not clobber and not stale:
print("Root skills validation passed.")
return 0
print("Root skills validation failed:", file=sys.stderr)
if clobber:
print(
"\nYou edited a GENERATED file. `skills/` is produced from the harness\n"
"SKILL.md sources; running the sync script would DELETE these edits.",
file=sys.stderr,
)
for source, target, mirror_only in clobber:
print(f"\n- {target.relative_to(REPO_ROOT)}", file=sys.stderr)
print(
f" contains content not present in {source.relative_to(REPO_ROOT)}:",
file=sys.stderr,
)
for line in mirror_only[:10]:
print(f" + {line}", file=sys.stderr)
if len(mirror_only) > 10:
print(f" … and {len(mirror_only) - 10} more line(s)", file=sys.stderr)
print(
f" Fix: move the change into {source.relative_to(REPO_ROOT)},\n"
f" then run `python3 .github/scripts/sync_root_skills.py`.",
file=sys.stderr,
)
if stale:
print("\nThe following root skills are stale and can be regenerated:", file=sys.stderr)
for error in stale:
print(f"- {error}", file=sys.stderr)
print(
"Run `python3 .github/scripts/sync_root_skills.py` and commit the updated root skills.",
file=sys.stderr,
)
return 1
print("Root skills validation passed.")
return 0
return 1
if __name__ == "__main__":