From 68fa2abb6279398ff84f2eb346a46b31943a7da0 Mon Sep 17 00:00:00 2001 From: Defame1297 Date: Thu, 1 Oct 2026 16:28:15 +0000 Subject: [PATCH] fix(kyberforge): address independent review of #154 Quote the template description so the raw scaffold is valid YAML, reject newline-containing names in new-instructions.sh, correct the empty-compile facts (exit 1, --clean exits 0), and finish removing commit steps from the skill-author, agent-author and forge references. Adds regression tests. Refs #148 Co-Authored-By: Claude Code Claude-Session: https://claude.ai/code/session_01KkT7RSDwDbmrM9T34b6sTi --- .../skills/agent-author/references/create.md | 4 ++-- .../skills/agent-author/references/improve.md | 4 ++-- .../skills/forge/references/author-routes.md | 2 +- .../skills/forge/references/version-bump.md | 2 +- .../assets/templates/instructions.md | 2 +- .../references/target-mapping.md | 2 +- .../scripts/new-instructions.sh | 2 +- .../tests/new-instructions.bats | 20 ++++++++++++++++++- .../skills/skill-author/references/create.md | 4 ++-- .../skills/skill-author/references/improve.md | 4 ++-- .../microsoft-apm/instructions-gotchas.md | 5 ++--- 11 files changed, 34 insertions(+), 17 deletions(-) diff --git a/plugins/kyberforge/.apm/skills/agent-author/references/create.md b/plugins/kyberforge/.apm/skills/agent-author/references/create.md index 758f5dc..4b70cf8 100644 --- a/plugins/kyberforge/.apm/skills/agent-author/references/create.md +++ b/plugins/kyberforge/.apm/skills/agent-author/references/create.md @@ -7,8 +7,8 @@ source_keys: # Creating a new agent -Return to `SKILL.md` Step 4 once Step 3 below is done — validation, the version bump and commit -verification are shared with the improve flow and are not repeated here. +Return to `SKILL.md` Step 4 once Step 3 below is done — validation and the version bump +are shared with the improve flow and are not repeated here. ## Prerequisites diff --git a/plugins/kyberforge/.apm/skills/agent-author/references/improve.md b/plugins/kyberforge/.apm/skills/agent-author/references/improve.md index f3e9813..1c4268d 100644 --- a/plugins/kyberforge/.apm/skills/agent-author/references/improve.md +++ b/plugins/kyberforge/.apm/skills/agent-author/references/improve.md @@ -5,8 +5,8 @@ source_keys: # Improving an existing agent -Return to `SKILL.md` Step 4 once Step 4 below is done — validation, the version bump and commit -verification are shared with the create flow and are not repeated here. +Return to `SKILL.md` Step 4 once Step 4 below is done — validation and the version bump +are shared with the create flow and are not repeated here. ## Step 1 — Verify inputs diff --git a/plugins/kyberforge/.apm/skills/forge/references/author-routes.md b/plugins/kyberforge/.apm/skills/forge/references/author-routes.md index 560ad6b..6cbb6e0 100644 --- a/plugins/kyberforge/.apm/skills/forge/references/author-routes.md +++ b/plugins/kyberforge/.apm/skills/forge/references/author-routes.md @@ -35,7 +35,7 @@ Fall back to an **inline invocation** — same conversation, no subagent — whe ## Two-tier verification -Both author skills already close out with their own inline audit, in the same context as the +The skill and agent author skills already close out with their own inline audit, in the same context as the authoring work: `skill-author` and `agent-author` each invoke `factory-audit` on what they wrote. That is tier one, and forge does not change it. diff --git a/plugins/kyberforge/.apm/skills/forge/references/version-bump.md b/plugins/kyberforge/.apm/skills/forge/references/version-bump.md index 80f9d24..5a5fc25 100644 --- a/plugins/kyberforge/.apm/skills/forge/references/version-bump.md +++ b/plugins/kyberforge/.apm/skills/forge/references/version-bump.md @@ -31,7 +31,7 @@ brief: > "The package at `` gained a new `` (``). Bump the > `version` field in that package's `apm.yml`. Determine whether to bump minor (0.1.0) or patch > (0.0.1) based on whether this is a new capability (minor) or a fix/refactor (patch). Do not -> release or tag — just update `apm.yml` and commit." +> release or tag — just update `apm.yml`." Clean context rather than a fork is the point: the bump decision is made independently, without anchoring on the authoring conversation that just argued for the artifact's significance. diff --git a/plugins/kyberforge/.apm/skills/instructions-author/assets/templates/instructions.md b/plugins/kyberforge/.apm/skills/instructions-author/assets/templates/instructions.md index f8a5fb1..f80567f 100644 --- a/plugins/kyberforge/.apm/skills/instructions-author/assets/templates/instructions.md +++ b/plugins/kyberforge/.apm/skills/instructions-author/assets/templates/instructions.md @@ -1,6 +1,6 @@ --- # Delete these comments once filled in; Copilot receives this file verbatim. -description: FILL IN: one line on what this rule covers. Only Copilot and Cursor keep it. +description: "FILL IN: one line on what this rule covers. Only Copilot and Cursor keep it." applyTo: "FILL IN: quoted glob, e.g. **/*.py" # applyTo is always quoted: an unquoted ** is a YAML alias error and the rule # deploys unscoped. Several globs: "**/*.css,**/*.scss". Delete the line only diff --git a/plugins/kyberforge/.apm/skills/instructions-author/references/target-mapping.md b/plugins/kyberforge/.apm/skills/instructions-author/references/target-mapping.md index 4dee2dd..60ec53d 100644 --- a/plugins/kyberforge/.apm/skills/instructions-author/references/target-mapping.md +++ b/plugins/kyberforge/.apm/skills/instructions-author/references/target-mapping.md @@ -50,7 +50,7 @@ For Claude Code the body's first line or heading is the only descriptive text th - `--target claude` writes `CLAUDE.md`; Gemini writes `GEMINI.md` and `AGENTS.md`; every other target writes `AGENTS.md`. - Compile skips instructions already deployed natively, for Claude, Copilot and Antigravity only. With rules populated, `--target claude` exits 0, prints "produced no output files" and writes nothing. `--force-instructions` (alias `--no-dedup`) overrides. - Cursor, Windsurf, Kiro, Grok, Codex and OpenCode have no dedup: compile writes `AGENTS.md` that repeats rules the tool already loads natively. -- A compile with no instruction primitives exits 0. +- A package with no instruction primitives (skills only) makes plain `apm compile` exit 1 with "No instruction files found"; `apm compile --clean` exits 0. ## Native format facts diff --git a/plugins/kyberforge/.apm/skills/instructions-author/scripts/new-instructions.sh b/plugins/kyberforge/.apm/skills/instructions-author/scripts/new-instructions.sh index 5f8287d..ca034e7 100755 --- a/plugins/kyberforge/.apm/skills/instructions-author/scripts/new-instructions.sh +++ b/plugins/kyberforge/.apm/skills/instructions-author/scripts/new-instructions.sh @@ -40,7 +40,7 @@ fi NAME="$1" ROOT="${2/#\~/$HOME}" -if ! grep -qE '^[a-z0-9]+(-[a-z0-9]+)*$' <<< "$NAME"; then +if [[ ! $NAME =~ ^[a-z0-9]+(-[a-z0-9]+)*$ ]]; then echo "Error: name must use lowercase letters, numbers, and hyphens only." >&2 echo " No leading, trailing, or consecutive hyphens." >&2 echo " Received: '$NAME'" >&2 diff --git a/plugins/kyberforge/.apm/skills/instructions-author/tests/new-instructions.bats b/plugins/kyberforge/.apm/skills/instructions-author/tests/new-instructions.bats index b8b8068..d2531a9 100644 --- a/plugins/kyberforge/.apm/skills/instructions-author/tests/new-instructions.bats +++ b/plugins/kyberforge/.apm/skills/instructions-author/tests/new-instructions.bats @@ -21,7 +21,7 @@ make_package() { fill() { sed -i -E \ -e '/^#/{/^# FILL IN/!d}' \ - -e 's/^description: FILL IN.*/description: Python style rules/' \ + -e 's/^description: .?FILL IN.*/description: Python style rules/' \ -e 's/^applyTo: .*/applyTo: "**\/*.py"/' \ -e 's/^# FILL IN.*/# Python style/' \ -e 's/^- FILL IN.*/- Use type hints./' \ @@ -217,3 +217,21 @@ need_apm() { assert_output --partial "Missing 'description'" assert_output --partial "Empty content" } + +@test "rejects a name containing a newline" { + make_package + run bash "$SCRIPT" $'my-rule\nextra' "$ROOT" + assert_failure + assert_output --partial "lowercase letters" + assert [ ! -d "$ROOT/.apm" ] +} + +@test "the raw scaffold is valid YAML and compiles with no parse warnings" { + need_apm + make_package + run bash "$SCRIPT" my-rule "$ROOT" + assert_success + cd "$ROOT" + run apm compile --dry-run --target claude + refute_output --partial "Failed to parse" +} diff --git a/plugins/kyberforge/.apm/skills/skill-author/references/create.md b/plugins/kyberforge/.apm/skills/skill-author/references/create.md index 9dd569e..8a60ede 100644 --- a/plugins/kyberforge/.apm/skills/skill-author/references/create.md +++ b/plugins/kyberforge/.apm/skills/skill-author/references/create.md @@ -9,8 +9,8 @@ source_keys: # Creating a new skill -Return to `SKILL.md` Step 4 once Step 6 below is done — validation, versioning and commit -verification are shared with the improve flow and are not repeated here. +Return to `SKILL.md` Step 4 once Step 6 below is done — validation and versioning +are shared with the improve flow and are not repeated here. ## Prerequisites diff --git a/plugins/kyberforge/.apm/skills/skill-author/references/improve.md b/plugins/kyberforge/.apm/skills/skill-author/references/improve.md index 93e48dc..ab97fc2 100644 --- a/plugins/kyberforge/.apm/skills/skill-author/references/improve.md +++ b/plugins/kyberforge/.apm/skills/skill-author/references/improve.md @@ -7,8 +7,8 @@ source_keys: # Improving an existing skill -Return to `SKILL.md` Step 4 once Step 4 below is done — validation, versioning and commit -verification are shared with the create flow and are not repeated here. +Return to `SKILL.md` Step 4 once Step 4 below is done — validation and versioning +are shared with the create flow and are not repeated here. ## Step 1 — Verify inputs diff --git a/plugins/kyberforge/docs/research/docs/microsoft-apm/instructions-gotchas.md b/plugins/kyberforge/docs/research/docs/microsoft-apm/instructions-gotchas.md index 1bc11f3..882c0f9 100644 --- a/plugins/kyberforge/docs/research/docs/microsoft-apm/instructions-gotchas.md +++ b/plugins/kyberforge/docs/research/docs/microsoft-apm/instructions-gotchas.md @@ -43,9 +43,9 @@ For cursor, windsurf, kiro, codex (and grok, opencode by source) compile still w For claude, cursor, windsurf, kiro and antigravity, an existing file at the deployed path is replaced without warning. Copilot skips it and asks for `--force`. -### Empty-source compile is not an error +### Empty-source compile -Plain `apm compile` and `--target all` in a project with no instruction primitives print "no source primitives remain" and exit 0. The apm-workflow compile reference currently says this hard-fails with exit 1 and "No instruction files found in .apm/ directory"; that does not hold in 0.28.0 (see contradictions). +In a package with no instruction primitives, plain `apm compile --target claude` prints "No instruction files found in .apm/ directory" and exits 1; `apm compile --clean` exits 0. This matches the apm-workflow compile reference. Exit 0 with no output is a different case: instructions exist but are already deployed natively (above). ## Cursor-specific @@ -66,7 +66,6 @@ Plain `apm compile` and `--target all` in a project with no instruction primitiv | `applyTo` | Listed as required and also as optional | Optional, warning only | | Instruction with no `applyTo` | Folded into compiled root files instead of a per-file rule | Still deployed per-file on every rule-directory target (Claude: no frontmatter; Cursor: description only; Windsurf: `always_on`; Kiro: `always`) and also compiled | | Grok deployed name | `.grok/rules/.md` | `.grok/rules/.instructions.md` | -| Compile with nothing to compile | apm-workflow compile reference: exit 1 with a "No instruction files found" message | Exit 0 | | Compile scope | Docs say compile "only handles instructions" | Consistent for content, but compile also emits GEMINI.md and honours the agents_md mode | | Cursor and Windsurf at user scope | Two fetches of the docs disagreed | Source excludes both at user scope; the source was preferred |