feat(kyberforge): retarget forge skills to author/audit APM content #93

Merged
Defame1297 merged 14 commits from feat/89-apm-native-authoring into main 2026-08-12 11:48:50 +00:00
Collaborator

What changed

Retargets the four factory skills — skill-author, skill-audit, agent-author, agent-audit — plus forge/SKILL.md Step 4, to author and audit APM-native .apm/ content instead of the old hand-maintained plugin.json shape, per ADR-0015 and ADR-0016.

  • skill-author now scaffolds into <package-root>/.apm/skills/<name>/ at plugin/APM scope. Standalone authoring (no apm.yml/.git anywhere, e.g. ~/.agents/skills/) is unchanged.
  • agent-author now scaffolds a single vendor-neutral .apm/agents/<name>.agent.md file at plugin/APM scope — name/description/model/body only. No tools, isolation, maxTurns, effort, memory, or permissionMode: apm compile has 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-audit restructured 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-audit and forge/SKILL.md Step 4 swapped their plugin.json-presence scope signal for the same apm.yml + top-level type: walk-up used everywhere else in this batch; forge.md's version-bump detection now hands off to apm-workflow instead of the deprecated plugin-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 ancestor apm.yml with a top-level type: field is plugin/APM scope; a type:-less apm.yml is 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-.git scope-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 both new-agent.sh and validate.sh.
  • Dead/unreachable code left over from the restructure in agent-audit's validate.sh (an always-False parameter and its branch).
  • A provenance-chain gap in one research doc (missing source_keys entry, inconsistent with its sibling files from the same commit).

Test plan

  • skill-author bats suite passing
  • agent-author bats suite passing (30/30)
  • agent-audit bats suite passing (37/37, includes 2 new regression tests)
  • Full repo suite (bash tests/run-tests.sh) — 39/39 shell-script tests, 12/12 summary categories, 0 failures

Refs: #89
ADR: 0015, 0016

## What changed Retargets the four factory skills — `skill-author`, `skill-audit`, `agent-author`, `agent-audit` — plus `forge/SKILL.md` Step 4, to author and audit APM-native `.apm/` content instead of the old hand-maintained `plugin.json` shape, per ADR-0015 and ADR-0016. - **`skill-author`** now scaffolds into `<package-root>/.apm/skills/<name>/` at plugin/APM scope. Standalone authoring (no `apm.yml`/`.git` anywhere, e.g. `~/.agents/skills/`) is unchanged. - **`agent-author`** now scaffolds a single vendor-neutral `.apm/agents/<name>.agent.md` file at plugin/APM scope — `name`/`description`/`model`/body only. No `tools`, `isolation`, `maxTurns`, `effort`, `memory`, or `permissionMode`: `apm compile` has 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-audit`** restructured 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-audit`** and **`forge/SKILL.md` Step 4** swapped their `plugin.json`-presence scope signal for the same `apm.yml` + top-level `type:` walk-up used everywhere else in this batch; `forge.md`'s version-bump detection now hands off to `apm-workflow` instead of the deprecated `plugin-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 ancestor `apm.yml` with a top-level `type:` field is plugin/APM scope; a `type:`-less `apm.yml` is 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-`.git` scope-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 both `new-agent.sh` and `validate.sh`. - Dead/unreachable code left over from the restructure in `agent-audit`'s `validate.sh` (an always-`False` parameter and its branch). - A provenance-chain gap in one research doc (missing `source_keys` entry, inconsistent with its sibling files from the same commit). ## Test plan - [x] `skill-author` bats suite passing - [x] `agent-author` bats suite passing (30/30) - [x] `agent-audit` bats suite passing (37/37, includes 2 new regression tests) - [x] Full repo suite (`bash tests/run-tests.sh`) — 39/39 shell-script tests, 12/12 summary categories, 0 failures Refs: #89 ADR: 0015, 0016
Claude added 10 commits 2026-08-11 18:38:17 +00:00
skill-author/agent-author's #89 retarget needs to know exactly how each
.apm/ primitive compiles to Claude Code and Copilot CLI output. The
existing microsoft-apm corpus only had a full schema for skills and one
minimal example for agents, and nothing for prompts/instructions/hooks.

Deepened via APM's own Python source (not just docs) where prose was
thin. Key finding for #89: agents have no per-target integrator, so
apm compile does a naive verbatim copy to both Claude and Copilot,
unlike prompts/instructions/hooks which each get real per-target
reconstruction. That means the agent primitive's `tools:` field can't
express both harnesses' incompatible vocabularies at once — a real
upstream gap, not something we can schema our way around.
#89's grilling session surfaced a real upstream gap: APM's agent
primitive has no per-target compile integrator, so tools: and every
Claude-only field (isolation, maxTurns, effort, memory, permissionMode)
would verbatim-copy into Copilot's file with an incompatible or
unrecognized shape. Omitting them entirely is the only option that
never ships a wrong value to a real harness.

Scoped to plugin-scope agent-author only — project/user scope keep
today's Claude+Copilot pair model unchanged. Partially supersedes
ADR-0005 and ADR-0008's plugin-scope clauses (addenda appended to
both); ADR-0009's field-inventory.md mechanism is unaffected, only
its plugin-scope content shape changes.

