fix(kyberforge): resolve second round of PR #91 review findings
Two review passes on PR #91 (correctness + simplification) found the
prior fixup commit (d6fd9b6) left the deprecation notice it added
half-applied, cited a nonexistent ADR, and introduced/compounded a
few duplicate restatements of the same facts across apm-workflow's
skill files and apm-orchestrate's Hard rules.
- plugin-author/SKILL.md cited ADR-0016 (doesn't exist) instead of
ADR-0006 for the version-parity invariant.
- The deprecation banner on plugin-author/marketplace-author wasn't
reflected in their routing `description:` frontmatter, forge's
routing table, either kyberforge README skill table, or CONTEXT.md
— now propagated consistently; routing behavior is unchanged since
issue #90 (repo-wide apm conversion) hasn't landed yet.
- apm-orchestrate's `configure` operation only covered scaffolding a
new package, not editing an existing apm.yml (e.g. removing a
marketplace.packages[] entry, which has no dedicated `apm` CLI verb
and is just a manifest edit). Added `edit-config`, intentionally
ungated — a local file edit isn't the same class of irreversible
action as `apm publish`, and normal review already gates the commit.
- Deduplicated facts restated 3-4x across SKILL.md/references/ADR
boundaries (audit semantics, reserved name prefixes, marketplace-add
direction, registries precondition), and trimmed configure.md's
~55-line apm.yml schema dump — copied near-verbatim from the
research doc — down to a compact key list with a pointer, matching
install.md's existing terse cross-referencing style.
Deliberately left as-is: apm-orchestrate's parallel/sequential
fan-out logic stays inline rather than extracted to a shared
reference, since neither git-orchestrate nor gitea-orchestrate need
it yet — premature extraction for a single caller.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186ERbyACLRuRxPRnqwpa4m
This commit is contained in:
@@ -46,8 +46,10 @@ copilot plugin install ./plugins/kyberforge
|
||||
| `agent-author` | Author an agent definition file |
|
||||
| `pc-author` | Create, add, remove, and configure `.pre-commit-config.yaml` |
|
||||
| `pc-run` | Install, run, autoupdate, and maintain the pre-commit setup |
|
||||
| `plugin-author` | Create, update, and release plugin scaffolds for the holocron marketplace |
|
||||
| `marketplace-author` | Add, remove, and update plugin entries in the holocron marketplace manifest |
|
||||
| `plugin-author` | (Deprecated — see apm-workflow) Create, update, and release plugin scaffolds for the holocron marketplace |
|
||||
| `marketplace-author` | (Deprecated — see apm-workflow) Add, remove, and update plugin entries in the holocron marketplace manifest |
|
||||
| `apm-install` | Install or upgrade the apm CLI and set up the agent runtimes it drives (Copilot CLI, Codex, Gemini, generic llm) |
|
||||
| `apm-workflow` | Author apm.yml, scaffold an apm package/marketplace, install dependencies, and compile/pack/publish/audit apm content |
|
||||
|
||||
## Author
|
||||
|
||||
|
||||
@@ -20,10 +20,10 @@ You resolve the package root once per dispatched operation (the directory contai
|
||||
|
||||
These are non-negotiable regardless of `confirm` or any skill-local override:
|
||||
- `apm publish` claims a version on a registry — treat it as irreversible. Refuse without explicit `confirm: true`; always dispatch with `--dry-run -v` first and surface that output to the caller before the real publish, even when `confirm: true` was given.
|
||||
- `apm marketplace add` (registering a marketplace as a consumer) and `apm marketplace package add` (registering a local package into a marketplace being built) are opposite directions — resolve which one the caller means from the operation name, never guess from context alone.
|
||||
- Never guess the marketplace-add direction from context — resolve strictly from the operation name (`add-package` vs `add-marketplace`); see apm-workflow/SKILL.md Gotchas for why the two are easy to conflate.
|
||||
- `apm.yml`'s `type:` field constrains what `.apm/` may contain — when scaffolding (`init-package`), set `type:` before any primitive content is added; do not defer it.
|
||||
- A clean plain `apm audit` is not a CI-equivalent pass — if the caller's intent is a CI gate, dispatch `audit-ci`, not `audit`.
|
||||
- `apm experimental enable registries` must have already run before any `registry.*` config takes effect — check this precondition before dispatching an operation that depends on a named registry, and fail with a clear diagnostic rather than silently no-op'ing like apm itself does.
|
||||
- Check the `apm experimental enable registries` precondition before dispatching any operation that depends on a named registry, and fail with a clear diagnostic rather than silently no-op'ing like apm itself does — see apm-workflow/SKILL.md Gotchas for the underlying constraint.
|
||||
|
||||
When invoked, you:
|
||||
1. Parse the incoming workflow request (operation type, parameters, target package(s), context overrides)
|
||||
@@ -37,7 +37,7 @@ When invoked, you:
|
||||
## Inputs
|
||||
|
||||
- **operation:** string, one of:
|
||||
- configure: init-package
|
||||
- configure: init-package, edit-config (→ author/edit an existing package's `apm.yml` directly — adding a dependency, script, registries block, or removing a `marketplace.packages[]` entry; not a distinct `apm` CLI verb, just a manifest edit, optionally followed by `compile: pack` if it affects a published marketplace listing)
|
||||
- marketplace: init-marketplace, check-marketplace, add-package (→ `apm marketplace package add` — register a local package into a marketplace being built), add-marketplace (→ `apm marketplace add` — register a marketplace as a consumer)
|
||||
- install: install (→ `apm install [PACKAGE_REF]` — resolve/fetch dependencies declared in `apm.yml` against `apm.lock.yaml`; no arguments re-resolves everything)
|
||||
- compile: compile, pack, publish, run-script
|
||||
|
||||
@@ -20,10 +20,10 @@ You resolve the package root once per dispatched operation (the directory contai
|
||||
|
||||
These are non-negotiable regardless of `confirm` or any skill-local override:
|
||||
- `apm publish` claims a version on a registry — treat it as irreversible. Refuse without explicit `confirm: true`; always dispatch with `--dry-run -v` first and surface that output to the caller before the real publish, even when `confirm: true` was given.
|
||||
- `apm marketplace add` (registering a marketplace as a consumer) and `apm marketplace package add` (registering a local package into a marketplace being built) are opposite directions — resolve which one the caller means from the operation name, never guess from context alone.
|
||||
- Never guess the marketplace-add direction from context — resolve strictly from the operation name (`add-package` vs `add-marketplace`); see apm-workflow/SKILL.md Gotchas for why the two are easy to conflate.
|
||||
- `apm.yml`'s `type:` field constrains what `.apm/` may contain — when scaffolding (`init-package`), set `type:` before any primitive content is added; do not defer it.
|
||||
- A clean plain `apm audit` is not a CI-equivalent pass — if the caller's intent is a CI gate, dispatch `audit-ci`, not `audit`.
|
||||
- `apm experimental enable registries` must have already run before any `registry.*` config takes effect — check this precondition before dispatching an operation that depends on a named registry, and fail with a clear diagnostic rather than silently no-op'ing like apm itself does.
|
||||
- Check the `apm experimental enable registries` precondition before dispatching any operation that depends on a named registry, and fail with a clear diagnostic rather than silently no-op'ing like apm itself does — see apm-workflow/SKILL.md Gotchas for the underlying constraint.
|
||||
|
||||
When invoked, you:
|
||||
1. Parse the incoming workflow request (operation type, parameters, target package(s), context overrides)
|
||||
@@ -37,7 +37,7 @@ When invoked, you:
|
||||
## Inputs
|
||||
|
||||
- **operation:** string, one of:
|
||||
- configure: init-package
|
||||
- configure: init-package, edit-config (→ author/edit an existing package's `apm.yml` directly — adding a dependency, script, registries block, or removing a `marketplace.packages[]` entry; not a distinct `apm` CLI verb, just a manifest edit, optionally followed by `compile: pack` if it affects a published marketplace listing)
|
||||
- marketplace: init-marketplace, check-marketplace, add-package (→ `apm marketplace package add` — register a local package into a marketplace being built), add-marketplace (→ `apm marketplace add` — register a marketplace as a consumer)
|
||||
- install: install (→ `apm install [PACKAGE_REF]` — resolve/fetch dependencies declared in `apm.yml` against `apm.lock.yaml`; no arguments re-resolves everything)
|
||||
- compile: compile, pack, publish, run-script
|
||||
|
||||
@@ -27,5 +27,7 @@ authoring standard — trigger descriptions and body discipline are required.
|
||||
| `agent-author` | Author an agent definition file |
|
||||
| `pc-author` | Create, add, remove, and configure `.pre-commit-config.yaml` |
|
||||
| `pc-run` | Install, run, autoupdate, and maintain the pre-commit setup |
|
||||
| `plugin-author` | Create, update, and release plugin scaffolds for the holocron marketplace |
|
||||
| `marketplace-author` | Add, remove, and update plugin entries in the holocron marketplace manifest |
|
||||
| `plugin-author` | (Deprecated — see apm-workflow) Create, update, and release plugin scaffolds for the holocron marketplace |
|
||||
| `marketplace-author` | (Deprecated — see apm-workflow) Add, remove, and update plugin entries in the holocron marketplace manifest |
|
||||
| `apm-install` | Install or upgrade the apm CLI and set up the agent runtimes it drives (Copilot CLI, Codex, Gemini, generic llm) |
|
||||
| `apm-workflow` | Author apm.yml, scaffold an apm package/marketplace, install dependencies, and compile/pack/publish/audit apm content |
|
||||
|
||||
@@ -24,7 +24,6 @@ metadata:
|
||||
- `apm.yml`'s `type:` field (`instructions`, `skill`, `hybrid`, `prompts`) constrains what `.apm/` may contain — set it before scaffolding content, not after. Changing it later doesn't retroactively validate what's already on disk.
|
||||
- `includes: auto` publishes the authoritative local layout as-is. Anything narrower needs an explicit repo-path list — don't assume `auto` means "scoped down to what's relevant."
|
||||
- `apm marketplace add` (registering a marketplace as a *consumer*, pointing at someone else's catalog) and `apm marketplace package add` (registering a local package *into* a marketplace you're building) are opposite directions of the same command family — don't conflate them.
|
||||
- `apm pack` is the same command that both bundles a distributable artifact *and* emits `.claude-plugin/marketplace.json` as one of its compile targets — regenerating the Claude Code-native manifest isn't a separate step from packing.
|
||||
- MCP server secrets (headers, env vars) inside `apm.yml` must use `${VAR}` indirection, never literal values, so they're resolved at install/runtime and never committed to the manifest.
|
||||
- `apm experimental enable registries` must run before any `registry.*` config takes effect. Declaring a `registries:` block or running `apm config set registry.*` without it silently does nothing — no error, no warning.
|
||||
- Plain `apm audit` and `apm audit --ci` check different things: plain `apm audit` scans deployed files for hidden Unicode only; `--ci` additionally runs lockfile-consistency checks, install-replay drift detection, and org policy checks. A clean plain `apm audit` is not a CI-equivalent pass.
|
||||
|
||||
@@ -14,7 +14,7 @@ apm audit --ci # CI gate: lockfile consistency + drift replay + pol
|
||||
apm audit --file <path> # standalone: scan an arbitrary file
|
||||
```
|
||||
|
||||
`apm audit --ci` runs baseline lockfile checks, install-replay drift detection (does a clean `apm install` reproduce what's on disk), and org policy checks. Exit code `0` on success, `1` on any violation — composes as a normal CI gate step. It covers AI agent configuration integrity, hidden-content scanning, lockfile verification, and policy enforcement — it does not replace general lint/test/security-scan CI steps, it sits alongside them.
|
||||
Exit code `0` on success, `1` on any violation — composes as a normal CI gate step. It does not replace general lint/test/security-scan CI steps, it sits alongside them.
|
||||
|
||||
## Policy checks
|
||||
|
||||
|
||||
@@ -21,73 +21,21 @@ name: my-pkg
|
||||
version: 1.0.0
|
||||
```
|
||||
|
||||
## `apm.yml` — full schema
|
||||
## `apm.yml` — top-level keys
|
||||
|
||||
```yaml
|
||||
name: my-pkg
|
||||
version: 1.0.0
|
||||
description: Code review skills for Python services
|
||||
author: Jane Doe # plain string, or {name, email?, url?} object
|
||||
license: MIT
|
||||
homepage: https://example.com/my-pkg
|
||||
repository: https://github.com/org/my-pkg
|
||||
keywords: [ai, review, python]
|
||||
- `name`, `version` — required (see above)
|
||||
- `description`, `author`, `license`, `homepage`, `repository`, `keywords` — standard package metadata
|
||||
- `type` — `instructions | skill | hybrid | prompts`; constrains `.apm/` contents
|
||||
- `targets` — which harnesses this package compiles to (plural list form preferred; legacy singular `target: copilot,claude` CSV form still accepted)
|
||||
- `includes` — `auto` publishes the authoritative local layout as-is, or list explicit repo paths
|
||||
- `dependencies`/`devDependencies` — `apm`/`mcp`/`lsp` entries; `devDependencies` share the same shape but are excluded from the shipped artifact
|
||||
- `scripts` — named commands runnable via `apm run <name>`
|
||||
- `compilation` — target/strategy/exclude/placement controls for `apm compile`/`apm pack`
|
||||
- `policy` — e.g. `fetch_failure_default`
|
||||
- `registries` — named registry endpoints for shorthand dependency resolution
|
||||
- `marketplace` — owner + packages list; see `references/marketplace.md` for the full marketplace workflow
|
||||
|
||||
type: skill # instructions | skill | hybrid | prompts — constrains .apm/ contents
|
||||
|
||||
targets: # which harnesses this package compiles to; prefer plural list form — legacy singular `target: copilot,claude` CSV form is still accepted
|
||||
- copilot
|
||||
- claude
|
||||
|
||||
includes: auto # "auto" = publish the authoritative local layout; or list explicit repo paths
|
||||
|
||||
dependencies:
|
||||
apm:
|
||||
- microsoft/apm-sample-package#v1.0.0 # pinned to a tag
|
||||
- github/awesome-copilot/skills/review-and-refactor # single primitive
|
||||
mcp:
|
||||
- microsoft/azure-devops-mcp # MCP server dependency
|
||||
lsp:
|
||||
- name: pyright
|
||||
command: pyright-langserver
|
||||
args: ["--stdio"]
|
||||
extensionToLanguage:
|
||||
".py": python
|
||||
|
||||
devDependencies: # same shape as dependencies, excluded from the shipped artifact
|
||||
apm:
|
||||
- my-org/internal-test-skills
|
||||
|
||||
scripts: # named commands runnable via `apm run <name>`
|
||||
start: "copilot -p 'README.prompt.md'"
|
||||
review: "copilot -p 'code-review.prompt.md'"
|
||||
|
||||
compilation:
|
||||
target: all
|
||||
strategy: distributed
|
||||
exclude:
|
||||
- "apm_modules/**"
|
||||
placement:
|
||||
min_instructions_per_file: 1
|
||||
|
||||
policy:
|
||||
fetch_failure_default: warn
|
||||
|
||||
registries:
|
||||
public-apm:
|
||||
url: https://registry.example.com/api/public-apm
|
||||
default: public-apm
|
||||
|
||||
marketplace: # see references/marketplace.md for the full marketplace workflow
|
||||
owner:
|
||||
name: contoso
|
||||
url: https://github.com/contoso
|
||||
packages:
|
||||
- name: code-review
|
||||
source: contoso/code-review
|
||||
version: "^1.0.0"
|
||||
tags: [review, quality]
|
||||
```
|
||||
See `docs/research/docs/microsoft-apm/configuration.md` for the complete annotated schema.
|
||||
|
||||
## Dependency reference forms
|
||||
|
||||
@@ -95,23 +43,7 @@ marketplace: # see references/marketplace.md for the full market
|
||||
|
||||
## MCP server secrets
|
||||
|
||||
Use `${VAR}` indirection for headers/env vars — never a literal secret value in `apm.yml`:
|
||||
|
||||
```yaml
|
||||
mcp:
|
||||
- name: linear
|
||||
registry: false
|
||||
transport: http
|
||||
url: https://mcp.linear.app/sse
|
||||
headers:
|
||||
Authorization: "Bearer ${LINEAR_TOKEN}"
|
||||
- name: my-internal
|
||||
registry: false
|
||||
transport: stdio
|
||||
command: my-server
|
||||
env:
|
||||
API_TOKEN: "${MY_API_TOKEN}"
|
||||
```
|
||||
`${VAR}` indirection is required for MCP server secrets in `apm.yml` — see SKILL.md Gotchas.
|
||||
|
||||
## Registries (config-level, not `apm.yml`)
|
||||
|
||||
|
||||
@@ -67,5 +67,5 @@ Without this block, the default versioning strategy ties every listed package to
|
||||
|
||||
## Gotchas
|
||||
|
||||
- A package/plugin `name` compiling to the `claude` target must avoid Claude Code's reserved prefixes — `anthropic-*`, `claude-*`, `agent-skills`, `official-claude-plugins` — otherwise the compiled `.claude-plugin/marketplace.json` is rejected by Claude Code's validator. This is a Claude Code platform constraint, independent of how the manifest gets authored.
|
||||
- Removing a package from `marketplace.packages[]` in `apm.yml` and re-packing changes the public/consumed catalog. Whoever/whatever is driving this skill (human or agent) should confirm before proceeding, the same way `apm publish` is treated — see `apm-orchestrate.md`'s "Hard rules" section for the existing precedent on `publish`.
|
||||
- A package/plugin `name` compiling to the `claude` target must avoid Claude Code's reserved name prefixes (see `plugin-author/SKILL.md` Gotchas for the current list) — otherwise the compiled `.claude-plugin/marketplace.json` is rejected by Claude Code's validator. This is a Claude Code platform constraint, independent of how the manifest gets authored.
|
||||
- Removing a package from `marketplace.packages[]` in `apm.yml` and re-packing changes the public/consumed catalog. Confirm with the user in conversation before removing the entry and running `apm pack` — this is a normal `apm.yml` edit (not a distinct apm-orchestrate operation with its own gate), so the confirmation is conversational, the same as any other consequential manifest edit.
|
||||
|
||||
@@ -21,7 +21,7 @@ metadata:
|
||||
|
||||
## Gotchas
|
||||
|
||||
- forge is an optional guided entry point, not a gate — the six existing factory skills (`skill-author`, `skill-audit`, `agent-author`, `agent-audit`, `plugin-author`, `marketplace-author`) remain directly invokable and forge does not intercept those calls.
|
||||
- forge is an optional guided entry point, not a gate — the six existing factory skills (`skill-author`, `skill-audit`, `agent-author`, `agent-audit`, `plugin-author`, `marketplace-author`) remain directly invokable and forge does not intercept those calls. Note `plugin-author` and `marketplace-author` are deprecated per ADR-0015 — superseded by `apm-workflow`, pending issue #90 — but still function normally until that lands.
|
||||
- Claude Code's skill-level `context: fork` frontmatter field and the `/fork` subagent command are opposites despite sharing a name — `context: fork` isolates (fresh context, no parent access), while `/fork` inherits the full conversation. Keep this straight when deciding how to invoke a subagent in Step 3.
|
||||
|
||||
## Step 1 — Grill the intent
|
||||
@@ -43,6 +43,8 @@ Match the grilled intent against exactly one row (or more than one, if the inten
|
||||
|
||||
If the intent is genuinely ambiguous between rows even after grilling, ask the user directly rather than guessing.
|
||||
|
||||
Note: `kyberforge:plugin-author` and `kyberforge:marketplace-author` are deprecated per ADR-0015, superseded by `apm-workflow` — pending issue #90, so routing to them is unchanged for now.
|
||||
|
||||
This table classifies what to build, not how to run it — a one-off task that merely needs an isolated vs. context-inheriting run (rather than a new, reusable definition) isn't an artifact at all; there's nothing here to route it to.
|
||||
|
||||
## Step 3 — Announce, then route
|
||||
|
||||
@@ -9,7 +9,8 @@ description: >
|
||||
the same pass. Out of scope: plugin scaffold and configuration — use
|
||||
/plugin-author for that. Does not run `claude plugin marketplace add` or
|
||||
equivalent CLI registration commands — only manages `marketplace.json`
|
||||
entries.
|
||||
entries. Deprecated per ADR-0015 — superseded by apm-workflow; kept until
|
||||
issue #90 lands.
|
||||
allowed-tools: Bash Read Write Edit
|
||||
metadata:
|
||||
category: factory
|
||||
|
||||
@@ -8,7 +8,8 @@ description: >
|
||||
(.claude-plugin/plugin.json) and Copilot CLI (plugin.json) manifests in one
|
||||
pass. Do not use when the request is about plugin content (skills, agents,
|
||||
hooks, or MCP servers inside those directories). Do not use for
|
||||
marketplace.json entries — use /marketplace-author for that.
|
||||
marketplace.json entries — use /marketplace-author for that. Deprecated
|
||||
per ADR-0015 — superseded by apm-workflow; kept until issue #90 lands.
|
||||
allowed-tools: Bash Read Write Edit
|
||||
metadata:
|
||||
category: factory
|
||||
@@ -26,7 +27,7 @@ metadata:
|
||||
|
||||
## Gotchas
|
||||
|
||||
- Both manifests must carry identical `version` values — version parity is a hard invariant (ADR-0016). Never update version in one manifest without updating the other in the same edit pass.
|
||||
- Both manifests must carry identical `version` values — version parity is a hard invariant (ADR-0006). Never update version in one manifest without updating the other in the same edit pass.
|
||||
- Every UPDATE to non-version fields (description, keywords, author, etc.) requires a patch bump to `version` in both manifests in the same edit pass. Consumers cache plugin metadata and use the version to detect changes — skipping the bump makes the update invisible until a forced refresh.
|
||||
- `author.email` is placed in the Copilot manifest by convention; `author.url` is placed in the CC manifest by convention. Both fields are supported by both platforms — do not add them to the other manifest without a deliberate reason.
|
||||
- `claude plugin tag --push` is irreversible: it creates a git tag and pushes it to remote. Always present the HITL gate and wait for explicit confirmation before running it.
|
||||
@@ -105,7 +106,7 @@ Open both manifest files and replace every `FILL_IN_*` placeholder.
|
||||
Check:
|
||||
- [ ] `name` identical in both manifests, kebab-case, no reserved prefixes
|
||||
- [ ] `description` identical in both manifests, non-empty
|
||||
- [ ] `version` identical in both manifests (version parity — ADR-0016)
|
||||
- [ ] `version` identical in both manifests (version parity — ADR-0006)
|
||||
- [ ] `author.name` identical in both manifests
|
||||
- [ ] `license` identical in both manifests
|
||||
- [ ] `keywords` identical in both manifests
|
||||
|
||||
Reference in New Issue
Block a user