fix(build): deduplicate skill trees and mark generated files - #2
Open
smestern wants to merge 1 commit into
Open
Conversation
Resolves #1. Three skill trees existed because build/plugin/** was accidentally tracked alongside the true source (templates/) and the legitimate CI artifact (dist/sciagent/, compact profile, sci- prefix). This made it unclear which copy was canonical and invited drift from direct edits to generated files. Changes: * .gitignore: drop !build/plugin/** exception so the local build cache is no longer tracked. * git rm --cached build/plugin/sciagent/** (8 files): untrack accidentally committed build output. * scripts/build_plugin.py: every generated .md now carries an AUTO-GENERATED / DO NOT EDIT banner identifying its source template and regen command. Banner is injected as YAML # comments inside existing frontmatter (preserves line-1 ---, required by Copilot/Claude SKILL parsers and by _split_frontmatter); HTML-comment fallback for files without frontmatter. Banner ruler uses = to avoid colliding with the --- frontmatter delimiter. * scripts/build_plugin.py: _merge_skill_bodies now synthesises a proper top-level YAML frontmatter block (name + description, with argument-hint inherited from first source if available). Previously, merged skills (e.g. compact-profile review) emitted no top-level frontmatter and were malformed/unloadable. * templates/README.md: source-of-truth callout pointing readers at the build pipeline. * tests/test_build_plugin.py: 5 regression tests covering banner injection and an E2E check that every generated dist/sciagent/**/*.md starts with parseable YAML frontmatter containing 'name:'.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #1.
Diagnosis. Three skill trees existed because they play different roles, but that wasn't documented and one was committed by mistake:
templates/skills/build/plugin/sciagent/skills/build_plugin.pyoutput)dist/sciagent/skills/sci-prefix; mirrored tosmestern/sciagent-plugin)build-plugin.ymlThe
dist/content legitimately differs fromtemplates/because CI builds it with--profile compact --name-prefix sci, which merges some skills (e.g.code-reviewer+rigor-reviewer→review) and prefixes agent names. Thebuild/plugin/copy had no business being in git — it was a stale local build that snuck into a commit.Changes
!build/plugin/**exception from.gitignoreand untrack the 8 stray files (git rm --cached).AUTO-GENERATED … DO NOT EDITbanner into every generated.mdso it's unambiguous which file is the source. The banner is YAML#comments placed inside the frontmatter, so line 1 is still---and the file remains a valid SKILL.md / agent spec for Copilot, Claude Code, and_split_frontmatter. HTML-comment fallback for files without frontmatter. Banner ruler uses=to avoid colliding with the---delimiter._merge_skill_bodies()now synthesises a proper top-level YAML frontmatter block (name+description,argument-hintinherited from the first source). Previously, the mergedreviewskill indist/had no top-level frontmatter and would not load as a real skill — pre-existing bug surfaced by the new E2E test below.templates/README.md.tests/test_build_plugin.py, 5 tests): banner injection edge cases + E2E assertion that everydist/sciagent/**/*.mdparses as valid frontmatter with aname:field.Out of this PR by design
dist/sciagent/is left atorigin/mainhere;build-plugin.ymlwill regenerate and commit it on merge (force-adds viagit add -f), picking up the new banners and the merged-skill frontmatter fix automatically.Testing
pytest tests/ -m "not live"→ 57 passed.Credit to @yarikoptic for spotting and reporting the duplication.