Refs: #89
Skills now migrate from hand-authored plugin.json to Microsoft APM
(apm.yml + .apm/) per ADR-0015/issue #89. skill-author's write location
switches accordingly: walk up from the destination to the nearest
ancestor apm.yml declaring a type: field (skipping type:-less
marketplace-only manifests) and scaffold into <package-root>/.apm/skills/
<name>/. Standalone authoring (no apm.yml/.git anywhere, e.g.
~/.agents/skills/) is unchanged — only the old plugin.json-based path is
replaced, not standalone mode.

Refs: #89
Plugin scope now authors a single vendor-neutral .apm/agents/<name>.agent.md
file (name/description/model/body only) instead of a Claude Code + Copilot
CLI pair, per ADR-0016: apm compile has no per-target field integrator, so
tools: and all Claude-only fields (isolation/maxTurns/effort/memory/
permissionMode) are omitted entirely rather than shipping a value that's
guaranteed wrong on one harness. Scope detection switches from a flat
plugin.json check to the same apm.yml+type: walk-up agent-audit's
validate.sh already uses. Project scope and user scope are unaffected —
both keep the existing dual-file pair model.

Refs: #89
Validates the new single-file .apm/agents/<name>.agent.md shape agent-author
now produces at plugin/APM scope: frontmatter allowlist (name/description/
model only, from a new apm-agent-allowlist entry in field-inventory.md),
no counterpart derivation, and Pair Consistency dropped from that scope's
report entirely (nothing to pair by design). Adds a plugin/APM-scope-only
SUGGESTION when an agent's description/body implies a tool restriction or
Claude-only behavior the vendor-neutral frontmatter can no longer express
(ADR-0016).

Scope detection in both validate.sh and validate-provenance.sh switches
from a flat plugin.json/.claude-plugin/plugin.json check to a walk-up for
the nearest ancestor apm.yml with a top-level type: field, skipping
type:-less marketplace-only manifests — full switch, no dual-mode fallback
to the old plugin.json signal. validate-provenance.sh's walk-up was fixed
to match validate.sh's (it still used the old plugin.json check, and its
counterpart-merge logic was rewritten to read a single file's source_keys
instead of merging a CC+Copilot pair, since plugin/APM scope has no
counterpart). Project/user scope validation is unchanged in both scripts.

Refs: #89
skill-audit's cross-plugin path check now also recognizes .apm/skills/
<other-skill>/ as a breaking reference shape, alongside the existing
plugins/<plugin>/skills/<other-skill>/ pattern — the APM-native equivalent
of the same cache-isolation problem.

forge.md's Step 4 plugin-version-bump detection switches from "any
directory containing a plugin.json" to the same apm.yml+type: walk-up
used elsewhere in this batch, and hands off to apm-workflow (bump the
single apm.yml version) instead of the deprecated plugin-author.

Refs: #89
new-agent.sh's walk-up checked for a .git directory before checking
whether it had reached $HOME, so a dotfiles-managed home directory
(e.g. `~/.git` from yadm or a bare-repo checkout) made `new-agent.sh
<name> ~` silently resolve to project scope instead of user scope,
writing .github/agents/ instead of ~/.copilot/agents/. Check the
$HOME boundary before the .git check so it can no longer be shadowed.

Found via post-implementation review of issue #89.
check_file()'s is_plugin_scope param and its plugin-silently-ignored
field check were unreachable dead code left over from the issue #89
restructure: plugin/APM scope now exits via check_apm_agent_file()
before check_file() is ever called, so is_plugin was always False.
Remove the param, its branch, the unused plugin_ignored_fields parse,
the now-stale field-inventory.md section, and the SKILL.md mention.

Found via post-implementation review of issue #89.
detect_scope() had the same bug class fixed in new-agent.sh (099bdec):
it checked for a .git directory before checking whether it had reached
$HOME, so a dotfiles-managed home directory (yadm, chezmoi bare-repo,
etc.) made validate.sh misresolve to project scope, deriving the
counterpart as ~/.github/agents/<name>.agent.md instead of the correct
~/.copilot/agents/<name>.agent.md and failing with a false "counterpart
file not found". Check the $HOME boundary before the .git check, same
fix shape as 099bdec.

Found via post-implementation review of issue #89.
The file's body cites APM's Python source directly (apm_cli/models/validation.py,
apm_cli/primitives/models.py) to support its "no dedicated Agent validation"
claim, but source_keys only listed context7-microsoft-apm — the apm-github-repo
source_key was missing, and sources.md's apm-github-repo entry didn't list this
file under Contributing files either. The three sibling schema files from the
same commit (prompt/instructions/hooks) all cited both sources correctly.

Found via post-implementation review of issue #89.
Claude reviewed 2026-08-11 18:57:32 +00:00
Claude left a comment
Author
Collaborator

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.

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-allowlist
name description model
Author
Collaborator

Allowlist contradicts agent-author's own instructions. apm-agent-allowlist omits source_keys, contradicting agent-author/SKILL.md's explicit instruction to add source_keys at plugin/APM scope when research-sourced — so a correctly-authored file fails validation.

Reproduced: a package-scope agent file with source_keys: fails validate.sh with FAIL field 'source_keys' is not in the vendor-neutral APM agent allowlist (description, model, name).

