refactor(kyberforge): address #154 review and drop commit steps from author skills

- instructions-author: keep two Gotchas, move the rest to the Step 2 contract and verify.md; add references/content.md on what belongs in an instructions file and tighten the template bullets to match - instructions-author, skill-author, agent-author: remove the commit verification step; committing is out of scope for author skills - skill-author 1.0.6, agent-author 1.0.4 (ADR-0022 patch bumps) Refs #148 Co-Authored-By: Claude Code <[email protected]> Claude-Session: https://claude.ai/code/session_01KkT7RSDwDbmrM9T34b6sTi
2026-10-01 16:19:14 +00:00
1 parent 6328816584
commit d576695bb9
@@ -6,7 +6,7 @@ description: >
  Not read-only review -> `factory-audit`. Not skills -> `skill-author`.
 allowed-tools: Bash Read Write Edit
 metadata:
-  version: "1.0.3"
+  version: "1.0.4"
  category: factory
  source_keys:
    - context7-websites-code-claude
@@ -31,7 +31,7 @@ metadata:

 Signals: grill output, `factory-audit` findings, inline feedback, session context describing what went wrong. With none, ask: "No improvement signals found. Did you mean to create a new agent, or do you have feedback to apply?"

-Read only the reference for the resolved flow. Capture `rtk git log --oneline -1` before touching the filesystem; Step 4 needs it.
+Read only the reference for the resolved flow.

 ## Step 2 — Scope

@@ -61,5 +61,3 @@ At every scope, five tools reach no subagent whatever `tools` says — `AskUserQ
 Invoke `factory-audit` on each file written and resolve every FAIL before reporting done. It checks the field allowlist, name-to-stem match, leftover placeholders and template comments, the description budget and the Copilot body limit — do not hand-check those.

 At plugin/APM scope bump the resolved package's `apm.yml` `version` — **minor** on create, **patch** on improve — because consumers compare it to detect updates. Project and user scope have no manifest.
-
-**Commit verification.** 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 lost if the tree is cleaned up. Report done only once the hash has changed.
@@ -20,9 +20,6 @@ metadata:

 - Claude Code drops `description`; only Copilot and Cursor keep it. Write a body that explains itself.
 - Quote every `applyTo`. An unquoted `**/*.py` fails to parse, compile skips the file, and `apm install` still deploys it with no `paths:`, so it loads in every session and nothing errors.
- `apm compile --validate` always exits 0 and hides the missing-`description`, missing-`applyTo` and empty-body warnings. It is not a lint gate.
- Once rules sit in `.claude/rules/`, `apm compile --target claude` writes no `CLAUDE.md` and still exits 0; an exit-code check verifies nothing.
- A source must be flat in `.apm/instructions/` and end `.instructions.md`; anything else is ignored or never installed.

 ## Step 1 — Dispatch

@@ -34,7 +31,7 @@ metadata:

 Signals: grill output, audit findings, inline feedback, a session describing a rule that loaded when it should not or failed to load. With none, ask whether the user meant to create a new file or has feedback to apply.

-Read only the reference for the resolved flow. Capture `rtk git log --oneline -1` before touching the filesystem; Step 3 needs it.
+Read only the reference for the resolved flow.

 ## Step 2 — Contract

@@ -42,8 +39,9 @@ Gates on every file, whichever flow wrote it:

 - **One topic per file.** Two topics are two files.
 - **Scope.** Omit `applyTo` only for a rule that must load in every session, and tell the user it then costs context at every launch.
+- **Source.** Flat in `.apm/instructions/`, named `<stem>.instructions.md`. Anything nested or misnamed is ignored or never installed.
 - **Stem.** It becomes the deployed filename, and install overwrites a hand-authored rule of the same name on most targets without a prompt. Check for a collision before choosing it.
- **Body.** Bullets, paths in backticks, nothing assuming another file is loaded, under 200 lines.
+- **Body.** Concrete, checkable bullets, paths in backticks, nothing assuming another file is loaded, under 200 lines. Whether the content belongs in an instructions file at all: read `references/content.md`.

 If a field, glob or location is in question, read `references/schema.md`. If the question is which target keeps which field, or what compile does, read `references/target-mapping.md`.

@@ -53,5 +51,3 @@ If a field, glob or location is in question, read `references/schema.md`. If the
 - [ ] Bump the owning package's `apm.yml` `version` — **minor** on create, **patch** on improve — because consumers compare it to detect updates.

 `factory-audit` has no instructions checks yet, so nothing else gates the file; report only what the verification showed.
-
-**Commit verification.** Once verification is clean, run `rtk git add` and `rtk git commit`. 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 lost if the tree is cleaned up. Report done only once the hash has changed.
@@ -2,4 +2,4 @@

 ## templates/

- **`instructions.md`** — minimal valid `.apm/instructions/<name>.instructions.md`, copied by `scripts/new-instructions.sh`. Carries a `description`, a quoted `applyTo` and a one-topic body, each marked `FILL IN:`. The `applyTo` comment is the only guidance it carries; field semantics are in `references/schema.md`.
+- **`instructions.md`** — minimal valid `.apm/instructions/<name>.instructions.md`, copied by `scripts/new-instructions.sh`. Carries a `description`, a quoted `applyTo` and a one-topic body, each marked `FILL IN:`. The bullets model a checkable rule; what belongs in the body is in `references/content.md`, field semantics in `references/schema.md`.
@@ -8,4 +8,5 @@ applyTo: "FILL IN: quoted glob, e.g. **/*.py"
 ---
 # FILL IN: one topic per file

