From 9ac5340e159b595ec93123746a4991141d536fb0 Mon Sep 17 00:00:00 2001 From: Defame1297 Date: Mon, 28 Sep 2026 17:40:12 +0000 Subject: [PATCH] fix(kyberforge): resolve clean-context audit findings on primitive support primitive-author: - description excludes read-only review (-> factory-audit) - validation Gotcha now matches the research: compile never reads prompts, install fails only on a bad Copilot hook payload and warns on prompt input names and dropped keys - instruction fold-in into AGENTS.md/CLAUDE.md stated as conditional on dedup and --force-instructions - hook checklist gains the wrapped-shape Must, drops hardlinks, notes why executable is stricter than the research, and states the separate Copilot-targeted package route instead of a blanket "don't" - prompt Must 5 keeps the research's Copilot-only-key exception; adds model-slug and 250-char Shoulds; descriptions name skills or agents - placeholder instruction covers both FILL IN and FILL_IN_ tokens factory-audit: hardlink FAIL scoped to instructions and prompts (find_hook_files skips symlinks only), with bats cases; prompt-flow description rubric names skills or agents. forge: version-bump, apm-routes and sources references updated for the primitive route. Refs #94 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KkT7RSDwDbmrM9T34b6sTi --- .../factory-audit/references/prompt-flow.md | 2 +- .../scripts/lib-checks-primitive.sh | 11 ++++--- .../tests/validate-primitive.bats | 25 +++++++++++++++ .../skills/forge/references/apm-routes.md | 2 +- .../.apm/skills/forge/references/sources.md | 2 +- .../skills/forge/references/version-bump.md | 5 +-- .../.apm/skills/primitive-author/SKILL.md | 10 +++--- .../assets/templates/name.prompt.md.template | 2 +- .../primitive-author/references/hook.md | 31 +++++++++++-------- .../references/instruction.md | 7 +++-- .../primitive-author/references/prompt.md | 15 ++++++--- 11 files changed, 76 insertions(+), 36 deletions(-) diff --git a/plugins/kyberforge/.apm/skills/factory-audit/references/prompt-flow.md b/plugins/kyberforge/.apm/skills/factory-audit/references/prompt-flow.md index 4234107..e1ba8ac 100644 --- a/plugins/kyberforge/.apm/skills/factory-audit/references/prompt-flow.md +++ b/plugins/kyberforge/.apm/skills/factory-audit/references/prompt-flow.md @@ -46,7 +46,7 @@ Cite file and line for every finding. **description** -- SUGGESTION: the description does not read as one user-facing action, or does not name the skills the prompt steers. On Claude the description is model-visible and apm drops `disable-model-invocation`, so naming the steered skills keeps the router pointed at the capability rather than the wrapper. +- SUGGESTION: the description does not read as one user-facing action, or does not name the skills or agents the prompt steers. On Claude the description is model-visible and apm drops `disable-model-invocation`, so naming what it steers keeps the router pointed at the capability rather than the wrapper. Then return to `SKILL.md` Step 4, opening the report with this coverage line: diff --git a/plugins/kyberforge/.apm/skills/factory-audit/scripts/lib-checks-primitive.sh b/plugins/kyberforge/.apm/skills/factory-audit/scripts/lib-checks-primitive.sh index 75496cd..452d984 100755 --- a/plugins/kyberforge/.apm/skills/factory-audit/scripts/lib-checks-primitive.sh +++ b/plugins/kyberforge/.apm/skills/factory-audit/scripts/lib-checks-primitive.sh @@ -99,12 +99,15 @@ def read_text(path): return None -def check_not_linked(): - # apm's find_files_by_glob rejects symlinks and hardlinks (link count > 1), - # so a linked file is silently never deployed. +def check_not_linked(hardlinks=True): + # apm's find_files_by_glob (instructions, prompts) rejects symlinks and + # hardlinks (link count > 1); find_hook_files skips symlinks only, so hooks + # pass hardlinks=False. A rejected file is silently never deployed. if os.path.islink(target): fail(f"is a symlink — apm's discovery skips symlinks, so it is never deployed — {fname}") return + if not hardlinks: + return try: if os.stat(target).st_nlink > 1: fail(f"is a hardlink (link count > 1) — apm's discovery rejects hardlinks, so it is never deployed — {fname}") @@ -231,7 +234,7 @@ def check_script(kind_, rel, first, pkg_root, where): def audit_hook(): - check_not_linked() + check_not_linked(hardlinks=False) stem = fname[:-len('.json')] hooks_dir = os.path.basename(parent_dir) if hooks_dir != 'hooks': diff --git a/plugins/kyberforge/.apm/skills/factory-audit/tests/validate-primitive.bats b/plugins/kyberforge/.apm/skills/factory-audit/tests/validate-primitive.bats index c490b79..1f6ec3f 100644 --- a/plugins/kyberforge/.apm/skills/factory-audit/tests/validate-primitive.bats +++ b/plugins/kyberforge/.apm/skills/factory-audit/tests/validate-primitive.bats @@ -165,6 +165,14 @@ teardown() { assert_output --partial "is a symlink" } +@test "hook: a hardlinked hook file is not a FAIL (find_hook_files skips symlinks only)" { + write_hook real.json '{"hooks":{"Stop":[{"hooks":[{"type":"command","command":"true"}]}]}}' + ln "$PKG/.apm/hooks/real.json" "$PKG/.apm/hooks/linked.json" + run bash "$SCRIPT" "$PKG/.apm/hooks/linked.json" + assert_success + refute_output --partial "hardlink" +} + # --------------------------------------------------------------------------- # Instructions # --------------------------------------------------------------------------- @@ -227,6 +235,15 @@ applyTo: "**/*.py"' 'body' assert_output --partial "also exists at the package root" } +@test "instruction: a hardlinked instruction file is a FAIL" { + write_instruction python 'description: x +applyTo: "**/*.py"' 'body' + ln "$PKG/.apm/instructions/python.instructions.md" "$TMPDIR/python.instructions.md" + run bash "$SCRIPT" "$PKG/.apm/instructions/python.instructions.md" + assert_failure + assert_output --partial "is a hardlink" +} + # --------------------------------------------------------------------------- # Prompts # --------------------------------------------------------------------------- @@ -314,3 +331,11 @@ $(printf 'line\n%.0s' $(seq 1 80)) assert_success refute_output --partial "SUGGESTION" } + +@test "prompt: a hardlinked prompt file is a FAIL" { + write_prompt review-pr 'description: Review a pull request with gitea-prs.' 'Review the PR with gitea-prs.' + ln "$PKG/.apm/prompts/review-pr.prompt.md" "$TMPDIR/review-pr.prompt.md" + run bash "$SCRIPT" "$PKG/.apm/prompts/review-pr.prompt.md" + assert_failure + assert_output --partial "is a hardlink" +} diff --git a/plugins/kyberforge/.apm/skills/forge/references/apm-routes.md b/plugins/kyberforge/.apm/skills/forge/references/apm-routes.md index 3e04823..1056baf 100644 --- a/plugins/kyberforge/.apm/skills/forge/references/apm-routes.md +++ b/plugins/kyberforge/.apm/skills/forge/references/apm-routes.md @@ -21,7 +21,7 @@ checkpoints to the user in real time. ## No clean-context recheck, and no automatic audit -Skill and agent routes close with a clean-context audit rerun; these two do not, and the omission +Skill, agent and primitive routes close with a clean-context audit rerun; these two do not, and the omission is deliberate rather than an oversight. Neither artifact type has an audit skill counterpart to re-run, so detaching the route to earn a recheck it would never get buys nothing. diff --git a/plugins/kyberforge/.apm/skills/forge/references/sources.md b/plugins/kyberforge/.apm/skills/forge/references/sources.md index f64666f..cbc37f1 100644 --- a/plugins/kyberforge/.apm/skills/forge/references/sources.md +++ b/plugins/kyberforge/.apm/skills/forge/references/sources.md @@ -28,7 +28,7 @@ - **URL:** https://agentskills.io/specification.md - **Research doc:** plugins/kyberforge/docs/research/docs/agentskillsio/sources.md -- **Description:** Complete SKILL.md format specification — confirms `assets/`, `references/`, and `scripts/` are warranted only by the bulk/reusability of supporting content (large reference material, executable code, templates), not by a skill's category, and forge's per-route procedures are that kind of supporting content — so the spec permits the `references/` split here but does not require it. The warrant is a house decision: dispatch is mandatory at two or more mutually exclusive flows, which forge's four-row table is. `references/sources.md` likewise exists for this repo's own provenance-chain convention (see `CONTEXT.md`), not because the spec requires it. +- **Description:** Complete SKILL.md format specification — confirms `assets/`, `references/`, and `scripts/` are warranted only by the bulk/reusability of supporting content (large reference material, executable code, templates), not by a skill's category, and forge's per-route procedures are that kind of supporting content — so the spec permits the `references/` split here but does not require it. The warrant is a house decision: dispatch is mandatory at two or more mutually exclusive flows, which forge's five-row table is. `references/sources.md` likewise exists for this repo's own provenance-chain convention (see `CONTEXT.md`), not because the spec requires it. - **Contributing files:** SKILL.md - **Status:** `extracted` diff --git a/plugins/kyberforge/.apm/skills/forge/references/version-bump.md b/plugins/kyberforge/.apm/skills/forge/references/version-bump.md index 5c05039..b6ed547 100644 --- a/plugins/kyberforge/.apm/skills/forge/references/version-bump.md +++ b/plugins/kyberforge/.apm/skills/forge/references/version-bump.md @@ -7,8 +7,9 @@ source_keys: Reached from `SKILL.md` Step 3 after a route has finished. A skill route always lands here: `skill-author` moves only a skill's own `metadata.version`, which is not the package manifest's -number, so the package version is still behind when it reports done. `agent-author` bumps the -resolved package's `apm.yml` itself at plugin/APM scope, and `apm-workflow`'s configure flow +number, so the package version is still behind when it reports done. `agent-author` and +`primitive-author` bump the resolved package's `apm.yml` themselves at plugin/APM scope, and +`apm-workflow`'s configure flow carries the same policy — read those routes' output before acting here, because a second bump for one change is wrong. diff --git a/plugins/kyberforge/.apm/skills/primitive-author/SKILL.md b/plugins/kyberforge/.apm/skills/primitive-author/SKILL.md index ccc768c..a101a6e 100644 --- a/plugins/kyberforge/.apm/skills/primitive-author/SKILL.md +++ b/plugins/kyberforge/.apm/skills/primitive-author/SKILL.md @@ -2,8 +2,8 @@ name: primitive-author description: > Use when the user wants an apm hook, instruction or prompt file created, or - audit findings or feedback applied to an existing one. - Not skills -> skill-author. Not agents -> agent-author. + audit findings or feedback applied to an existing one. Not read-only + review -> factory-audit. Not skills -> skill-author. Not agents -> agent-author. allowed-tools: Bash Read Write Edit metadata: version: "0.1.0" @@ -15,8 +15,8 @@ metadata: ## Gotchas -- `apm compile --validate` is not a gate. apm turns every instruction and prompt problem into a warning and exits 0, and `apm install` never validates at all — `/factory-audit` is the only check that fails. -- Never draft with the real suffix outside `.apm//`. apm's local discovery globs `**/*.instructions.md` across the whole tree, so a draft or template named that way anywhere in the repo compiles into `AGENTS.md`. The templates carry a trailing `.template` for this reason; drop it only on the final path. +- `apm compile --validate` is not a gate: it reports instruction problems only as warnings, exits 0, and never reads prompts. `apm install` fails only on a hook payload Copilot would reject, and merely warns on bad prompt input names and dropped keys — `/factory-audit` is the only check that fails on the rest. +- Never draft with the real suffix outside `.apm//`. apm's local discovery globs `**/*.instructions.md` across the whole tree, so a draft or template named that way anywhere in the repo is picked up as a real instruction. The templates carry a trailing `.template` for this reason; drop it only on the final path. - Never hand-write `.claude/settings.json`, even to test a hook. apm owns that file (ADR-0019), overwrites it outright when it is malformed, and `apm audit --ci` fails on anything it would not have written. ## Step 1 — Dispatch @@ -38,7 +38,7 @@ Run the reference's **Gate** section before writing anything. A failed gate stop | Condition | Action | |---|---| -| No file at the target path | Create: copy the reference's template from `assets/templates/`, drop `.template`, fill every `FILL IN`, and apply the reference's checklist | +| No file at the target path | Create: copy the reference's template from `assets/templates/`, drop `.template`, fill every `FILL IN` and `FILL_IN_` placeholder, and apply the reference's checklist | | File exists, at least one signal | Improve: read the whole file, then apply each signal against the reference's checklist | | File exists, no signal | Stop and ask whether the user meant a new file or has feedback to apply | diff --git a/plugins/kyberforge/.apm/skills/primitive-author/assets/templates/name.prompt.md.template b/plugins/kyberforge/.apm/skills/primitive-author/assets/templates/name.prompt.md.template index ff54818..84ff4ee 100644 --- a/plugins/kyberforge/.apm/skills/primitive-author/assets/templates/name.prompt.md.template +++ b/plugins/kyberforge/.apm/skills/primitive-author/assets/templates/name.prompt.md.template @@ -1,5 +1,5 @@ --- -description: "FILL IN: one user-facing action naming the skills it steers" +description: "FILL IN: one user-facing action naming the skills or agents it steers" input: - FILL_IN_name: "FILL IN: what the user supplies" --- diff --git a/plugins/kyberforge/.apm/skills/primitive-author/references/hook.md b/plugins/kyberforge/.apm/skills/primitive-author/references/hook.md index 5888e27..96ecc60 100644 --- a/plugins/kyberforge/.apm/skills/primitive-author/references/hook.md +++ b/plugins/kyberforge/.apm/skills/primitive-author/references/hook.md @@ -50,33 +50,38 @@ target: `${CLAUDE_PLUGIN_ROOT}` also works but ties the source to one harness's name. - **Claude is the verified target.** apm 0.28.0 passes this nested shape to Copilot without reshaping it, and whether Copilot CLI runs nested entries or honours `matcher` is unverified. That - gap is apm's to close — do not work around it with a second, Copilot-flat file. + gap is apm's to close. Per-file target routing is deprecated, so a Copilot-native flat hook + (`bash` / `powershell` / `timeoutSec`) can only live in a separate Copilot-targeted package — hand + that to `apm-workflow` rather than adding a second file here. ## Checklist Must: -1. The file sits directly in `.apm/hooks/`, is not a symlink or hardlink, and parses as a JSON - object. apm skips invalid JSON silently. -2. Every event value is a list of objects, and every nested `hooks` is a list of objects. Anything +1. The file sits directly in `.apm/hooks/`, is not a symlink, and parses as a JSON object. apm + skips invalid JSON silently. +2. Use the wrapped shape `{"hooks": {Event: [...]}}`. If a naked settings slice is used instead, + every top-level value must be a list, with no stray scalar keys anywhere. +3. Every event value is a list of objects, and every nested `hooks` is a list of objects. Anything else fails the Copilot install outright. -3. Event names are PascalCase (`PreToolUse`, `PostToolUse`, `UserPromptSubmit`, `SessionStart`, +4. Event names are PascalCase (`PreToolUse`, `PostToolUse`, `UserPromptSubmit`, `SessionStart`, `Stop`, …). An all-lowercase name (`stop`) never warns and never fires; a camelCase name outside apm's rename map (`userPromptSubmit`) deploys verbatim to Claude and never fires. -4. The script is referenced as `${PLUGIN_ROOT}/…` (package root) or `./…` (hook directory), +5. The script is referenced as `${PLUGIN_ROOT}/…` (package root) or `./…` (hook directory), exists inside the package, and is executable. No absolute path, and no `$` or backtick in the - path itself. A missing script is only a warning at install time. -5. No `hooks-` or `*--hooks` filename. That routing is deprecated; reach belongs to + path itself. A missing script is only a warning at install time. Executable is stricter than the + research's Should: a script invoked as the command's first token fails at runtime without it. +6. No `hooks-` or `*--hooks` filename. That routing is deprecated; reach belongs to `targets:` (see Gate). Should: -6. Every handler sets `"type": "command"` and an explicit `timeout` in seconds — apm passes `type` +7. Every handler sets `"type": "command"` and an explicit `timeout` in seconds — apm passes `type` through but never supplies it. -7. Set `matcher` explicitly on tool events and on `SessionStart` (`startup`, `resume`, …). Omitted, +8. Set `matcher` explicitly on tool events and on `SessionStart` (`startup`, `resume`, …). Omitted, Claude receives `"*"`. -8. Do not author Copilot's flat `bash` / `powershell` / `timeoutSec` keys in a Claude-shaped file; +9. Do not author Copilot's flat `bash` / `powershell` / `timeoutSec` keys in a Claude-shaped file; they render onto Claude as stray keys. -9. Quote a script path that may contain spaces: `"${PLUGIN_ROOT}/scripts/my hook.sh"`. -10. Keep helper files in the hook directory non-JSON. Copilot's loader rejects any bundled `.json` +10. Quote a script path that may contain spaces: `"${PLUGIN_ROOT}/scripts/my hook.sh"`. +11. Keep helper files in the hook directory non-JSON. Copilot's loader rejects any bundled `.json` without a `hooks` key. diff --git a/plugins/kyberforge/.apm/skills/primitive-author/references/instruction.md b/plugins/kyberforge/.apm/skills/primitive-author/references/instruction.md index 38e7068..cbb5114 100644 --- a/plugins/kyberforge/.apm/skills/primitive-author/references/instruction.md +++ b/plugins/kyberforge/.apm/skills/primitive-author/references/instruction.md @@ -17,8 +17,9 @@ glob. On Claude it deploys to `.claude/rules/.md` with `applyTo` renamed t - **A rule for this repo alone** → it belongs in the repo's AGENTS.md, which is the single always-on source. Stop and hand to `agentsmd-author`. - **No file pattern fits** → an instruction without `applyTo` is always-on in every session of - every repo that installs this package, and `apm compile` folds it into the global sections of - `AGENTS.md` and `CLAUDE.md`. Say exactly that to the user and continue only on an explicit yes. + every repo that installs this package, and `apm compile` can fold it into the global sections of + `AGENTS.md` and `CLAUDE.md` (skipped when `.github/instructions/` or `.claude/rules/` is already + populated, unless `--force-instructions`). Say exactly that to the user and continue only on an explicit yes. Legitimate when a package deliberately ships guidance to its consumers; never a default. - **Procedure the agent follows step by step** → a skill. Stop and hand to `skill-author`. - **A rule scoped to a file pattern** → continue. @@ -32,7 +33,7 @@ Must: 1. The path is `.apm/instructions/.instructions.md`, directly in that directory, not a symlink or hardlink. 2. `description` is a non-empty string. apm only warns when it is missing. -3. The body is non-empty. apm deploys an empty rule silently. +3. The body is non-empty after trimming whitespace. apm deploys an empty rule silently. 4. `applyTo` is a non-empty glob or comma-separated list — top-level commas only as separators, alternation inside `{}` (`"**/*.{ts,tsx}"`) — or absent after the Gate's explicit yes. 5. The stem is unique across the package and its dependencies: a `.claude/rules/.md` diff --git a/plugins/kyberforge/.apm/skills/primitive-author/references/prompt.md b/plugins/kyberforge/.apm/skills/primitive-author/references/prompt.md index 17f6ea1..2739601 100644 --- a/plugins/kyberforge/.apm/skills/primitive-author/references/prompt.md +++ b/plugins/kyberforge/.apm/skills/primitive-author/references/prompt.md @@ -23,15 +23,16 @@ worse skill on every harness. afterwards, come back and write it against the new skill. - **A single-intent message the user would otherwise type repeatedly, steering existing skills or agents by name** → continue. Confirm each skill or agent it names exists and is not - `disable-model-invocation: true`, which the model cannot invoke. + `disable-model-invocation: true`: the prompt's body reaches the model, and the model cannot + invoke a skill that sets it, so the steering would dead-end (the same check `factory-audit`'s + prompt flow applies). ## Description contract -One plain, user-facing sentence stating the action and naming the skills it steers — "Review the +One plain, user-facing sentence stating the action and naming the skills or agents it steers — "Review the current PR with `gitea-prs` and `factory-audit`, then summarise the findings." No "Use when" trigger clause and no `Not X -> Y` boundary: on Claude the description is model-visible, and a -trigger clause invites the router to pick the wrapper over the skills it wraps. 250 characters at -most. +trigger clause invites the router to pick the wrapper over the skills it wraps. ## Checklist @@ -50,10 +51,14 @@ Must: 4. Every `${input:x}` in the body is declared in `input:`, and every declared name is used. Without `input:`, no `${input:…}` may appear — it would reach Claude unrewritten. 5. Frontmatter keys stay within `description`, `allowed-tools`, `model`, `argument-hint` and - `input`. Claude drops everything else with only a warning. + `input`. Claude drops everything else with only a warning. The one exception: a Copilot-only key + (`agent`, `tools`, …) that is intended, with its Claude drop accepted and said so. Should: 6. Spell keys in kebab-case — `allowed-tools`, `argument-hint` — not the camelCase aliases. 7. Omit `argument-hint` when `input:` is set; apm synthesises ` ` from the input names. 8. Keep one intent per prompt, and write the body as second-person instructions. +9. Keep `description` to 250 characters or fewer. +10. Give `model` a slug the target accepts. Copilot ignores `allowed-tools` and `model`, so neither + constrains a Copilot run.