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
188 lines
6.5 KiB
Bash
Executable File
188 lines
6.5 KiB
Bash
Executable File
#!/usr/bin/env bash
|
|
set -euo pipefail
|
|
|
|
SKILL_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
|
TEMPLATES_DIR="$SKILL_DIR/../assets/templates"
|
|
|
|
usage() {
|
|
cat <<EOF
|
|
Usage: new-skill.sh <skill-name> <path>
|
|
|
|
Create a new skill scaffold by copying annotated templates to the resolved
|
|
destination. <path> is any existing path inside or at the target — a
|
|
package or a standalone location. It does not have to be a package root
|
|
itself.
|
|
|
|
The script walks up from <path> to pick one of two modes:
|
|
|
|
Package mode:
|
|
If an apm.yml with a top-level 'type:' field (instructions, skill,
|
|
hybrid, or prompts) is found at or above <path>, the skill is
|
|
scaffolded into <package-root>/.apm/skills/<skill-name>/ — not under
|
|
<path> itself. An apm.yml with no 'type:' field is a marketplace-only
|
|
manifest, not a package; it is skipped and the walk continues upward.
|
|
|
|
Standalone mode:
|
|
If the walk reaches a '.git' directory or the filesystem root without
|
|
finding a type-bearing apm.yml, the skill is scaffolded directly into
|
|
<path>/<skill-name>/, exactly as <path> was given.
|
|
|
|
Arguments:
|
|
skill-name Kebab-case skill identifier (e.g. my-tool, data-analyzer).
|
|
Must match the directory name exactly.
|
|
path Any existing path inside/at the target. Used to locate the
|
|
package (package mode) or as the literal parent directory
|
|
(standalone mode). Must already exist.
|
|
Examples: ~/.agents/skills/ packages/my-pkg/some/subdir/
|
|
|
|
Output:
|
|
Package mode: <package-root>/.apm/skills/<skill-name>/
|
|
Standalone mode: <path>/<skill-name>/
|
|
|
|
Exit codes:
|
|
0 Scaffold created successfully, or destination already exists (no-op)
|
|
1 Invalid arguments, missing path, or templates not found
|
|
EOF
|
|
}
|
|
|
|
if [[ "${1:-}" == "--help" || "${1:-}" == "-h" ]]; then
|
|
usage
|
|
exit 0
|
|
fi
|
|
|
|
if [[ $# -lt 2 ]]; then
|
|
echo "Error: skill-name and path are required." >&2
|
|
echo "" >&2
|
|
usage >&2
|
|
exit 1
|
|
fi
|
|
|
|
SKILL_NAME="$1"
|
|
TARGET_INPUT="$2"
|
|
|
|
# Validate skill name format
|
|
if ! echo "$SKILL_NAME" | grep -qE '^[a-z0-9]+(-[a-z0-9]+)*$'; then
|
|
echo "Error: skill-name must use lowercase letters, numbers, and hyphens only." >&2
|
|
echo " No leading, trailing, or consecutive hyphens." >&2
|
|
echo " Received: '$SKILL_NAME'" >&2
|
|
exit 1
|
|
fi
|
|
|
|
# Validate templates directory exists
|
|
if [[ ! -d "$TEMPLATES_DIR" ]]; then
|
|
echo "Error: templates directory not found at '$TEMPLATES_DIR'." >&2
|
|
echo " Run this script from its original location inside the skill-author skill." >&2
|
|
exit 1
|
|
fi
|
|
|
|
# Validate path exists
|
|
if [[ ! -d "$TARGET_INPUT" ]]; then
|
|
echo "Error: path '$TARGET_INPUT' does not exist." >&2
|
|
exit 1
|
|
fi
|
|
|
|
# True if apm_yml's top-level `type:` line names one of the four APM package
|
|
# types (instructions/skill/hybrid/prompts) — tolerating an optional matching
|
|
# quote around the value and requiring the value end there, so a malformed
|
|
# value like `prompts-only` doesn't false-match on the `prompts` prefix.
|
|
# Identical to agent-author's new-agent.sh copy of this helper.
|
|
is_apm_package_manifest() {
|
|
local apm_yml="$1" line value
|
|
while IFS= read -r line; do
|
|
[[ "$line" =~ ^type:[[:space:]]*(.*)$ ]] || continue
|
|
value="${BASH_REMATCH[1]}"
|
|
value="${value%%[[:space:]]*}"
|
|
value="${value#\"}"; value="${value%\"}"
|
|
value="${value#\'}"; value="${value%\'}"
|
|
case "$value" in
|
|
instructions|skill|hybrid|prompts) return 0 ;;
|
|
esac
|
|
done < "$apm_yml"
|
|
return 1
|
|
}
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Walk up from <path> looking for a type-bearing apm.yml (package mode) or a
|
|
# .git boundary / filesystem root (standalone mode). An apm.yml with no
|
|
# 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() {
|
|
local current
|
|
current="$(cd "$1" && pwd)"
|
|
while true; do
|
|
if [[ -f "$current/apm.yml" ]]; then
|
|
if is_apm_package_manifest "$current/apm.yml"; then
|
|
echo "$current"
|
|
echo "package"
|
|
return 0
|
|
fi
|
|
# apm.yml exists but has no type: field — marketplace-only manifest.
|
|
# Not a package match; keep walking up.
|
|
fi
|
|
# .git is a directory in a normal checkout but a file (`gitdir: ...`) in
|
|
# a git worktree — -e covers both.
|
|
if [[ -e "$current/.git" ]]; then
|
|
echo "$current"
|
|
echo "no-package"
|
|
return 0
|
|
fi
|
|
local parent
|
|
parent="$(dirname "$current")"
|
|
if [[ "$parent" == "$current" ]]; then
|
|
echo "$current"
|
|
echo "no-package"
|
|
return 0
|
|
fi
|
|
current="$parent"
|
|
done
|
|
}
|
|
|
|
# `mapfile`/`readarray` are bash 4.0+ builtins with no fallback on macOS's
|
|
# stock /bin/bash 3.2 — read the two output lines individually instead.
|
|
WALK_OUTPUT="$(find_package_root "$TARGET_INPUT")"
|
|
PKG_ROOT="$(echo "$WALK_OUTPUT" | sed -n '1p')"
|
|
MODE="$(echo "$WALK_OUTPUT" | sed -n '2p')"
|
|
|
|
if [[ "$MODE" == "package" ]]; then
|
|
TARGET="$PKG_ROOT/.apm/skills/$SKILL_NAME"
|
|
else
|
|
TARGET="$TARGET_INPUT/$SKILL_NAME"
|
|
fi
|
|
|
|
# Destination already exists — treat as a no-op so retries are safe
|
|
if [[ -d "$TARGET" ]]; then
|
|
echo "Scaffold already exists at '$TARGET' — nothing to do." >&2
|
|
exit 0
|
|
fi
|
|
|
|
mkdir -p "$(dirname "$TARGET")"
|
|
|
|
# Copy templates to destination
|
|
cp -r "$TEMPLATES_DIR" "$TARGET"
|
|
|
|
# Set skill name in templates
|
|
sed -i "s/SKILL_NAME/$SKILL_NAME/g" "$TARGET/SKILL.md"
|
|
sed -i "s/SKILL_NAME/$SKILL_NAME/g" "$TARGET/README.md"
|
|
sed -i "s/SKILL_NAME/$SKILL_NAME/g" "$TARGET/tests/README.md"
|
|
|
|
if [[ "$MODE" == "package" ]]; then
|
|
echo "Mode: package — type-bearing apm.yml found at '$PKG_ROOT'" >&2
|
|
echo "Scaffold created: $TARGET" >&2
|
|
echo "" >&2
|
|
echo "Note: if '$PKG_ROOT/apm.yml' has an explicit 'includes:' list (not 'auto')," >&2
|
|
echo " add '.apm/skills/$SKILL_NAME/' to it." >&2
|
|
else
|
|
echo "Mode: standalone — no type-bearing apm.yml found above '$TARGET_INPUT'" >&2
|
|
echo "Scaffold created: $TARGET" >&2
|
|
fi
|
|
echo "" >&2
|
|
echo "Next steps:" >&2
|
|
echo " 1. Fill in $TARGET/SKILL.md — replace all FILL IN: placeholders" >&2
|
|
echo " 2. Add scripts to scripts/ if needed (or delete the directory)" >&2
|
|
echo " 3. Add docs to references/ if needed (or delete the directory)" >&2
|
|
echo " 4. Add resources to assets/ if needed (or delete the directory)" >&2
|
|
echo " 5. Add tests to tests/ if the skill has scripts (or delete the directory)" >&2
|
|
echo " 6. Populate references/sources.md with research sources, or delete it" >&2
|
|
echo " 7. Validate: run /skill-audit on $TARGET" >&2
|