- FILL IN: the first rule, stated as a bullet.
+- FILL IN: a concrete rule an agent can check, e.g. "Use 2-space indentation", not "Format code properly".
+- FILL IN: a convention that differs from the tool's default, or a pitfall with the reason for it.
@@ -0,0 +1,41 @@
+---
+source_keys:
+  - claude-code-memory-docs
+---
+
+# What belongs in an instructions file
+
+Reached from `SKILL.md` Step 2. Claude reads instructions as context, not as enforced configuration, so a rule only helps if it is specific, short and not contradicted elsewhere.
+
+## Write rules an agent can check
+
+| Weak | Checkable |
+|---|---|
+| Format code properly | Use 2-space indentation |
+| Test your changes | Run `npm test` before committing |
+| Keep files organized | API handlers live in `src/api/handlers/` |
+
+Group related bullets under a short heading. Give the reason when a rule looks arbitrary; a rule with a stated reason survives the edge case.
+
+## Keep
+
+- Conventions that differ from the tool's default.
+- Pitfalls the agent would walk into, with the reason.
+- Build, test and lint commands; where things live when a path cannot be guessed.
+
+## Cut
+
+- What the agent can read from the code: directory listings, dependency lists, architecture overviews.
+- Anything stated in another file that loads alongside this one. Two copies drift, and contradictory rules are followed arbitrarily.
+- Generalities ("write clean code").
+
+## Right artifact?
+
+| The content is | Put it in |
+|---|---|
+| A rule for part of the codebase | This file, with a quoted `applyTo` |
+| A rule for every session | This file without `applyTo`, or `AGENTS.md` (`agentsmd-author`) |
+| A multi-step procedure or one task's guidance | A skill (`skill-author`) |
+| Something that must run at a fixed point or be blocked | A hook, or a `permissions.deny` setting; an instruction is not enforcement |
+
+If the answer is not this file, say so to the user and stop; do not bend the content into a rule.
@@ -34,6 +34,6 @@ Replace every `FILL IN:` and delete the template's comments.

 - `applyTo`: quoted. Omit it only for a rule that must load in every session, and say so to the user; it costs context at every launch.
 - `description`: one line. Write the body as if it were absent, because Claude Code never sees it.
- Body: bullets, one topic, paths in backticks, nothing that assumes another file is loaded.
+- Body: concrete bullets, one topic, paths in backticks, nothing that assumes another file is loaded. Read `references/content.md` if unsure the content belongs in an instructions file.

 For glob syntax or a field question, read `references/schema.md`.
@@ -38,8 +38,8 @@ source_keys:

 - **URL:** https://code.claude.com/docs/en/memory
 - **Research doc:** plugins/kyberforge/docs/research/docs/microsoft-apm/sources.md (digest: plugins/kyberforge/docs/research/docs/microsoft-apm/instructions-target-mapping.md)
