diff --git a/plugins/kyberforge/.apm/skills/skill-author/SKILL.md b/plugins/kyberforge/.apm/skills/skill-author/SKILL.md index b3b356f..a310423 100644 --- a/plugins/kyberforge/.apm/skills/skill-author/SKILL.md +++ b/plugins/kyberforge/.apm/skills/skill-author/SKILL.md @@ -57,6 +57,6 @@ Gates `/factory-audit` enforces in both flows: Run `/factory-audit` on the resolved skill directory; resolve every FAIL before reporting done. It checks name-to-directory match, placeholders, both size budgets, boundary-target resolution and script hygiene — do not hand-check those. Hand-check the one thing it misses: an empty body reports `PASS SKILL.md body word count 0 (ADR-0020 target: 600)`, so confirm at least one non-empty section exists. -Versioning: on create, keep the scaffold's `0.1.0` — do not bump it (ADR-0022); on improve, bump the **patch** version. +Versioning: on create, keep the scaffold's `0.1.0` — do not bump it (ADR-0022, Decision: `0.1.0` means "created and never yet revised"); on improve, bump the **patch** version. **Commit verification.** Inside a git worktree: once the audit is clean, run `rtk git add` and `rtk git commit` — do not stop at staging. Re-run `rtk git log --oneline -1` and confirm the hash changed from Step 1's. A non-empty `git diff --stat` is not proof: staged-but-uncommitted work is part of no commit and is silently lost if the tree is cleaned up. Report done only once the hash has changed. Outside a worktree (a skill under `~/.claude/skills/`, say) nothing is committable — report done on a clean audit, naming that as the reason. diff --git a/plugins/kyberforge/.apm/skills/skill-author/references/deployment-modes.md b/plugins/kyberforge/.apm/skills/skill-author/references/deployment-modes.md index 8255e03..37cae74 100644 --- a/plugins/kyberforge/.apm/skills/skill-author/references/deployment-modes.md +++ b/plugins/kyberforge/.apm/skills/skill-author/references/deployment-modes.md @@ -1,6 +1,7 @@ --- source_keys: - agentskills-spec + - apm-docs-llms-full --- # Deployment Modes @@ -11,7 +12,7 @@ Skills deploy standalone, or as part of an APM package (an `apm.yml`-governed `. When a host installs a plugin, it copies the plugin directory to a cache. Only the plugin's own files are copied. **Any path that leaves the skill directory breaks post-install:** -``` +```text ../other-skill/validate.sh # breaks plugins//.apm/skills/other/ # breaks ../../shared/utils.sh # breaks @@ -23,7 +24,7 @@ Fix: duplicate the file into the skill's own `scripts/` or `assets/`. There is n For a package (an `apm.yml`-governed `.apm/` source tree), the deployable artifact is generated by `apm compile` per target harness — not produced by copying the raw `.apm/` directory wholesale the way a plugin cache install copies a plugin directory. The same self-containment rule still applies at the skill level: **file references inside `.apm/skills//` must not reach outside that skill's own directory.** -``` +```text ../other-skill/validate.sh # breaks .apm/skills/other-skill/ # breaks ../../shared/utils.sh # breaks diff --git a/plugins/kyberforge/.apm/skills/skill-author/references/sources.md b/plugins/kyberforge/.apm/skills/skill-author/references/sources.md index 2d169d7..c899ba9 100644 --- a/plugins/kyberforge/.apm/skills/skill-author/references/sources.md +++ b/plugins/kyberforge/.apm/skills/skill-author/references/sources.md @@ -7,6 +7,7 @@ source_keys: - agentskills-evaluating-skills - agentskills-using-scripts - agentskills-quickstart + - apm-docs-llms-full --- # Sources @@ -68,3 +69,11 @@ source_keys: - **Description:** Step-by-step guide to creating a first skill (roll-dice example), how discovery/activation/execution work in practice - **Contributing files:** SKILL.md, references/create.md - **Status:** `extracted` + +## apm-docs-llms-full + +- **URL:** https://microsoft.github.io/apm/llms-full.txt +- **Research doc:** plugins/kyberforge/docs/research/docs/microsoft-apm/sources.md +- **Description:** Published apm docs bundle — `apm compile` producing per-target output from an `apm.yml` package, the target-neutral `${PLUGIN_ROOT}` hook token apm rewrites per target, and hooks as primitives under `.apm/hooks/` +- **Contributing files:** references/deployment-modes.md +- **Status:** `extracted` diff --git a/plugins/kyberforge/.apm/skills/skill-author/scripts/new-skill.sh b/plugins/kyberforge/.apm/skills/skill-author/scripts/new-skill.sh index 4c1b4da..6a28d2b 100755 --- a/plugins/kyberforge/.apm/skills/skill-author/scripts/new-skill.sh +++ b/plugins/kyberforge/.apm/skills/skill-author/scripts/new-skill.sh @@ -42,7 +42,8 @@ Output: Exit codes: 0 Scaffold created, destination already complete (no-op), or a partial scaffold from an earlier failed run repaired - 1 Invalid arguments, missing path, or templates not found + 1 Invalid arguments, missing path, templates not found, name + substitution failed, or the destination appeared mid-build EOF } @@ -162,41 +163,60 @@ SUBST_MARKERS=("name: SKILL_NAME" "bats /SKILL_NAME/tests/") # Replace SKILL_NAME in one file. `sed -i` is not portable — GNU takes an # optional attached suffix, BSD/macOS requires a separate suffix argument and # reads the expression as one — so write to a temp file and move it over. +# The move runs only if sed succeeded: a failed sed leaves an empty or partial +# temp file, and moving that over the original would destroy it. substitute_file() { local f="$1" - sed "s/SKILL_NAME/$SKILL_NAME/g" "$f" > "$f.tmp" - mv "$f.tmp" "$f" + if sed "s/SKILL_NAME/$SKILL_NAME/g" "$f" > "$f.tmp" && mv "$f.tmp" "$f"; then + return 0 + fi + rm -f "$f.tmp" + echo "Error: could not substitute the skill name in '$f'." >&2 + return 1 } # Substitute every placeholder file under dir $1 (a fresh template copy). substitute_name() { local dir="$1" rel for rel in "${SUBST_FILES[@]}"; do - [[ -f "$dir/$rel" ]] && substitute_file "$dir/$rel" + if [[ -f "$dir/$rel" ]]; then + substitute_file "$dir/$rel" || return 1 + fi done return 0 } # Substitute only the placeholder files under dir $1 that still carry their -# template marker line; print how many were repaired. +# template marker line. Sets REPAIRED to how many were repaired; returns 1 on +# the first failure. Called directly, never inside $(...): a command +# substitution would swallow the failure and let the caller report success. +REPAIRED=0 repair_placeholders() { - local dir="$1" i f n=0 + local dir="$1" i f + REPAIRED=0 for i in "${!SUBST_FILES[@]}"; do f="$dir/${SUBST_FILES[$i]}" if [[ -f "$f" ]] && grep -qxF "${SUBST_MARKERS[$i]}" "$f"; then - substitute_file "$f" - n=$((n + 1)) + substitute_file "$f" || return 1 + REPAIRED=$((REPAIRED + 1)) fi done - echo "$n" + return 0 } if [[ -d "$TARGET" ]]; then # A scaffold left half-built by an earlier failed run still carries a # template marker line; finish it instead of reporting a silent no-op. # Anything else — including a complete skill — is left untouched. - if [[ "$(repair_placeholders "$TARGET")" -gt 0 ]]; then - echo "Repaired partial scaffold at '$TARGET' — substituted SKILL_NAME." >&2 + if ! repair_placeholders "$TARGET"; then + echo "Error: repair of '$TARGET' failed; no file was left half-written." >&2 + exit 1 + fi + if [[ "$REPAIRED" -gt 0 ]]; then + # The marker line proves only that the name was never substituted, not + # that the earlier copy finished — a file may still be missing. + echo "Repaired partial scaffold at '$TARGET' — only the name placeholder (SKILL_NAME) was substituted." >&2 + echo "The earlier run may also have left files missing: run /factory-audit on it, or delete it and re-run this script." >&2 exit 0 fi echo "Scaffold already exists at '$TARGET' — nothing to do." >&2 @@ -207,10 +227,23 @@ mkdir -p "$(dirname "$TARGET")" # Build in a sibling staging directory and rename it into place only once # complete, so a failure mid-build never leaves a half-built $TARGET behind. -STAGING="$TARGET.partial.$$" +# The dot prefix matters: a SIGKILL skips the trap, and a leftover must not +# look like a skill to anything scanning .apm/skills/. +STAGING="$(mktemp -d "$(dirname "$TARGET")/.new-skill.XXXXXX")" trap 'rm -rf "$STAGING"' EXIT -cp -r "$TEMPLATES_DIR" "$STAGING" +# mktemp creates the directory 0700; give the skill the umask default instead. +chmod "$(umask -S)" "$STAGING" +cp -R "$TEMPLATES_DIR/." "$STAGING" substitute_name "$STAGING" + +# $TARGET may have appeared since the check above (a concurrent run). `mv` +# onto an existing directory nests the source inside it instead of failing, +# and GNU `mv -T` is not portable, so re-check immediately before the rename. +# This narrows the window to the gap between two syscalls; it does not close it. +if [[ -e "$TARGET" ]]; then + echo "Error: '$TARGET' appeared while the scaffold was being built; left it untouched." >&2 + exit 1 +fi mv "$STAGING" "$TARGET" trap - EXIT diff --git a/plugins/kyberforge/.apm/skills/skill-author/tests/new-skill.bats b/plugins/kyberforge/.apm/skills/skill-author/tests/new-skill.bats index 2455628..2e349c5 100644 --- a/plugins/kyberforge/.apm/skills/skill-author/tests/new-skill.bats +++ b/plugins/kyberforge/.apm/skills/skill-author/tests/new-skill.bats @@ -113,9 +113,90 @@ teardown() { assert_failure } +# Lists every entry in $1 other than my-tool and the fixture copies, so a +# staging directory of any name (dotted or not) left behind is caught. +leftovers() { + local entry name + for entry in "$1"/* "$1"/.[!.]* "$1"/..?*; do + [[ -e "$entry" ]] || continue + name=${entry##*/} + case "$name" in my-tool|bin|skill.orig) ;; *) printf '%s\n' "$name" ;; esac + done +} + +# Puts a `sed` on PATH that fails without writing output, so a test can +# inject a failure into the substitution step. +stub_failing_sed() { + mkdir -p "$DEST/bin" + printf '#!/usr/bin/env bash\nexit 1\n' > "$DEST/bin/sed" + chmod +x "$DEST/bin/sed" +} + @test "leaves no staging directory behind after a successful run" { bash "$SCRIPT" my-tool "$DEST" - run bash -c "ls -d '$DEST'/my-tool.partial.* 2>/dev/null" + run leftovers "$DEST" + assert_output "" +} + +@test "stages the build in a dot-prefixed directory that cannot pass for a skill" { + # A SIGKILL skips the cleanup trap, so the staging directory's name is what + # keeps a leftover from looking like a skill under .apm/skills/. + mkdir -p "$DEST/bin" + real_sed="$(command -v sed)" + printf '#!/usr/bin/env bash\nprintf "%%s\\n" "${@: -1}" >> "%s/sed.log"\nexec "%s" "$@"\n' \ + "$DEST/bin" "$real_sed" > "$DEST/bin/sed" + chmod +x "$DEST/bin/sed" + PATH="$DEST/bin:$PATH" run bash "$SCRIPT" my-tool "$DEST" + assert_success + run grep -c . "$DEST/bin/sed.log" + refute_output "0" + run grep -vE "^$DEST/\.[^/]+/" "$DEST/bin/sed.log" + assert_output "" +} + +@test "scaffold directory gets umask-default permissions, not mktemp's 0700" { + bash "$SCRIPT" my-tool "$DEST" + mkdir "$DEST/reference-dir" + assert_equal "$(ls -ld "$DEST/my-tool" | cut -c1-10)" \ + "$(ls -ld "$DEST/reference-dir" | cut -c1-10)" + rmdir "$DEST/reference-dir" +} + +@test "fresh scaffold: a failing sed aborts non-zero with no target and no staging left" { + stub_failing_sed + PATH="$DEST/bin:$PATH" run bash "$SCRIPT" my-tool "$DEST" + assert_failure + assert [ ! -e "$DEST/my-tool" ] + run leftovers "$DEST" + assert_output "" +} + +@test "repair: a failing sed aborts non-zero and leaves the original file unchanged" { + cp -r "$BATS_TEST_DIRNAME/../assets/templates" "$DEST/my-tool" + cp "$DEST/my-tool/SKILL.md" "$DEST/skill.orig" + stub_failing_sed + PATH="$DEST/bin:$PATH" run bash "$SCRIPT" my-tool "$DEST" + assert_failure + refute_output --partial "Repaired" + run cmp "$DEST/skill.orig" "$DEST/my-tool/SKILL.md" + assert_success + run find "$DEST/my-tool" -name '*.tmp' + assert_output "" +} + +@test "a target that appears after the existence check is not nested into" { + # A sed wrapper creates the target mid-build, standing in for a concurrent + # run winning the race between the check and the final rename. + mkdir -p "$DEST/bin" + real_sed="$(command -v sed)" + printf '#!/usr/bin/env bash\nmkdir -p "%s/my-tool"\nexec "%s" "$@"\n' \ + "$DEST" "$real_sed" > "$DEST/bin/sed" + chmod +x "$DEST/bin/sed" + PATH="$DEST/bin:$PATH" run bash "$SCRIPT" my-tool "$DEST" + assert_failure + run ls -A "$DEST/my-tool" + assert_output "" + run leftovers "$DEST" assert_output "" } @@ -126,6 +207,7 @@ teardown() { run bash "$SCRIPT" my-tool "$DEST" assert_success assert_output --partial "Repaired partial scaffold" + assert_output --partial "only the name placeholder" run grep -r "SKILL_NAME" "$DEST/my-tool" assert_failure run grep -E '^name: my-tool$' "$DEST/my-tool/SKILL.md"