Skip to content

Commit 9f3454f

Browse files
fix(skills): diff installed skills against bundled template by default
Default `brigade skills diff` to the Brigade package template so stale registry copies cannot mask drift from newer bundled skills. Add `--against registry` for the previous registry baseline. Fixes #484. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 6aba1c2 commit 9f3454f

4 files changed

Lines changed: 122 additions & 5 deletions

File tree

docs/technical-guide.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -889,7 +889,7 @@ Shared skill registry commands:
889889
- `brigade skills sync --workspace . --target all --trust workspace` scans the reviewed registry once and reports each selected skill and harness as current, missing, changed, blocked, or excluded. It is a dry run unless `--write` is present. Writes use explicit registry identities and the existing per-target receipt and rollback path, so completed targets survive a later target failure.
890890
- `brigade skills compatibility security-review` reports supported, installed, planned, and blocked harness targets for a skill, plus independent source, renderer, and local-copy drift, install history counts, trust score, and changelog status.
891891
- `brigade skills history security-review --harness codex` lists local install receipts for one skill and harness from `.brigade/skills/installs/history.jsonl`.
892-
- `brigade skills diff security-review --harness codex` compares the installed harness file against the current rendered resolved source. Bundled skills compare with the current Brigade package and report source, renderer, and local-edit drift separately. Receipts from older schemas report unknown provenance instead of guessing.
892+
- `brigade skills diff security-review --harness codex` compares the installed harness file against the bundled Brigade package template by default, even when a same-named registry entry exists. Pass `--against registry` to diff against the local `.brigade/skills/registry/` copy instead. Bundled skills report source, renderer, and local-edit drift separately. Receipts from older schemas report unknown provenance instead of guessing.
893893
- `brigade skills fleet status` reports installed skill copies across harnesses in stable order and prints one forced reinstall command for each supported stale or missing copy. Copies whose current metadata no longer supports their harness are listed separately as unsupported and get an uninstall command. Unknown legacy copies are listed without an automatic repair command.
894894
- `brigade skills rollback security-review --target claude` restores the latest rollback snapshot captured before a forced reinstall.
895895
- `brigade skills inbox add ./some-skill`, `list`, `show`, `diff`, `accept`, and `reject` keep agent-proposed skills in review before they enter the registry.

src/brigade/cli/skills.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,18 @@ def register(sub: argparse._SubParsersAction) -> None:
6767
p_skills_history.add_argument("--limit", type=int, default=20, help="Maximum history rows to show.")
6868
p_skills_history.add_argument("--json", action="store_true", help="Print machine-readable JSON.")
6969
p_skills_diff = skills_sub.add_parser(
70-
"diff", help="Diff an installed skill against the current rendered registry version."
70+
"diff",
71+
help="Diff an installed skill against the bundled Brigade template (default) or the local registry.",
7172
)
7273
p_skills_diff.add_argument("skill", help="Skill id, path, or directory.")
7374
p_skills_diff.add_argument("--target", "-t", type=Path, default=Path("."), help="Workspace registry to inspect.")
7475
p_skills_diff.add_argument("--harness", required=True, help="Harness target to compare.")
76+
p_skills_diff.add_argument(
77+
"--against",
78+
choices=["bundled", "registry"],
79+
default="bundled",
80+
help="Comparison baseline: bundled package template (default) or local registry entry.",
81+
)
7582
p_skills_diff.add_argument("--json", action="store_true", help="Print machine-readable JSON.")
7683
p_skills_fleet = skills_sub.add_parser("fleet", help="Inspect installed skill copies across harnesses.")
7784
skills_fleet_sub = p_skills_fleet.add_subparsers(dest="skills_fleet_command", metavar="<skills-fleet-command>")
@@ -251,7 +258,13 @@ def dispatch(args) -> int:
251258
json_output=args.json,
252259
)
253260
if args.skills_command == "diff":
254-
return skills_cmd.diff(target=args.target, skill=args.skill, harness=args.harness, json_output=args.json)
261+
return skills_cmd.diff(
262+
target=args.target,
263+
skill=args.skill,
264+
harness=args.harness,
265+
against=args.against,
266+
json_output=args.json,
267+
)
255268
if args.skills_command == "fleet":
256269
if args.skills_fleet_command == "status":
257270
return skills_cmd.fleet_status(target=args.target, json_output=args.json)

src/brigade/skills_cmd.py

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,37 @@ def _bundled_skill_path(skill_id: str) -> Path:
463463
return template_root() / "skills" / _slug(skill_id)
464464

465465

