Files
holocron/plugins/kyberforge/skills/skill-author/scripts/new-skill.sh
Defame1297 6f6b70781d fix(kyberforge): fix scope walk-up and manifest-parsing bugs from PR #93 review
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
2026-08-11 21:49:38 +00:00

190 lines
6.9 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) — mirrors validate.sh's
# APM_TYPE_RE: an optional quote around the value must be closed by the
# *same* quote character (a mismatched or unterminated quote is rejected,
# not silently stripped), and the value must be followed by whitespace or
# end-of-line so `prompts-only` doesn't false-match on the `prompts` prefix.
# `|| [[ -n "$line" ]]` in the read condition also processes a final line
# that lacks a trailing newline, which `read` alone would otherwise skip.
# Identical to agent-author's new-agent.sh copy of this helper.
is_apm_package_manifest() {
local apm_yml="$1" line
while IFS= read -r line || [[ -n "$line" ]]; do
if [[ "$line" =~ ^type:[[:space:]]*(instructions|skill|hybrid|prompts)([[:space:]]|$) ]]; then
return 0
fi
if [[ "$line" =~ ^type:[[:space:]]*([\"\'])(instructions|skill|hybrid|prompts)([\"\'])([[:space:]]|$) ]] \
&& [[ "${BASH_REMATCH[1]}" == "${BASH_REMATCH[3]}" ]]; then
return 0
fi
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 one space-separated line: mode, then the resolved root.
# ---------------------------------------------------------------------------
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 "package $current"
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 "no-package $current"
return 0
fi
local parent
parent="$(dirname "$current")"
if [[ "$parent" == "$current" ]]; then
echo "no-package $current"
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 single space-separated output line with a
# plain `read` instead (bash 3.2-safe). `read` consumes only one line, so
# mode and path must be on the same line: MODE first (never contains
# whitespace), PKG_ROOT last (safely absorbs a path containing spaces).
WALK_OUTPUT="$(find_package_root "$TARGET_INPUT")"
read -r MODE PKG_ROOT <<< "$WALK_OUTPUT"
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