- **Description:** Claude Code memory documentation: `.claude/rules/` loading, the `paths` field as the only field read, invalid YAML ignored, size guidance.
- **Contributing files:** SKILL.md, references/schema.md, references/target-mapping.md
+- **Description:** Claude Code memory documentation: `.claude/rules/` loading, the `paths` field as the only field read, invalid YAML ignored, size and specificity guidance, instructions versus skills and hooks.
+- **Contributing files:** SKILL.md, references/schema.md, references/target-mapping.md, references/content.md
 - **Status:** `extracted`

 ## github-copilot-custom-instructions-docs
@@ -7,7 +7,7 @@ source_keys:

 Reached from `SKILL.md` Step 3. Run step 2 outside the repo: `apm install` writes `apm_modules/`, `apm.lock.yaml` and a rules directory, and install overwrites hand-authored rule files without warning.

-1. From the package root, a real compile, never `--validate`:
+1. From the package root, a real compile. `--validate` always exits 0 and hides the missing-`description`, missing-`applyTo` and empty-body warnings, so it verifies nothing:

   ```bash
   apm compile --dry-run --target claude
@@ -27,6 +27,6 @@ Reached from `SKILL.md` Step 3. Run step 2 outside the repo: `apm install` write

 3. The deployed file must open with `paths:` listing the intended globs. No frontmatter block at all means `applyTo` was missing or did not parse: the rule would load in every session.

-4. To check the compiled root file instead, compile in that same clean directory *before* installing, or pass `--force-instructions`; after an install, `--target claude` writes nothing.
+4. Once rules sit in `.claude/rules/`, `apm compile --target claude` writes no `CLAUDE.md` and still exits 0, so an exit-code check proves nothing. To check the compiled root file instead, compile in that same clean directory *before* installing, or pass `--force-instructions`; after an install, `--target claude` writes nothing.

 Delete the directory afterwards. Report only what was observed; Cursor's list-form `globs` and the Windsurf, Kiro and Antigravity runtimes stay unverified.
@@ -6,7 +6,7 @@ description: >
  Not read-only review -> `factory-audit`. Not agent files -> `agent-author`.
 allowed-tools: Bash Read Write Edit
 metadata:
-  version: "1.0.5"
+  version: "1.0.6"
  category: factory
  source_keys:
    - agentskills-home
@@ -34,7 +34,7 @@ metadata:

 Signals: grill output, `/factory-audit` findings, inline feedback, eval results, session context describing what went wrong. With none, ask whether the user meant to create a new skill or has feedback to apply.

-Read only the reference matching the resolved flow — each is self-contained. If the target sits inside a git worktree, capture `rtk git log --oneline -1` before touching the filesystem; Step 4 needs it.
+Read only the reference matching the resolved flow — each is self-contained.

 ## Step 2 — Invocation axis

@@ -58,5 +58,3 @@ 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.

 Bump `metadata.version`: the **minor** version on create (new skills start at `0.1.0`) and the **patch** version on improve.
-
-**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.
@@ -80,3 +80,14 @@ An earlier version of this topic's schema file described missing `description` a
 - Windsurf user-scope global rules.
 - The Context7 step was unavailable (invalid API key), so the registry carries no fresh Context7 pull. Doc pages were summarised by a smaller model before reaching this file and can be lossy.
 - Apm versions other than 0.28.0 were not tested.
+
+## What belongs in an instruction body
+
+From the Claude Code memory docs (`claude-code-memory-docs`): instructions reach Claude as context, not enforced configuration, so adherence rises with specificity and falls with length and contradiction.
+
+- Write rules concrete enough to verify: "Use 2-space indentation", "Run `npm test` before committing", "API handlers live in `src/api/handlers/`", not "Format code properly" or "Keep files organized".
+- Keep to facts Claude should hold every session: build commands, conventions, layout, "always do X" rules. The `/doctor` trim check cuts what Claude can derive from the codebase (directory layouts, dependency lists, architecture overviews) and keeps pitfalls, rationale and conventions that differ from tool defaults.
+- A multi-step procedure, or guidance that matters for one task, belongs in a skill. Guidance that matters for one part of the codebase belongs in a path-scoped rule.
+- Something that must happen at a fixed point (before every commit) or must be blocked is a hook or a `permissions.deny` entry, never an instruction: "Settings rules are enforced by the client regardless of what Claude decides to do. CLAUDE.md instructions shape Claude's behavior but are not a hard enforcement layer."
+- Two instructions that contradict each other make Claude pick one arbitrarily, across user and project files and across rules.
+- Under 200 lines per file; one topic per file.