466+
def _bundled_skill_exists(skill_id: str) -> bool:
467+
return _skill_md_path(_bundled_skill_path(skill_id)).is_file()
468+
469+
470+
def _resolve_diff_baseline(target: Path, skill_or_path: str, *, against: str = "bundled") -> str:
471+
requested = str(skill_or_path)
472+
candidate = Path(requested).expanduser()
473+
if candidate.exists():
474+
return requested
475+
if requested.startswith("registry:"):
476+
skill_id = _slug(requested.removeprefix("registry:"))
477+
elif requested.startswith("bundled:"):
478+
skill_id = _slug(requested.removeprefix("bundled:"))
479+
else:
480+
skill_id = _slug(requested)
481+
registry_dir = _skill_path(target, skill_id)
482+
registry_exists = _skill_md_path(registry_dir).is_file()
483+
bundled_exists = _bundled_skill_exists(skill_id)
484+
if against == "registry":
485+
if registry_exists:
486+
return f"registry:{skill_id}"
487+
if bundled_exists:
488+
return f"bundled:{skill_id}"
489+
return requested
490+
if bundled_exists:
491+
return f"bundled:{skill_id}"
492+
if registry_exists:
493+
return f"registry:{skill_id}"
494+
return requested
495+
496+
466497
def _source_identity(*, skill_dir: Path, skill_id: str, kind: str, reviewed: bool) -> dict[str, Any]:
467498
if kind == "brigade-bundle":
468499
identity = f"{BUNDLED_SOURCE_PREFIX}{skill_id}"
@@ -1540,9 +1571,10 @@ def history(
15401571
return 0
15411572

15421573

1543-
def diff(*, target: Path, skill: str, harness: str, json_output: bool = False) -> int:
1574+
def diff(*, target: Path, skill: str, harness: str, against: str = "bundled", json_output: bool = False) -> int:
15441575
target = target.expanduser().resolve()
1545-
lint_payload = _lint_payload(target, skill)
1576+
baseline_skill = _resolve_diff_baseline(target, skill, against=against)
1577+
lint_payload = _lint_payload(target, baseline_skill)
15461578
if not lint_payload["valid"]:
15471579
if json_output:
15481580
print(
@@ -1588,6 +1620,8 @@ def diff(*, target: Path, skill: str, harness: str, json_output: bool = False) -
15881620
"target": str(target),
15891621
"skill_id": skill_id,
15901622
"harness": harness,
1623+
"against": against,
1624+
"baseline_skill": baseline_skill,
15911625
"installed": installed_skill.is_file(),
15921626
"installed_path": str(installed_skill),
15931627
"changed": bool(diff_lines),

tests/test_skills_cmd.py

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -927,6 +927,76 @@ def test_bundled_install_and_diff_share_canonical_source(tmp_path, capsys):
927927
}
928928

929929

930+
def test_skills_diff_defaults_to_bundled_template_over_stale_registry(tmp_path, capsys):
931+
stale = _write_skill(tmp_path / ".brigade" / "skills" / "registry", name="brigade-work")
932+
(stale / "SKILL.md").write_text("# stale registry copy\n")
933+
934+
assert (
935+
skills_cmd.install(workspace=tmp_path, skill="registry:brigade-work", harness="claude", json_output=True) == 0
936+
)
937+
capsys.readouterr()
938+
939+
assert skills_cmd.diff(target=tmp_path, skill="brigade-work", harness="claude", json_output=True) == 0
940+
diff = json.loads(capsys.readouterr().out)
941+
assert diff["against"] == "bundled"
942+
assert diff["baseline_skill"] == "bundled:brigade-work"
943+
assert diff["source"]["kind"] == "brigade-bundle"
944+
assert diff["changed"] is True
945+
assert diff["drift"]["content_changed"] is True
946+
947+
948+
def test_skills_diff_registry_selector_still_uses_bundled_by_default(tmp_path, capsys):
949+
stale = _write_skill(tmp_path / ".brigade" / "skills" / "registry", name="brigade-work")
950+
(stale / "SKILL.md").write_text("# stale registry copy\n")
951+
952+
assert (
953+
skills_cmd.install(workspace=tmp_path, skill="registry:brigade-work", harness="claude", json_output=True) == 0
954+
)
955+
capsys.readouterr()
956+
957+
assert skills_cmd.diff(target=tmp_path, skill="registry:brigade-work", harness="claude", json_output=True) == 0
958+
diff = json.loads(capsys.readouterr().out)
959+
assert diff["baseline_skill"] == "bundled:brigade-work"
960+
assert diff["changed"] is True
961+
962+
963+
def test_skills_diff_against_registry_compares_installed_to_registry(tmp_path, capsys):
964+
stale = _write_skill(tmp_path / ".brigade" / "skills" / "registry", name="brigade-work")
965+
(stale / "SKILL.md").write_text("# stale registry copy\n")
966+
967+
assert (
968+
skills_cmd.install(workspace=tmp_path, skill="registry:brigade-work", harness="claude", json_output=True) == 0
969+
)
970+
capsys.readouterr()
971+
972+
assert (
973+
skills_cmd.diff(
974+
target=tmp_path,
975+
skill="brigade-work",
976+
harness="claude",
977+
against="registry",
978+
json_output=True,
979+
)
980+
== 0
981+
)
982+
diff = json.loads(capsys.readouterr().out)
983+
assert diff["against"] == "registry"
984+
assert diff["baseline_skill"] == "registry:brigade-work"
985+
assert diff["source"]["kind"] == "registry"
986+
assert diff["changed"] is False
987+
988+
989+
def test_skills_diff_matches_bundled_template_reports_clean(tmp_path, capsys):
990+
assert skills_cmd.install(workspace=tmp_path, skill="brigade-work", harness="cursor", json_output=True) == 0
991+
capsys.readouterr()
992+
993+
assert skills_cmd.diff(target=tmp_path, skill="brigade-work", harness="cursor", json_output=True) == 0
994+
diff = json.loads(capsys.readouterr().out)
995+
assert diff["baseline_skill"] == "bundled:brigade-work"
996+
assert diff["changed"] is False
997+
assert diff["drift"]["content_changed"] is False
998+
999+
9301000
def test_explicit_registry_source_does_not_inherit_bundled_review(tmp_path, capsys):
9311001
source = _write_skill(tmp_path / "source", name="brigade-work")
9321002
assert skills_cmd.import_skill(target=tmp_path, source=source, json_output=True) == 0

0 commit comments

Comments
 (0)