feat(kyberforge): retarget forge skills to author/audit APM content #93
Reference in New Issue
Block a user
Delete Branch "feat/89-apm-native-authoring"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What changed
Retargets the four factory skills —
skill-author,skill-audit,agent-author,agent-audit— plusforge/SKILL.mdStep 4, to author and audit APM-native.apm/content instead of the old hand-maintainedplugin.jsonshape, per ADR-0015 and ADR-0016.skill-authornow scaffolds into<package-root>/.apm/skills/<name>/at plugin/APM scope. Standalone authoring (noapm.yml/.gitanywhere, e.g.~/.agents/skills/) is unchanged.agent-authornow scaffolds a single vendor-neutral.apm/agents/<name>.agent.mdfile at plugin/APM scope —name/description/model/body only. Notools,isolation,maxTurns,effort,memory, orpermissionMode:apm compilehas no per-target field integrator for the agent primitive, so those fields can't be emitted safely to both Claude Code and Copilot CLI at once (ADR-0016). Project/user scope keep the existing Claude Code + Copilot dual-file pair, unchanged.agent-auditrestructured to validate that single-file shape: a frontmatter allowlist check (name/description/model) in place of the old CC/Copilot pair-consistency check, which no longer applies at plugin/APM scope by design.skill-auditandforge/SKILL.mdStep 4 swapped theirplugin.json-presence scope signal for the sameapm.yml+ top-leveltype:walk-up used everywhere else in this batch;forge.md's version-bump detection now hands off toapm-workflowinstead of the deprecatedplugin-author.Scope detection, now consistent across all four skills' scripts (
new-skill.sh,new-agent.sh,validate.sh,validate-provenance.sh): walk up from the given path; the nearest ancestorapm.ymlwith a top-leveltype:field is plugin/APM scope; atype:-lessapm.ymlis a marketplace-only manifest, skipped; otherwise stop at.git(project scope) or$HOME/filesystem root (user scope).Review
Went through two rounds of post-implementation review (parallel-fork review of all four workstreams, then a second pass over the docs/ADR commits and the first round's own fixes). Found and fixed 4 real defects, each with a regression test:
$HOME-shadowed-by-dotfiles-.gitscope-detection bug — a dotfiles-managed home directory (yadm, chezmoi bare-repo, etc.) misresolved to project scope instead of user scope. Same bug class, found and fixed independently in bothnew-agent.shandvalidate.sh.agent-audit'svalidate.sh(an always-Falseparameter and its branch).source_keysentry, inconsistent with its sibling files from the same commit).Test plan
skill-authorbats suite passingagent-authorbats suite passing (30/30)agent-auditbats suite passing (37/37, includes 2 new regression tests)bash tests/run-tests.sh) — 39/39 shell-script tests, 12/12 summary categories, 0 failuresRefs: #89
ADR: 0015, 0016
Automated review findings (10) — all reproduced by direct script execution. Root cause: the plugin-scope walk-up logic (
detect_scope/find_package_root) was independently reimplemented across four scripts (validate.sh,validate-provenance.sh,new-agent.sh,new-skill.sh) and the copies have drifted from each other and from the pre-PR behavior.@@ -29,0 +25,4 @@## apm-agent-allowlistname description modelAllowlist contradicts agent-author's own instructions.
apm-agent-allowlistomitssource_keys, contradictingagent-author/SKILL.md's explicit instruction to addsource_keysat plugin/APM scope when research-sourced — so a correctly-authored file fails validation.Reproduced: a package-scope agent file with
source_keys:failsvalidate.shwithFAIL field 'source_keys' is not in the vendor-neutral APM agent allowlist (description, model, name).@@ -5,8 +5,10 @@ usage() {cat <<EOFDocumented parity with validate.sh doesn't hold. This usage text and
agent-author/SKILL.mdboth claim this script walks up 'the same wayvalidate.shdoes', but it lacksvalidate.sh's$HOME-boundary and quote-tolerance handling — the documented behavioral parity is false (see the two paired findings above on this PR).@@ -59,3 +55,3 @@sys.exit(2)# --- Find plugin root ---TYPE_RE = re.compile(r'^type:\s*(instructions|skill|hybrid|prompts)\b')Not quote-tolerant, unlike validate.sh.
TYPE_RElacks the quote-tolerant['\"]?group thatvalidate.sh'sAPM_TYPE_REhas, so a quotedtype: "skill"value is recognized as plugin scope byvalidate.shbut not byvalidate-provenance.sh(ornew-agent.sh/new-skill.sh).Reproduced: with
type: "skill"inapm.yml,validate.shcorrectly detects plugin scope;validate-provenance.shon the same file exits 0 silently even with an unresolvedsource_keysentry that should have FAILed.@@ -62,1 +61,4 @@# marketplace-only manifest (see monorepo-and-repo-shapes.md) — skip it and# keep walking. Stop at a .git boundary or the filesystem root: neither is# plugin/APM scope, so this script has nothing to check there.def find_plugin_root(start_dir):Silent no-op on plugin.json-only plugins.
find_plugin_root()sharesvalidate.sh's plugin.json blind spot, so provenance validation silently exits 0 (no check performed) for every existing plugin.json-only plugin.Reproduced:
validate-provenance.sh plugins/kyberforge/agents/apm-orchestrate.mdexits 0 with no output even though the file has realsource_keysand a matchingsources.mdexists — the check this script exists to run never fires, with no signal that it was skipped.@@ -171,0 +146,4 @@return Truereturn Falsedef detect_scope(start_dir):Misclassifies legacy plugin.json-only plugins.
detect_scope()no longer recognizesplugin.json(onlyapm.yml), so existing plugin.json-only plugins are misclassified as project scope with the wrong counterpart path.Reproduced:
validate.sh plugins/kyberforge/agents/apm-orchestrate.md— a real, correctly-paired agent in this very plugin — fails withFAIL counterpart file not found: .github/agents/apm-orchestrate.agent.md, even though the real counterpart sits right next to it.@@ -171,0 +158,4 @@# can't shadow user scope by being its own .git repo.if current == home:return 'user', homeif os.path.isdir(os.path.join(current, '.git')):.git-as-file (worktrees) not recognized as a boundary. The project-boundary check usesisdir()/[[ -d ]]in all four rewritten walk-up implementations, which misses git worktrees where.gitis a regular file (gitdir: ...), not a directory.In a
git worktree addcheckout, none of the four walk-ups (validate.sh,validate-provenance.sh,new-agent.sh,new-skill.sh) recognize.gitas a project boundary, so the walk continues past the intended project root for any agent/skill work done inside a worktree.@@ -171,0 +161,4 @@if os.path.isdir(os.path.join(current, '.git')):return 'project', currentparent = os.path.dirname(current)if parent == current:Root-fallback scope disagrees with new-agent.sh.
detect_scope()'s filesystem-root fallback returns 'user' scope (pinned to real$HOME), whilenew-agent.sh's equivalent fallback returns 'project' scope rooted at the given path — the two scripts disagree on any directory outside$HOMEwith no.git/apm.ymlabove it.Reproduced:
new-agent.sh test-agent /tmp/scratch(outside$HOME, no.gitanywhere above it) correctly creates a project-scope pair. Runningvalidate.shon the created file then falls back to user scope and looks for the counterpart at$HOME/.copilot/agents/test-agent.agent.md, failing withFAIL counterpart file not foundeven though the valid pair sits right there.@@ -83,0 +96,4 @@# below, so a dotfiles repo at $HOME can't shadow user scope).# - a .git directory marks the project-scope boundary — stop.# - filesystem root reached with neither found — boundary-reached.find_package_root() {Scaffolds into the wrong directory for existing plugins.
find_package_root()dropped the directplugin.jsoncheck at$ROOT, so scaffolding a new agent inside an existing plugin.json-only plugin creates files in the wrong place.Reproduced:
new-agent.sh <name> plugins/kyberforge/createsplugins/kyberforge/.claude/agents/<name>.md+.github/agents/<name>.agent.mdinstead ofplugins/kyberforge/agents/<name>.md/.agent.md, diverging from where every other agent in that plugin actually lives.@@ -65,0 +87,4 @@# top-level 'type:' field is a marketplace-only manifest — skip it and keep# walking up. Prints two lines: the resolved root, then the mode.# ---------------------------------------------------------------------------find_package_root() {Missing $HOME boundary check.
find_package_root()has no$HOMEboundary check (unlikenew-agent.sh's), so the walk-up can continue past$HOMEand bind to an unrelated ancestor package.Reproduced with a fake
$HOMEnested under a directory with a type-bearingapm.ymlabove it:new-skill.sh my-skill $HOME/skillswalked past$HOMEand scaffolded into the ancestor package's.apm/skills/my-skill/instead of the intended standalone location under$HOME.@@ -65,0 +92,4 @@current="$(cd "$1" && pwd)"while true; doif [[ -f "$current/apm.yml" ]]; thenif grep -qE '^type:[[:space:]]*(instructions|skill|hybrid|prompts)\b' "$current/apm.yml"; thenLoose regex lets malformed
type:values false-match. Thistype:regex uses\bword-boundary matching (also present invalidate.sh/validate-provenance.sh) instead ofnew-agent.sh's stricter([[:space:]]|$), so a malformed value liketype: prompts-onlyfalse-matches as validpromptsin three scripts but is correctly rejected innew-agent.sh.An
apm.ymlwithtype: prompts-only(typo) is treated as a validtype: promptspackage byvalidate.sh,validate-provenance.sh, andnew-skill.sh, butnew-agent.shwalks past it looking for a different package root — an agent and a skill scaffolded from the same directory land in different roots for the same manifest.Automated review findings (10) — all reproduced by direct script execution. Root cause: the plugin-scope walk-up logic (detect_scope/find_package_root) was independently reimplemented across four scripts (validate.sh, validate-provenance.sh, new-agent.sh, new-skill.sh) and the copies have drifted from each other and from the pre-PR behavior.
@@ -29,0 +25,4 @@## apm-agent-allowlistname description modelAllowlist contradicts agent-author's own instructions.
apm-agent-allowlistomitssource_keys, contradictingagent-author/SKILL.md's explicit instruction to addsource_keysat plugin/APM scope when research-sourced — so a correctly-authored file fails validation.Reproduced: a package-scope agent file with
source_keys:failsvalidate.shwithFAIL field 'source_keys' is not in the vendor-neutral APM agent allowlist (description, model, name).@@ -5,8 +5,10 @@ usage() {cat <<EOFDocumented parity with validate.sh doesn't hold. This usage text and
agent-author/SKILL.mdboth claim this script walks up 'the same wayvalidate.shdoes', but it lacksvalidate.sh's$HOME-boundary and quote-tolerance handling — the documented behavioral parity is false (see the two paired findings above on this PR).@@ -59,3 +55,3 @@sys.exit(2)# --- Find plugin root ---TYPE_RE = re.compile(r'^type:\s*(instructions|skill|hybrid|prompts)\b')Not quote-tolerant, unlike validate.sh.
TYPE_RElacks the quote-tolerant['\"]?group thatvalidate.sh'sAPM_TYPE_REhas, so a quotedtype: "skill"value is recognized as plugin scope byvalidate.shbut not byvalidate-provenance.sh(ornew-agent.sh/new-skill.sh).Reproduced: with
type: "skill"inapm.yml,validate.shcorrectly detects plugin scope;validate-provenance.shon the same file exits 0 silently even with an unresolvedsource_keysentry that should have FAILed.@@ -62,1 +61,4 @@# marketplace-only manifest (see monorepo-and-repo-shapes.md) — skip it and# keep walking. Stop at a .git boundary or the filesystem root: neither is# plugin/APM scope, so this script has nothing to check there.def find_plugin_root(start_dir):Silent no-op on plugin.json-only plugins.
find_plugin_root()sharesvalidate.sh's plugin.json blind spot, so provenance validation silently exits 0 (no check performed) for every existing plugin.json-only plugin.Reproduced:
validate-provenance.sh plugins/kyberforge/agents/apm-orchestrate.mdexits 0 with no output even though the file has realsource_keysand a matchingsources.mdexists — the check this script exists to run never fires, with no signal that it was skipped.@@ -171,0 +146,4 @@return Truereturn Falsedef detect_scope(start_dir):Misclassifies legacy plugin.json-only plugins.
detect_scope()no longer recognizesplugin.json(onlyapm.yml), so existing plugin.json-only plugins are misclassified as project scope with the wrong counterpart path.Reproduced:
validate.sh plugins/kyberforge/agents/apm-orchestrate.md— a real, correctly-paired agent in this very plugin — fails withFAIL counterpart file not found: .github/agents/apm-orchestrate.agent.md, even though the real counterpart sits right next to it.@@ -171,0 +158,4 @@# can't shadow user scope by being its own .git repo.if current == home:return 'user', homeif os.path.isdir(os.path.join(current, '.git')):.git-as-file (worktrees) not recognized as a boundary. The project-boundary check usesisdir()/[[ -d ]]in all four rewritten walk-up implementations, which misses git worktrees where.gitis a regular file (gitdir: ...), not a directory.In a
git worktree addcheckout, none of the four walk-ups (validate.sh,validate-provenance.sh,new-agent.sh,new-skill.sh) recognize.gitas a project boundary, so the walk continues past the intended project root for any agent/skill work done inside a worktree.@@ -171,0 +161,4 @@if os.path.isdir(os.path.join(current, '.git')):return 'project', currentparent = os.path.dirname(current)if parent == current:Root-fallback scope disagrees with new-agent.sh.
detect_scope()'s filesystem-root fallback returns 'user' scope (pinned to real$HOME), whilenew-agent.sh's equivalent fallback returns 'project' scope rooted at the given path — the two scripts disagree on any directory outside$HOMEwith no.git/apm.ymlabove it.Reproduced:
new-agent.sh test-agent /tmp/scratch(outside$HOME, no.gitanywhere above it) correctly creates a project-scope pair. Runningvalidate.shon the created file then falls back to user scope and looks for the counterpart at$HOME/.copilot/agents/test-agent.agent.md, failing withFAIL counterpart file not foundeven though the valid pair sits right there.@@ -83,0 +96,4 @@# below, so a dotfiles repo at $HOME can't shadow user scope).# - a .git directory marks the project-scope boundary — stop.# - filesystem root reached with neither found — boundary-reached.find_package_root() {Scaffolds into the wrong directory for existing plugins.
find_package_root()dropped the directplugin.jsoncheck at$ROOT, so scaffolding a new agent inside an existing plugin.json-only plugin creates files in the wrong place.Reproduced:
new-agent.sh <name> plugins/kyberforge/createsplugins/kyberforge/.claude/agents/<name>.md+.github/agents/<name>.agent.mdinstead ofplugins/kyberforge/agents/<name>.md/.agent.md, diverging from where every other agent in that plugin actually lives.@@ -65,0 +87,4 @@# top-level 'type:' field is a marketplace-only manifest — skip it and keep# walking up. Prints two lines: the resolved root, then the mode.# ---------------------------------------------------------------------------find_package_root() {Missing $HOME boundary check.
find_package_root()has no$HOMEboundary check (unlikenew-agent.sh's), so the walk-up can continue past$HOMEand bind to an unrelated ancestor package.Reproduced with a fake
$HOMEnested under a directory with a type-bearingapm.ymlabove it:new-skill.sh my-skill $HOME/skillswalked past$HOMEand scaffolded into the ancestor package's.apm/skills/my-skill/instead of the intended standalone location under$HOME.@@ -65,0 +92,4 @@current="$(cd "$1" && pwd)"while true; doif [[ -f "$current/apm.yml" ]]; thenif grep -qE '^type:[[:space:]]*(instructions|skill|hybrid|prompts)\b' "$current/apm.yml"; thenLoose regex lets malformed
type:values false-match. Thistype:regex uses\bword-boundary matching (also present invalidate.sh/validate-provenance.sh) instead ofnew-agent.sh's stricter([[:space:]]|$), so a malformed value liketype: prompts-onlyfalse-matches as validpromptsin three scripts but is correctly rejected innew-agent.sh.An
apm.ymlwithtype: prompts-only(typo) is treated as a validtype: promptspackage byvalidate.sh,validate-provenance.sh, andnew-skill.sh, butnew-agent.shwalks past it looking for a different package root — an agent and a skill scaffolded from the same directory land in different roots for the same manifest.Follow-up on the review above
I fixed the 6 confirmed defects (commit
f037d49) and re-verified against the docs before touching code:Fixed:
field-inventory.md'sapm-agent-allowlistwas missingsource_keys— contradictedagent-author/SKILL.md's own Step 5 checklist, which explicitly allowssource_keysat plugin/APM scope. A correctly-authored file would failvalidate.sh.validate.sh'sAPM_TYPE_REandvalidate-provenance.sh'sTYPE_REdisagreed on quote-tolerance (type: "skill"matched one but not the other) despiteagent-audit/SKILL.md:51explicitly documenting thatvalidate-provenance.shwalks up "the same wayvalidate.shdoes". Both also used\bword-boundary matching, which false-matches a malformed value liketype: prompts-onlyon thepromptsprefix. Unified both regexes (quote-tolerant, exact-match)..gitproject-boundary check usedisdir()/[[ -d ]], which misses git worktrees where.gitis a file (gitdir: ...), not a directory. Switched toexists()/[[ -e ]].new-agent.shandnew-skill.shhad the same quote-intolerance via inlinegrep -qE(plusnew-skill.shhad the same\bfalse-match bug) — replaced both with a shared-shapeis_apm_package_manifestbash helper mirroring the Python regex.Retracted — not bugs: 4 of my original 10 findings turned out to be intentional, already-documented design decisions, not defects:
validate.sh,validate-provenance.sh, andnew-agent.shno longer treating a bareplugin.jsonas a scope signal is explicit, stated behavior —agent-audit/SKILL.md:30("plugin.json/.claude-plugin/plugin.jsonare no longer scope signals for this skill") andagent-author/SKILL.md:87("A bareplugin.jsonwith noapm.ymlno longer signals plugin scope — that path is fully replaced, not dual-mode"). Real plugin.json-based plugins (including kyberforge itself, today) are expected to fall through to project scope until issue #90's actual conversion — my initial review flagged the resulting behavior onplugins/kyberforge/agents/apm-orchestrate.mdas a regression without checking these docs first. I was wrong to call it a bug.new-skill.shhaving no$HOMEboundary check (unlikenew-agent.sh) is also intentional —skill-authoruses a simpler 2-mode design (package/standalone) with no user-scope concept at all, per its own SKILL.md, which gives~/.agents/skills/as the canonical standalone example.Deferred, not fixed here:
validate.sh's filesystem-root fallback (→ user scope) andnew-agent.sh's (→ project scope) disagree for a non-git directory outside$HOMEwith no apm.yml above it. Traced this back — it predates PR #93 entirely; both fallbacks are faithful continuations of each script's own pre-PR behavior, just newly encoded into the walk-up loops here. Fixing it means picking a side and updating one script's documented, historical contract, which deserves its own deliberate decision rather than a fix folded into this PR. Recommend a follow-up issue.Verified via direct reproduction of each defect plus the full suite: 147/147 bats tests, 39/39 shell-script tests, 12/12 summary categories — all passing on the pushed commit.
Follow-up fixes from a fresh review
Ran a second, independent review of this PR's diff (separate from the earlier stale review already on this thread). Found and fixed 6 additional issues, all with regression tests. Full suite: 158 bats tests, 39 shell-script tests, 12/12 summary categories, 0 failures.
Fixed:
new-agent.sh: a marker-less subdirectory under$HOMEwas silently walked up to user scope, contradicting the script's own "user scope is checked directly, no walk-up" usage text — risking scaffolding into shared global~/.claude/~/.copilotinstead of the intended local path. Now resolves to project scope like any other unmatched boundary.apm.ymltype:manifest detector innew-agent.sh/new-skill.shaccepted mismatched quotes (e.g.type: "skill') thatvalidate.sh's regex correctly rejects. Now requires matching quote characters, mirroringvalidate.sh'sAPM_TYPE_RE.apm.ymlline lacking a trailing newline (awhile readloop quirk), causing the scaffolder and validator to disagree on scope for identical input.validate.shnow hard-FAILs if plugin-scope agent frontmatter still contains unstrippedapm-agent.mdtemplate HTML comments —apm compilecopies frontmatter verbatim and<!-- -->isn't valid YAML, so a file that previously "passed audit" could still break parsing on both downstream harnesses.agent-auditalready implements a SUGGESTION heuristic for tool-restriction-needing plugin-scope agents — it doesn't yet; marked as not-yet-implemented, tracked as follow-up.agent-audit/README.mdstill described the old plugin-pair/silent-tolerance model this PR replaced.Also closed a test-coverage gap:
validate.sh's project/user-scope CC-only/Copilot-only field cross-checks and counterpart-missing check lost their only test coverage when the old plugin-pair fixture was deleted — added project/user-scope equivalents.Deliberately not fixed here (scope kept tight to what's needed for correctness):
detect_scope/find_package_rootlogic is still hand-copied across 4 scripts (2 bash, 2 Python), which is the root cause of the quote/newline bugs above. Consolidating into a shared implementation is a real architectural change — opening a follow-up issue rather than expanding this PR.validate.shandnew-agent.shon scope fallback (uservsproject) for a path with no.git/apm.ymloutside$HOMEremains open, as this PR's author already noted. Worth folding into the same follow-up issue as the consolidation work, since a newly-scaffolded project-scope file nested under$HOME(per the scope-hijack fix above) can now also hit this same class ofvalidate.shdisagreement — noting it here so it's not lost.New commits pushed, approval review dismissed automatically according to repository settings
Follow-up fix pass (post-approval)
Three rounds of independent review surfaced a real bug family in the scope-walkup logic, now fixed and pushed.
fix(kyberforge)—044b2d3:validate.sh'sdetect_scope()andvalidate-provenance.sh'sfind_plugin_root()disagreed withnew-agent.sh's already-correct, documented walk-up semantics on three points, each causingvalidate.shto false-FAIL a legitimately-scaffolded project-scope agent pair:$HOMEwas misclassified as user scope instead of project scope.git-boundary branch returned the walked-to.gitlocation instead of the conventional scope root, breaking any<root>that's a subdirectory of a larger git-tracked tree (monorepo package dirs)Also adds
scripts/check-scope-walkup-sync.sh, a behavioral drift-guard (per ADR-0014's no-cross-skill-path precedent) that cross-checks the four independently hand-ported walk-up implementations (validate.sh,validate-provenance.sh,new-agent.sh,new-skill.sh) against real fixture scaffolds, wired into.pre-commit-config.yamlatpre-push.docs(kyberforge)—eada85d: corrected stale "3 fields, nothing else" documentation across 6 files (SKILL.mdx2,README.md,ADR-0016,deployment-modes.md,apm-agent.mdtemplate) to documentsource_keysas the intentional 4th field in theapm-agent-allowlist— it was already implemented infield-inventory.md/validate.sh, and the template itself instructed authors to add it despite its own header claiming otherwise.All changes verified via
bash tests/run-tests.sh(13/13 passed) plus targeted before/after reproduction of every bug fixed, and the new drift-guard ran clean on this push's own pre-push hooks.