**Allowlist contradicts agent-author's own instructions.** `apm-agent-allowlist` omits `source_keys`, contradicting `agent-author/SKILL.md`'s explicit instruction to add `source_keys` at plugin/APM scope when research-sourced — so a correctly-authored file fails validation. Reproduced: a package-scope agent file with `source_keys:` fails `validate.sh` with `FAIL field 'source_keys' is not in the vendor-neutral APM agent allowlist (description, model, name)`.
@@ -5,8 +5,10 @@ usage() {
cat <<EOF
Author
Collaborator

Documented parity with validate.sh doesn't hold. This usage text and agent-author/SKILL.md both claim this script walks up 'the same way validate.sh does', but it lacks validate.sh's $HOME-boundary and quote-tolerance handling — the documented behavioral parity is false (see the two paired findings above on this PR).

**Documented parity with validate.sh doesn't hold.** This usage text and `agent-author/SKILL.md` both claim this script walks up 'the same way `validate.sh` does', but it lacks `validate.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')
Author
Collaborator

Not quote-tolerant, unlike validate.sh. TYPE_RE lacks the quote-tolerant ['\"]? group that validate.sh's APM_TYPE_RE has, so a quoted type: "skill" value is recognized as plugin scope by validate.sh but not by validate-provenance.sh (or new-agent.sh/new-skill.sh).

Reproduced: with type: "skill" in apm.yml, validate.sh correctly detects plugin scope; validate-provenance.sh on the same file exits 0 silently even with an unresolved source_keys entry that should have FAILed.

**Not quote-tolerant, unlike validate.sh.** `TYPE_RE` lacks the quote-tolerant `['\"]?` group that `validate.sh`'s `APM_TYPE_RE` has, so a quoted `type: "skill"` value is recognized as plugin scope by `validate.sh` but not by `validate-provenance.sh` (or `new-agent.sh`/`new-skill.sh`). Reproduced: with `type: "skill"` in `apm.yml`, `validate.sh` correctly detects plugin scope; `validate-provenance.sh` on the same file exits 0 silently even with an unresolved `source_keys` entry 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):
Author
Collaborator

Silent no-op on plugin.json-only plugins. find_plugin_root() shares validate.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.md exits 0 with no output even though the file has real source_keys and a matching sources.md exists — the check this script exists to run never fires, with no signal that it was skipped.

**Silent no-op on plugin.json-only plugins.** `find_plugin_root()` shares `validate.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.md` exits 0 with no output even though the file has real `source_keys` and a matching `sources.md` exists — the check this script exists to run never fires, with no signal that it was skipped.
@@ -171,0 +146,4 @@
return True
return False
def detect_scope(start_dir):
Author
Collaborator

Misclassifies legacy plugin.json-only plugins. detect_scope() no longer recognizes plugin.json (only apm.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 with FAIL counterpart file not found: .github/agents/apm-orchestrate.agent.md, even though the real counterpart sits right next to it.

**Misclassifies legacy plugin.json-only plugins.** `detect_scope()` no longer recognizes `plugin.json` (only `apm.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 with `FAIL 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', home
if os.path.isdir(os.path.join(current, '.git')):
Author
Collaborator

.git-as-file (worktrees) not recognized as a boundary. The project-boundary check uses isdir()/[[ -d ]] in all four rewritten walk-up implementations, which misses git worktrees where .git is a regular file (gitdir: ...), not a directory.

In a git worktree add checkout, none of the four walk-ups (validate.sh, validate-provenance.sh, new-agent.sh, new-skill.sh) recognize .git as a project boundary, so the walk continues past the intended project root for any agent/skill work done inside a worktree.

**`.git`-as-file (worktrees) not recognized as a boundary.** The project-boundary check uses `isdir()`/`[[ -d ]]` in all four rewritten walk-up implementations, which misses git worktrees where `.git` is a regular file (`gitdir: ...`), not a directory. In a `git worktree add` checkout, none of the four walk-ups (`validate.sh`, `validate-provenance.sh`, `new-agent.sh`, `new-skill.sh`) recognize `.git` as 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', current
parent = os.path.dirname(current)
if parent == current:
Author
Collaborator

Root-fallback scope disagrees with new-agent.sh. detect_scope()'s filesystem-root fallback returns 'user' scope (pinned to real $HOME), while new-agent.sh's equivalent fallback returns 'project' scope rooted at the given path — the two scripts disagree on any directory outside $HOME with no .git/apm.yml above it.

Reproduced: new-agent.sh test-agent /tmp/scratch (outside $HOME, no .git anywhere above it) correctly creates a project-scope pair. Running validate.sh on the created file then falls back to user scope and looks for the counterpart at $HOME/.copilot/agents/test-agent.agent.md, failing with FAIL counterpart file not found even though the valid pair sits right there.

**Root-fallback scope disagrees with new-agent.sh.** `detect_scope()`'s filesystem-root fallback returns 'user' scope (pinned to real `$HOME`), while `new-agent.sh`'s equivalent fallback returns 'project' scope rooted at the given path — the two scripts disagree on any directory outside `$HOME` with no `.git`/`apm.yml` above it. Reproduced: `new-agent.sh test-agent /tmp/scratch` (outside `$HOME`, no `.git` anywhere above it) correctly creates a project-scope pair. Running `validate.sh` on the created file then falls back to user scope and looks for the counterpart at `$HOME/.copilot/agents/test-agent.agent.md`, failing with `FAIL counterpart file not found` even 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() {
Author
Collaborator

Scaffolds into the wrong directory for existing plugins. find_package_root() dropped the direct plugin.json check 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/ creates plugins/kyberforge/.claude/agents/<name>.md + .github/agents/<name>.agent.md instead of plugins/kyberforge/agents/<name>.md/.agent.md, diverging from where every other agent in that plugin actually lives.

**Scaffolds into the wrong directory for existing plugins.** `find_package_root()` dropped the direct `plugin.json` check 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/` creates `plugins/kyberforge/.claude/agents/<name>.md` + `.github/agents/<name>.agent.md` instead of `plugins/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() {
Author
Collaborator

Missing $HOME boundary check. find_package_root() has no $HOME boundary check (unlike new-agent.sh's), so the walk-up can continue past $HOME and bind to an unrelated ancestor package.

Reproduced with a fake $HOME nested under a directory with a type-bearing apm.yml above it: new-skill.sh my-skill $HOME/skills walked past $HOME and scaffolded into the ancestor package's .apm/skills/my-skill/ instead of the intended standalone location under $HOME.

**Missing $HOME boundary check.** `find_package_root()` has no `$HOME` boundary check (unlike `new-agent.sh`'s), so the walk-up can continue past `$HOME` and bind to an unrelated ancestor package. Reproduced with a fake `$HOME` nested under a directory with a type-bearing `apm.yml` above it: `new-skill.sh my-skill $HOME/skills` walked past `$HOME` and 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; do
if [[ -f "$current/apm.yml" ]]; then
if grep -qE '^type:[[:space:]]*(instructions|skill|hybrid|prompts)\b' "$current/apm.yml"; then
Author
Collaborator

Loose regex lets malformed type: values false-match. This type: regex uses \b word-boundary matching (also present in validate.sh/validate-provenance.sh) instead of new-agent.sh's stricter ([[:space:]]|$), so a malformed value like type: prompts-only false-matches as valid prompts in three scripts but is correctly rejected in new-agent.sh.

An apm.yml with type: prompts-only (typo) is treated as a valid type: prompts package by validate.sh, validate-provenance.sh, and new-skill.sh, but new-agent.sh walks 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.

**Loose regex lets malformed `type:` values false-match.** This `type:` regex uses `\b` word-boundary matching (also present in `validate.sh`/`validate-provenance.sh`) instead of `new-agent.sh`'s stricter `([[:space:]]|$)`, so a malformed value like `type: prompts-only` false-matches as valid `prompts` in three scripts but is correctly rejected in `new-agent.sh`. An `apm.yml` with `type: prompts-only` (typo) is treated as a valid `type: prompts` package by `validate.sh`, `validate-provenance.sh`, and `new-skill.sh`, but `new-agent.sh` walks 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.
Claude reviewed 2026-08-11 18:57:41 +00:00
Claude left a comment
Author
Collaborator

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.

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-allowlist
name description model
Author
Collaborator

Allowlist contradicts agent-author's own instructions. apm-agent-allowlist omits source_keys, contradicting agent-author/SKILL.md's explicit instruction to add source_keys at plugin/APM scope when research-sourced — so a correctly-authored file fails validation.

Reproduced: a package-scope agent file with source_keys: fails validate.sh with FAIL field 'source_keys' is not in the vendor-neutral APM agent allowlist (description, model, name).

**Allowlist contradicts agent-author's own instructions.** `apm-agent-allowlist` omits `source_keys`, contradicting `agent-author/SKILL.md`'s explicit instruction to add `source_keys` at plugin/APM scope when research-sourced — so a correctly-authored file fails validation. Reproduced: a package-scope agent file with `source_keys:` fails `validate.sh` with `FAIL field 'source_keys' is not in the vendor-neutral APM agent allowlist (description, model, name)`.
@@ -5,8 +5,10 @@ usage() {
cat <<EOF
Author
Collaborator

Documented parity with validate.sh doesn't hold. This usage text and agent-author/SKILL.md both claim this script walks up 'the same way validate.sh does', but it lacks validate.sh's $HOME-boundary and quote-tolerance handling — the documented behavioral parity is false (see the two paired findings above on this PR).

**Documented parity with validate.sh doesn't hold.** This usage text and `agent-author/SKILL.md` both claim this script walks up 'the same way `validate.sh` does', but it lacks `validate.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')
Author
Collaborator

Not quote-tolerant, unlike validate.sh. TYPE_RE lacks the quote-tolerant ['\"]? group that validate.sh's APM_TYPE_RE has, so a quoted type: "skill" value is recognized as plugin scope by validate.sh but not by validate-provenance.sh (or new-agent.sh/new-skill.sh).

Reproduced: with type: "skill" in apm.yml, validate.sh correctly detects plugin scope; validate-provenance.sh on the same file exits 0 silently even with an unresolved source_keys entry that should have FAILed.

**Not quote-tolerant, unlike validate.sh.** `TYPE_RE` lacks the quote-tolerant `['\"]?` group that `validate.sh`'s `APM_TYPE_RE` has, so a quoted `type: "skill"` value is recognized as plugin scope by `validate.sh` but not by `validate-provenance.sh` (or `new-agent.sh`/`new-skill.sh`). Reproduced: with `type: "skill"` in `apm.yml`, `validate.sh` correctly detects plugin scope; `validate-provenance.sh` on the same file exits 0 silently even with an unresolved `source_keys` entry 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):
Author
Collaborator

Silent no-op on plugin.json-only plugins. find_plugin_root() shares validate.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.md exits 0 with no output even though the file has real source_keys and a matching sources.md exists — the check this script exists to run never fires, with no signal that it was skipped.

**Silent no-op on plugin.json-only plugins.** `find_plugin_root()` shares `validate.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.md` exits 0 with no output even though the file has real `source_keys` and a matching `sources.md` exists — the check this script exists to run never fires, with no signal that it was skipped.
@@ -171,0 +146,4 @@
return True
return False
def detect_scope(start_dir):
Author
Collaborator

Misclassifies legacy plugin.json-only plugins. detect_scope() no longer recognizes plugin.json (only apm.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 with FAIL counterpart file not found: .github/agents/apm-orchestrate.agent.md, even though the real counterpart sits right next to it.

**Misclassifies legacy plugin.json-only plugins.** `detect_scope()` no longer recognizes `plugin.json` (only `apm.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 with `FAIL 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', home
if os.path.isdir(os.path.join(current, '.git')):
Author
Collaborator

.git-as-file (worktrees) not recognized as a boundary. The project-boundary check uses isdir()/[[ -d ]] in all four rewritten walk-up implementations, which misses git worktrees where .git is a regular file (gitdir: ...), not a directory.

In a git worktree add checkout, none of the four walk-ups (validate.sh, validate-provenance.sh, new-agent.sh, new-skill.sh) recognize .git as a project boundary, so the walk continues past the intended project root for any agent/skill work done inside a worktree.

**`.git`-as-file (worktrees) not recognized as a boundary.** The project-boundary check uses `isdir()`/`[[ -d ]]` in all four rewritten walk-up implementations, which misses git worktrees where `.git` is a regular file (`gitdir: ...`), not a directory. In a `git worktree add` checkout, none of the four walk-ups (`validate.sh`, `validate-provenance.sh`, `new-agent.sh`, `new-skill.sh`) recognize `.git` as 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', current
parent = os.path.dirname(current)
if parent == current:
Author
Collaborator

Root-fallback scope disagrees with new-agent.sh. detect_scope()'s filesystem-root fallback returns 'user' scope (pinned to real $HOME), while new-agent.sh's equivalent fallback returns 'project' scope rooted at the given path — the two scripts disagree on any directory outside $HOME with no .git/apm.yml above it.

Reproduced: new-agent.sh test-agent /tmp/scratch (outside $HOME, no .git anywhere above it) correctly creates a project-scope pair. Running validate.sh on the created file then falls back to user scope and looks for the counterpart at $HOME/.copilot/agents/test-agent.agent.md, failing with FAIL counterpart file not found even though the valid pair sits right there.

**Root-fallback scope disagrees with new-agent.sh.** `detect_scope()`'s filesystem-root fallback returns 'user' scope (pinned to real `$HOME`), while `new-agent.sh`'s equivalent fallback returns 'project' scope rooted at the given path — the two scripts disagree on any directory outside `$HOME` with no `.git`/`apm.yml` above it. Reproduced: `new-agent.sh test-agent /tmp/scratch` (outside `$HOME`, no `.git` anywhere above it) correctly creates a project-scope pair. Running `validate.sh` on the created file then falls back to user scope and looks for the counterpart at `$HOME/.copilot/agents/test-agent.agent.md`, failing with `FAIL counterpart file not found` even 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() {
Author
Collaborator

Scaffolds into the wrong directory for existing plugins. find_package_root() dropped the direct plugin.json check 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/ creates plugins/kyberforge/.claude/agents/<name>.md + .github/agents/<name>.agent.md instead of plugins/kyberforge/agents/<name>.md/.agent.md, diverging from where every other agent in that plugin actually lives.

**Scaffolds into the wrong directory for existing plugins.** `find_package_root()` dropped the direct `plugin.json` check 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/` creates `plugins/kyberforge/.claude/agents/<name>.md` + `.github/agents/<name>.agent.md` instead of `plugins/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() {
Author
Collaborator

Missing $HOME boundary check. find_package_root() has no $HOME boundary check (unlike new-agent.sh's), so the walk-up can continue past $HOME and bind to an unrelated ancestor package.

Reproduced with a fake $HOME nested under a directory with a type-bearing apm.yml above it: new-skill.sh my-skill $HOME/skills walked past $HOME and scaffolded into the ancestor package's .apm/skills/my-skill/ instead of the intended standalone location under $HOME.

**Missing $HOME boundary check.** `find_package_root()` has no `$HOME` boundary check (unlike `new-agent.sh`'s), so the walk-up can continue past `$HOME` and bind to an unrelated ancestor package. Reproduced with a fake `$HOME` nested under a directory with a type-bearing `apm.yml` above it: `new-skill.sh my-skill $HOME/skills` walked past `$HOME` and 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; do
if [[ -f "$current/apm.yml" ]]; then
if grep -qE '^type:[[:space:]]*(instructions|skill|hybrid|prompts)\b' "$current/apm.yml"; then
Author
Collaborator

Loose regex lets malformed type: values false-match. This type: regex uses \b word-boundary matching (also present in validate.sh/validate-provenance.sh) instead of new-agent.sh's stricter ([[:space:]]|$), so a malformed value like type: prompts-only false-matches as valid prompts in three scripts but is correctly rejected in new-agent.sh.

An apm.yml with type: prompts-only (typo) is treated as a valid type: prompts package by validate.sh, validate-provenance.sh, and new-skill.sh, but new-agent.sh walks 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.

**Loose regex lets malformed `type:` values false-match.** This `type:` regex uses `\b` word-boundary matching (also present in `validate.sh`/`validate-provenance.sh`) instead of `new-agent.sh`'s stricter `([[:space:]]|$)`, so a malformed value like `type: prompts-only` false-matches as valid `prompts` in three scripts but is correctly rejected in `new-agent.sh`. An `apm.yml` with `type: prompts-only` (typo) is treated as a valid `type: prompts` package by `validate.sh`, `validate-provenance.sh`, and `new-skill.sh`, but `new-agent.sh` walks 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.
Defame1297 added 1 commit 2026-08-11 19:18:00 +00:00
Post-implementation review of PR #93 (issue #89's apm.yml-native retargeting
of skill-author/skill-audit/agent-author/agent-audit) found four confirmed
defects across the four scripts' apm.yml `type:` walk-up logic:

- field-inventory.md's apm-agent-allowlist was missing `source_keys`,
  contradicting agent-author/SKILL.md's own instruction (Step 5 checklist)
  to allow it at plugin/APM scope — a correctly-authored file with
  source_keys failed validate.sh.
- validate.sh's APM_TYPE_RE and validate-provenance.sh's TYPE_RE disagreed:
  the former tolerated a quoted `type: "skill"` value, the latter didn't,
  despite agent-audit/SKILL.md explicitly documenting that
  validate-provenance.sh walks up "the same way validate.sh does". Both
  also used `\b` word-boundary matching, which false-matches a malformed
  value like `type: prompts-only` on the `prompts` prefix. Unified both
  regexes to be quote-tolerant and require an exact value.
- All four scripts' `.git` project-boundary check used isdir()/[[ -d ]],
  which misses git worktrees where `.git` is a regular file (`gitdir: ...`)
  rather than a directory. Switched to exists()/[[ -e ]].
- new-agent.sh and new-skill.sh had the same quote-intolerance as above via
  inline `grep -qE` calls (new-skill.sh's also had the `\b` false-match
  bug); replaced both with a shared-shape `is_apm_package_manifest` bash
  helper matching the Python regex's semantics.

Four other findings from the same review turned out not to be bugs: a
bare `plugin.json` no longer signaling plugin scope is documented,
intentional behavior (agent-audit/SKILL.md:30, agent-author/SKILL.md:87),
deferred to issue #90's real plugin.json-to-apm.yml conversion — not
something this fix should reverse.

Verified via direct reproduction of each defect plus the full test suite:
147/147 bats tests, 39/39 shell-script tests, 12/12 summary categories.

Refs: #89
Author
Collaborator

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's apm-agent-allowlist was missing source_keys — contradicted agent-author/SKILL.md's own Step 5 checklist, which explicitly allows source_keys at plugin/APM scope. A correctly-authored file would fail validate.sh.
  • validate.sh's APM_TYPE_RE and validate-provenance.sh's TYPE_RE disagreed on quote-tolerance (type: "skill" matched one but not the other) despite agent-audit/SKILL.md:51 explicitly documenting that validate-provenance.sh walks up "the same way validate.sh does". Both also used \b word-boundary matching, which false-matches a malformed value like type: prompts-only on the prompts prefix. Unified both regexes (quote-tolerant, exact-match).
  • All four scripts' .git project-boundary check used isdir()/[[ -d ]], which misses git worktrees where .git is a file (gitdir: ...), not a directory. Switched to exists()/[[ -e ]].
  • new-agent.sh and new-skill.sh had the same quote-intolerance via inline grep -qE (plus new-skill.sh had the same \b false-match bug) — replaced both with a shared-shape is_apm_package_manifest bash 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, and new-agent.sh no longer treating a bare plugin.json as a scope signal is explicit, stated behavior — agent-audit/SKILL.md:30 ("plugin.json/.claude-plugin/plugin.json are no longer scope signals for this skill") and agent-author/SKILL.md:87 ("A bare plugin.json with no apm.yml no 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 on plugins/kyberforge/agents/apm-orchestrate.md as a regression without checking these docs first. I was wrong to call it a bug.
  • new-skill.sh having no $HOME boundary check (unlike new-agent.sh) is also intentional — skill-author uses 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) and new-agent.sh's (→ project scope) disagree for a non-git directory outside $HOME with 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 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`'s `apm-agent-allowlist` was missing `source_keys` — contradicted `agent-author/SKILL.md`'s own Step 5 checklist, which explicitly allows `source_keys` at plugin/APM scope. A correctly-authored file would fail `validate.sh`. - `validate.sh`'s `APM_TYPE_RE` and `validate-provenance.sh`'s `TYPE_RE` disagreed on quote-tolerance (`type: "skill"` matched one but not the other) despite `agent-audit/SKILL.md:51` explicitly documenting that `validate-provenance.sh` walks up "the same way `validate.sh` does". Both also used `\b` word-boundary matching, which false-matches a malformed value like `type: prompts-only` on the `prompts` prefix. Unified both regexes (quote-tolerant, exact-match). - All four scripts' `.git` project-boundary check used `isdir()`/`[[ -d ]]`, which misses git worktrees where `.git` is a file (`gitdir: ...`), not a directory. Switched to `exists()`/`[[ -e ]]`. - `new-agent.sh` and `new-skill.sh` had the same quote-intolerance via inline `grep -qE` (plus `new-skill.sh` had the same `\b` false-match bug) — replaced both with a shared-shape `is_apm_package_manifest` bash 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`, and `new-agent.sh` no longer treating a bare `plugin.json` as a scope signal is explicit, stated behavior — `agent-audit/SKILL.md:30` ("`plugin.json`/`.claude-plugin/plugin.json` are no longer scope signals for this skill") and `agent-author/SKILL.md:87` ("A bare `plugin.json` with no `apm.yml` no 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 on `plugins/kyberforge/agents/apm-orchestrate.md` as a regression without checking these docs first. I was wrong to call it a bug. - `new-skill.sh` having no `$HOME` boundary check (unlike `new-agent.sh`) is also intentional — `skill-author` uses 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) and `new-agent.sh`'s (→ project scope) disagree for a non-git directory outside `$HOME` with 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.
Defame1297 added 1 commit 2026-08-12 06:25:34 +00:00
A fresh /code-review of the APM-native authoring retarget (PR #93) found
several correctness bugs beyond the ones already fixed on this branch:

- new-agent.sh silently walked a marker-less subdirectory under $HOME up
  to user scope, contradicting its own usage text ("user scope is checked
  directly, no walk-up") and risking scaffolding into shared global
  ~/.claude or ~/.copilot directories instead of the intended local path.
- The hand-copied apm.yml type: manifest detector in new-agent.sh and
  new-skill.sh accepted mismatched quotes (e.g. `type: "skill'`) that
  validate.sh's regex correctly rejects, and silently dropped a final
  apm.yml line lacking a trailing newline — causing the scaffolder and
  validator to disagree on scope for identical input.
- Plugin-scope agent frontmatter could still contain the apm-agent.md
  template's HTML comments at ship time with no audit signal, yet
  apm compile copies frontmatter verbatim and <!-- --> breaks YAML
  parsing on both downstream harnesses.
- ADR-0016 asserted agent-audit already implements a SUGGESTION heuristic
  for tool-restriction-needing plugin-scope agents; it doesn't.
- agent-audit/README.md still described the old plugin-pair model this
  PR replaced with a single-file allowlist model.
- validate.sh's project/user-scope CC-only/Copilot-only field checks and
  counterpart-missing check lost their only test coverage when the old
  plugin-pair fixture was deleted.

Also replaces an echo-into-sed two-value parse (4 forks per call) with a
single space-separated echo + read in both scaffolders.

Regression tests added for every fix above, including one for a bug this
pass introduced and the test suite caught: an initial two-line
echo + `read` attempt silently dropped the second value, since `read`
consumes only one line regardless of embedded newlines.

Full suite: 158 bats tests, 39 shell-script tests, 12/12 summary
categories, 0 failures.

Refs: #89, #93
Author
Collaborator

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:

  • Scope-hijack bug in new-agent.sh: a marker-less subdirectory under $HOME was 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/~/.copilot instead of the intended local path. Now resolves to project scope like any other unmatched boundary.
  • Quote-matching divergence: the hand-copied apm.yml type: manifest detector in new-agent.sh/new-skill.sh accepted mismatched quotes (e.g. type: "skill') that validate.sh's regex correctly rejects. Now requires matching quote characters, mirroring validate.sh's APM_TYPE_RE.
  • Trailing-newline bug: the same detector silently dropped a final apm.yml line lacking a trailing newline (a while read loop quirk), causing the scaffolder and validator to disagree on scope for identical input.
  • Template YAML breakage: validate.sh now hard-FAILs if plugin-scope agent frontmatter still contains unstripped apm-agent.md template HTML comments — apm compile copies frontmatter verbatim and <!-- --> isn't valid YAML, so a file that previously "passed audit" could still break parsing on both downstream harnesses.
  • ADR-0016 overclaim: corrected wording that asserted agent-audit already implements a SUGGESTION heuristic for tool-restriction-needing plugin-scope agents — it doesn't yet; marked as not-yet-implemented, tracked as follow-up.
  • Stale README: agent-audit/README.md still 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):

  • The detect_scope/find_package_root logic 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.
  • The previously-flagged, already-acknowledged disagreement between validate.sh and new-agent.sh on scope fallback (user vs project) for a path with no .git/apm.yml outside $HOME remains 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 of validate.sh disagreement — noting it here so it's not lost.
## 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:** - **Scope-hijack bug in `new-agent.sh`**: a marker-less subdirectory under `$HOME` was 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`/`~/.copilot` instead of the intended local path. Now resolves to project scope like any other unmatched boundary. - **Quote-matching divergence**: the hand-copied `apm.yml` `type:` manifest detector in `new-agent.sh`/`new-skill.sh` accepted mismatched quotes (e.g. `type: "skill'`) that `validate.sh`'s regex correctly rejects. Now requires matching quote characters, mirroring `validate.sh`'s `APM_TYPE_RE`. - **Trailing-newline bug**: the same detector silently dropped a final `apm.yml` line lacking a trailing newline (a `while read` loop quirk), causing the scaffolder and validator to disagree on scope for identical input. - **Template YAML breakage**: `validate.sh` now hard-`FAIL`s if plugin-scope agent frontmatter still contains unstripped `apm-agent.md` template HTML comments — `apm compile` copies frontmatter verbatim and `<!-- -->` isn't valid YAML, so a file that previously "passed audit" could still break parsing on both downstream harnesses. - **ADR-0016 overclaim**: corrected wording that asserted `agent-audit` already implements a SUGGESTION heuristic for tool-restriction-needing plugin-scope agents — it doesn't yet; marked as not-yet-implemented, tracked as follow-up. - **Stale README**: `agent-audit/README.md` still 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): - The `detect_scope`/`find_package_root` logic 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. - The previously-flagged, already-acknowledged disagreement between `validate.sh` and `new-agent.sh` on scope fallback (`user` vs `project`) for a path with no `.git`/`apm.yml` outside `$HOME` remains 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 of `validate.sh` disagreement — noting it here so it's not lost.
Defame1297 approved these changes 2026-08-12 07:35:42 +00:00
Dismissed
Defame1297 added 2 commits 2026-08-12 11:38:09 +00:00
validate.sh's detect_scope() and validate-provenance.sh's
find_plugin_root() disagreed with new-agent.sh's already-correct,
documented walk-up semantics on three points, each causing validate.sh
to false-FAIL a legitimately-scaffolded project-scope agent pair:

- a marker-less directory walked up into $HOME (no .git/apm.yml of its
  own) was classified as user scope instead of project scope
- the .git-boundary branch returned the walked-to .git location instead
  of the conventional scope root, breaking any <root> that is a
  subdirectory of a larger git-tracked tree (monorepo package dirs)
- the new conventional-root arithmetic introduced to fix the above two
  cases had no guard against non-conventional/hand-placed file paths,
  which could point it at the wrong ancestor

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.yaml at pre-push so
future drift between the ports is caught automatically.

Verified via bash tests/run-tests.sh (13/13) and targeted before/after
reproduction of each bug this closes.
field-inventory.md's apm-agent-allowlist and validate.sh's runtime
check already included source_keys as a 4th allowed field, and the
apm-agent.md template already instructed authors to add it for
provenance tracking — but SKILL.md (x2), README.md, ADR-0016, and
deployment-modes.md still described the allowlist as name/description/
model, "nothing else". The template itself even contradicted its own
source_keys guidance with a header claiming "ONLY the three fields
below — full stop" directly above it.

Updates all six locations to document source_keys as the intentional
4th field, resolving the contradiction.
Defame1297 dismissed Defame1297's review 2026-08-12 11:38:09 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

Author
Collaborator

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's detect_scope() and validate-provenance.sh's find_plugin_root() disagreed with new-agent.sh's already-correct, documented walk-up semantics on three points, each causing validate.sh to false-FAIL a legitimately-scaffolded project-scope agent pair:

  • a marker-less directory walked up into $HOME was misclassified as user scope instead of project scope
  • the .git-boundary branch returned the walked-to .git location instead of the conventional scope root, breaking any <root> that's a subdirectory of a larger git-tracked tree (monorepo package dirs)
  • the fix for the above two needed a guard against non-conventional/hand-placed file paths, to avoid pointing at the wrong ancestor

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.yaml at pre-push.

docs(kyberforge) — eada85d: corrected stale "3 fields, nothing else" documentation across 6 files (SKILL.md x2, README.md, ADR-0016, deployment-modes.md, apm-agent.md template) to document source_keys as the intentional 4th field in the apm-agent-allowlist — it was already implemented in field-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.

## 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`'s `detect_scope()` and `validate-provenance.sh`'s `find_plugin_root()` disagreed with `new-agent.sh`'s already-correct, documented walk-up semantics on three points, each causing `validate.sh` to false-FAIL a legitimately-scaffolded project-scope agent pair: - a marker-less directory walked up into `$HOME` was misclassified as user scope instead of project scope - the `.git`-boundary branch returned the walked-to `.git` location instead of the conventional scope root, breaking any `<root>` that's a subdirectory of a larger git-tracked tree (monorepo package dirs) - the fix for the above two needed a guard against non-conventional/hand-placed file paths, to avoid pointing at the wrong ancestor 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.yaml` at `pre-push`. **`docs(kyberforge)` — eada85d:** corrected stale "3 fields, nothing else" documentation across 6 files (`SKILL.md` x2, `README.md`, `ADR-0016`, `deployment-modes.md`, `apm-agent.md` template) to document `source_keys` as the intentional 4th field in the `apm-agent-allowlist` — it was already implemented in `field-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.
Defame1297 merged commit 50d5c30a3c into main 2026-08-12 11:48:50 +00:00
Defame1297 deleted branch feat/89-apm-native-authoring 2026-08-12 11:48:50 +00:00
Sign in to join this conversation.