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
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
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.
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
`claude plugin validate --strict` auto-discovers every .md under a
plugin's agents/ directory as an agent requiring frontmatter, so the
provenance file there always needs fake agent frontmatter to pass
validation. Confirmed empirically that an explicit `agents` manifest
array can't suppress this discovery. Move the file to <plugin-root>/
sources.md instead, and update agent-author/agent-audit accordingly.
Adds ADR-0010, partially superseding ADR-0005's `agents/sources.md`
convention. Fixes#63.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
## Why
skill-author explicitly excludes agent definition files ("Do not use to author
agent definition files"). No factory skill existed to create or improve the
.md / .agent.md files that define Claude Code subagents and Copilot CLI agents
in a plugin, project, or user scope. This fills that gap.
## Implementation Notes
Single-root scaffold convention: new-agent.sh <name> <root> derives both
provider file paths from the root by convention — plugin scope (plugin.json
present) writes both files into <root>/agents/; non-plugin scope writes
.claude/agents/<name>.md and .github/agents/<name>.agent.md. This keeps
input minimal while always generating both provider files. See ADR-0015.
Routing is file-level (not directory-level like skill-author): neither file
exists → create flow; at least one exists → improve flow; scaffold is a
file-by-file no-op so retries are safe.
No companion agent-audit skill — inline validation in the close step covers
the simpler agent field contract. agent-audit is tracked as a follow-on.
## Impact
Closes the skill-author gap for agent definitions. Follow-ons tracked in
Gitea #11: agent-audit skill and --copilot-dest override flag for non-standard
Copilot project paths.
---
ADR: docs/adr/0015-agent-author-dual-provider-scaffold.md
Refs: #10
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
