diff --git a/.agents/plugins/marketplace.json b/.agents/plugins/marketplace.json new file mode 100644 index 0000000..8100ea5 --- /dev/null +++ b/.agents/plugins/marketplace.json @@ -0,0 +1,95 @@ +{ + "name": "holocron", + "interface": { + "displayName": "holocron" + }, + "plugins": [ + { + "name": "kyberforge", + "source": { + "source": "local", + "path": "./plugins/kyberforge" + }, + "policy": { + "installation": "AVAILABLE", + "authentication": "ON_INSTALL" + }, + "category": "Developer Tools" + }, + { + "name": "bin", + "source": { + "source": "local", + "path": "./plugins/bin" + }, + "policy": { + "installation": "AVAILABLE", + "authentication": "ON_INSTALL" + }, + "category": "Utilities" + }, + { + "name": "git", + "source": { + "source": "local", + "path": "./plugins/git" + }, + "policy": { + "installation": "AVAILABLE", + "authentication": "ON_INSTALL" + }, + "category": "Version Control" + }, + { + "name": "gitea", + "source": { + "source": "local", + "path": "./plugins/gitea" + }, + "policy": { + "installation": "AVAILABLE", + "authentication": "ON_INSTALL" + }, + "category": "Version Control" + }, + { + "name": "core", + "source": { + "source": "local", + "path": "./plugins/core" + }, + "policy": { + "installation": "AVAILABLE", + "authentication": "ON_INSTALL" + }, + "category": "Productivity" + }, + { + "name": "mattpocock-skills", + "source": { + "source": "url", + "url": "mattpocock/skills", + "ref": "v1.2.3", + "sha": "835450ef244ab7335f75d95b83e7d979eae22a6d", + "tag_pattern": "v{version}" + }, + "policy": { + "installation": "AVAILABLE", + "authentication": "ON_INSTALL" + }, + "category": "Productivity" + }, + { + "name": "lint", + "source": { + "source": "local", + "path": "./plugins/lint" + }, + "policy": { + "installation": "AVAILABLE", + "authentication": "ON_INSTALL" + }, + "category": "Developer Tools" + } + ] +} diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 6ed72bf..c28e71d 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -1,49 +1,67 @@ { - "description": "AI development skills for Claude Code and GitHub Copilot CLI \u2014 factory, design, implement, review, and cross-cutting workflows.", "name": "holocron", + "description": "AI development skills for Claude Code and GitHub Copilot CLI — factory, design, implement, review, and cross-cutting workflows.", + "version": "0.3.4", "owner": { + "name": "Defame1297", "email": "defame1297@rkdr.net", - "name": "Defame1297" + "url": "https://git.dev.rkdr.net/Defame1297/" }, "plugins": [ { - "description": "Skills and agents for creating, maintaining, and managing a Claude Code / Copilot CLI plugin marketplace.", "name": "kyberforge", + "description": "Skills and agents for creating, maintaining, and managing a Claude Code / Copilot CLI plugin marketplace.", + "version": "1.4.1", + "category": "Developer Tools", "source": "./plugins/kyberforge" }, { - "description": "A place for things to be binned", "name": "bin", + "description": "A place for things to be binned", + "version": "1.1.2", + "category": "Utilities", "source": "./plugins/bin" }, { - "description": "Skills for working with Git \u2014 conventional commits, branch management, pull requests, and feature flow.", "name": "git", + "description": "Skills for working with Git — conventional commits, branch management, pull requests, and feature flow.", + "version": "1.3.3", + "category": "Version Control", "source": "./plugins/git" }, { - "description": "Skills for managing Gitea repositories \u2014 issues, pull requests, milestones, releases, and wikis.", "name": "gitea", + "description": "Skills for managing Gitea repositories — issues, pull requests, milestones, releases, and wikis.", + "version": "1.3.4", + "category": "Version Control", "source": "./plugins/gitea" }, { - "description": "Cross-cutting utility skills for everyday AI-assisted coding \u2014 triage, diagnosis, architecture review, and session navigation.", "name": "core", + "description": "Skills for authoring and auditing a repo's AGENTS.md and the provider adapter files that defer to it.", + "version": "1.1.1", + "category": "Productivity", "source": "./plugins/core" }, { - "description": "Skills for Real Engineers \u2014 planning, TDD, architecture, and debugging workflows from Matt Pocock's .claude directory.", "name": "mattpocock-skills", + "description": "Skills for Real Engineers — planning, TDD, architecture, and debugging workflows from Matt Pocock's .claude directory.", + "version": "1.2.3", + "category": "Productivity", "source": { + "source": "github", "repo": "mattpocock/skills", - "source": "github" + "ref": "v1.2.3", + "sha": "835450ef244ab7335f75d95b83e7d979eae22a6d", + "tag_pattern": "v{version}" } }, { - "description": "Skills and agents for configuring and running linters.", "name": "lint", + "description": "Skills and agents for configuring and running linters.", + "version": "1.1.6", + "category": "Developer Tools", "source": "./plugins/lint" } - ], - "version": "0.3.1" + ] } diff --git a/.github/plugin/marketplace.json b/.github/plugin/marketplace.json index 6ed72bf..c28e71d 100644 --- a/.github/plugin/marketplace.json +++ b/.github/plugin/marketplace.json @@ -1,49 +1,67 @@ { - "description": "AI development skills for Claude Code and GitHub Copilot CLI \u2014 factory, design, implement, review, and cross-cutting workflows.", "name": "holocron", + "description": "AI development skills for Claude Code and GitHub Copilot CLI — factory, design, implement, review, and cross-cutting workflows.", + "version": "0.3.4", "owner": { + "name": "Defame1297", "email": "defame1297@rkdr.net", - "name": "Defame1297" + "url": "https://git.dev.rkdr.net/Defame1297/" }, "plugins": [ { - "description": "Skills and agents for creating, maintaining, and managing a Claude Code / Copilot CLI plugin marketplace.", "name": "kyberforge", + "description": "Skills and agents for creating, maintaining, and managing a Claude Code / Copilot CLI plugin marketplace.", + "version": "1.4.1", + "category": "Developer Tools", "source": "./plugins/kyberforge" }, { - "description": "A place for things to be binned", "name": "bin", + "description": "A place for things to be binned", + "version": "1.1.2", + "category": "Utilities", "source": "./plugins/bin" }, { - "description": "Skills for working with Git \u2014 conventional commits, branch management, pull requests, and feature flow.", "name": "git", + "description": "Skills for working with Git — conventional commits, branch management, pull requests, and feature flow.", + "version": "1.3.3", + "category": "Version Control", "source": "./plugins/git" }, { - "description": "Skills for managing Gitea repositories \u2014 issues, pull requests, milestones, releases, and wikis.", "name": "gitea", + "description": "Skills for managing Gitea repositories — issues, pull requests, milestones, releases, and wikis.", + "version": "1.3.4", + "category": "Version Control", "source": "./plugins/gitea" }, { - "description": "Cross-cutting utility skills for everyday AI-assisted coding \u2014 triage, diagnosis, architecture review, and session navigation.", "name": "core", + "description": "Skills for authoring and auditing a repo's AGENTS.md and the provider adapter files that defer to it.", + "version": "1.1.1", + "category": "Productivity", "source": "./plugins/core" }, { - "description": "Skills for Real Engineers \u2014 planning, TDD, architecture, and debugging workflows from Matt Pocock's .claude directory.", "name": "mattpocock-skills", + "description": "Skills for Real Engineers — planning, TDD, architecture, and debugging workflows from Matt Pocock's .claude directory.", + "version": "1.2.3", + "category": "Productivity", "source": { + "source": "github", "repo": "mattpocock/skills", - "source": "github" + "ref": "v1.2.3", + "sha": "835450ef244ab7335f75d95b83e7d979eae22a6d", + "tag_pattern": "v{version}" } }, { - "description": "Skills and agents for configuring and running linters.", "name": "lint", + "description": "Skills and agents for configuring and running linters.", + "version": "1.1.6", + "category": "Developer Tools", "source": "./plugins/lint" } - ], - "version": "0.3.1" + ] } diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ab0d9b4..58b9c09 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -28,6 +28,16 @@ repos: - id: pretty-format-json stages: ['pre-commit'] args: [--autofix] + # Every generated manifest lives at a KNOWN path, so every alternative is + # root-anchored and spells that path out. This was five `(^|/)` + # any-depth alternatives plus one `^` root-only one -- a mixture with no + # rationale, under which a fixture or vendored tree containing + # `.../.claude-plugin/plugin.json` would have been silently excluded from + # formatting while an equivalent `.../.agents/plugins/marketplace.json` + # would not. All fifteen real files (3 root marketplace manifests, 2 per + # plugin x 6 plugins) match; anything else is hand-authored and gets + # formatted. + exclude: '^(\.claude-plugin/marketplace\.json|\.agents/plugins/marketplace\.json|\.github/plugin/marketplace\.json|plugins/[^/]+/\.claude-plugin/plugin\.json|plugins/[^/]+/\.github/plugin/plugin\.json)$' - id: check-yaml stages: ['pre-commit'] - id: trailing-whitespace @@ -45,8 +55,8 @@ repos: hooks: - id: run-tests name: Run test suite - description: Run all test-*.sh files and bats suite - entry: bash tests/run-tests.sh + description: Run all test-*.sh files and bats suite. --strict because a suite that exits 77 (SKIPPED) at pre-push means a documented dependency is missing on this machine, and pre-commit prints nothing for a passing hook -- without it the gate went green having verified 15 of 17 suites on a vale-less PATH, with the skip list swallowed. Ad-hoc `bash tests/run-tests.sh` still skips gracefully. + entry: bash tests/run-tests.sh --strict language: system stages: [pre-push] pass_filenames: false @@ -61,6 +71,105 @@ repos: pass_filenames: false always_run: true + - id: check-plugin-content-sync + name: Check plugin content sync + description: Verify each plugin's flat skills/agents/commands/hooks/hooks.json mirror is in sync with .apm/ -- Claude Code has no .apm/ awareness so this compiled mirror must stay current (see issue #90) + entry: bash scripts/sync-plugin-content.sh --check --all + language: system + stages: [pre-push] + pass_filenames: false + always_run: true + + - id: check-marketplace-mirror-sync + name: Check marketplace mirror sync + description: Verify .github/plugin/marketplace.json (Copilot CLI's legacy manifest path) is byte-identical to .claude-plugin/marketplace.json -- apm has no output profile for this path, so it must be kept in sync explicitly (see issue #90) + entry: bash scripts/sync-marketplace-mirror.sh --check + language: system + stages: [pre-push] + pass_filenames: false + always_run: true + + - id: apm-marketplace-check + name: apm marketplace check + description: Validate every marketplace.packages[] entry resolves, including network reachability of remote refs -- catches stale/unreachable remote package references that check-manifests.sh deliberately skips (local-source checks only) + entry: apm marketplace check + language: system + stages: [pre-push] + pass_filenames: false + always_run: true + + - id: apm-audit-ci + name: apm audit --ci + description: Run apm's producer-side CI gate over the root manifest AND each of the six plugin packages. Verifies exactly two things per manifest -- apm.yml parses as a valid APM manifest (manifest-parse), and, if it declares dependencies, apm.lock.yaml exists and is consistent (lockfile-exists). It does NOT enforce an org policy and does NOT scan for hidden Unicode; see the comment below for why. Reference:plugins/kyberforge/.apm/skills/apm-workflow/references/audit.md + entry: bash -c 'for d in . plugins/*/; do (cd "$d" && apm audit --ci) || { echo "apm audit --ci failed in $d" >&2; exit 1; }; done' + language: system + stages: [pre-push] + pass_filenames: false + always_run: true + # The description above deliberately claims less than this hook's old one + # did ("lockfile/policy/hidden-content integrity"), because two of those + # three were never happening: + # + # * POLICY. `apm audit --ci` discovers an org policy from the git remote, + # and apm's discovery only understands github.com and Azure DevOps. + # This repo's remote is a self-hosted Gitea, so discovery resolves + # nothing and the run prints `No org policy found at unknown; + # enforcement skipped`. apm's own message suggests + # `policy.fetch_failure_default=block` in apm.yml "to fail closed" -- + # that was tried on a scratch copy and REJECTED: it does not make the + # check meaningful, it makes it permanently red. `apm audit --ci` then + # exits 1 with `No org policy found at unknown + # (policy.fetch_failure_default=block)` on every push, because there is + # no org policy to find and no supported way for this remote to serve + # one. A gate that can never go green is not a gate. Revisit if this + # repo ever gains a policy source apm can actually reach. + # * HIDDEN CONTENT. The hidden-Unicode scan is plain `apm audit`, not + # `apm audit --ci` (the two are different modes, and --ci refuses to + # combine with --file/--strip/--dry-run/PACKAGE). Plain `apm audit` + # here reports `No apm.lock.yaml found -- nothing to scan` and exits 0, + # so adding it would buy a second vacuous check, not coverage. + # + # What IS left is worth keeping, and is now run against seven manifests + # instead of one. lockfile-exists is conditional -- it is vacuous while + # every apm.yml declares `dependencies: {apm: [], mcp: []}`, and it arms + # itself the moment one does not (verified: adding a git dependency to + # plugins/lint/apm.yml fails with `apm.yml declares dependencies but + # apm.lock.yaml is absent`). manifest-parse is unconditional and fires on + # any malformed manifest (verified: a dependency entry missing its + # git/path/registry field fails with `Cannot parse apm.yml`). Running the + # six plugin packages is what makes either reachable for them at all -- + # the root-only invocation audits the marketplace manifest and nothing + # else. Costs ~0.5s per package, needs no network (checked under + # `unshare -rn`), so this does NOT join apm-marketplace-check and + # apm-pack-check-clean on the offline SKIP= list. + + - id: check-apm-agents-valid + name: Validate real APM agent files + description: Run agent-audit's validate.sh over every plugins/*/.apm/agents/*.agent.md file in this repo -- the artifacts it governs, not fixtures + entry: bash scripts/check-apm-agents-valid.sh + language: system + stages: [pre-push] + pass_filenames: false + always_run: true + # validate.sh was previously exercised only by check-scope-walkup-sync, + # and only against synthetic mktemp fixtures -- it had never run against + # the four agent files it governs. That is how ADR-0016 could be amended + # to bless a `disallowedTools` frontmatter field while validate.sh's + # allowlist still rejected it: the spec and its enforcer disagreed and + # every gate stayed green. The expected file set is derived from + # `git ls-files` (the pattern tests/run-bats.sh established) rather than + # a hardcoded count, and discovering zero files is an error, not a pass. + # Needs no network. + + - id: apm-pack-check-clean + name: apm pack --check-clean + description: Release gate -- verify .claude-plugin/marketplace.json still matches what apm.yml + .apm/ would currently generate, and that per-package versions agree with the per_package versioning strategy. Closes issue #90's deferred item 3 (a check-clean-equivalent gate) using apm's own flag instead of custom drift logic. + entry: apm pack --check-versions --check-clean --dry-run + language: system + stages: [pre-push] + pass_filenames: false + always_run: true + - id: check-vale-style-sync name: Check Vale style copies are in sync description: Diff skill-audit's Vale copy against agent-audit's canonical copy @@ -69,6 +178,16 @@ repos: stages: [pre-push] pass_filenames: false always_run: true + # verbose so the DOWNGRADED run is audible. This hook can pass while + # having verified strictly less than its name claims: + # CHECK_VALE_STYLE_SYNC_ALLOW_MISSING_VALE=1 skips all six glob probes + # and says so on a `passed (text-level only, vale unavailable)` line. + # pre-commit prints nothing at all for a passing hook, so without this + # the opt-out reinstated exactly the silent vacuous pass the script was + # written to kill, one level up -- the run showed a bare `Passed` and + # AGENTS.md's instruction to read that summary line was impossible to + # follow in the one situation the opt-out exists for. The script's clean + # output is a single line, so this costs one line per push. - id: check-scope-walkup-sync name: Check scope walk-up implementations agree @@ -112,7 +231,7 @@ repos: description: Ensure SKILL.md files have required frontmatter fields entry: bash language: system - files: 'SKILL\.md$' + files: '^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$' args: - -c - | @@ -131,25 +250,25 @@ repos: description: Enforce agentskills.io's 500-line/5,000-token SKILL.md size ceiling entry: scripts/skill-size-check.sh language: script - files: '^plugins/[^/]+/skills/[^/]+/SKILL\.md$' + files: '^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$' pass_filenames: true - id: vale-audit-prefilter-skill stages: ['pre-commit'] name: Vale audit prefilter (SKILL.md) description: Run Vale against SKILL.md files as a deterministic prefilter for skill-audit, via skill-audit's own bundled copy - entry: plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh + entry: plugins/kyberforge/.apm/skills/skill-audit/scripts/vale-wrap.sh language: script - files: '^plugins/[^/]+/skills/[^/]+/SKILL\.md$' + files: '^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$' pass_filenames: true - id: vale-audit-prefilter-agent stages: ['pre-commit'] name: Vale audit prefilter (agent files) description: Run Vale against agent markdown files as a deterministic prefilter for agent-audit, via agent-audit's own bundled copy - entry: plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh + entry: plugins/kyberforge/.apm/skills/agent-audit/scripts/vale-wrap.sh language: script - files: '^plugins/[^/]+/agents/[^/]+\.md$' + files: '^plugins/[^/]+/\.apm/agents/[^/]+\.agent\.md$' pass_filenames: true - repo: meta diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index eee3986..479995a 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -1,14 +1,14 @@ - id: kyberforge-vale-audit-skill name: Kyberforge Vale prose audit (SKILL.md) description: Deterministic prose-pattern prefilter for kyberforge's skill-audit, via its own bundled Vale config/styles - entry: plugins/kyberforge/skills/skill-audit/scripts/vale-wrap.sh + entry: plugins/kyberforge/.apm/skills/skill-audit/scripts/vale-wrap.sh language: script files: '(^|/)SKILL\.md$' - id: kyberforge-vale-audit-agent name: Kyberforge Vale prose audit (agent files) description: Deterministic prose-pattern prefilter for kyberforge's agent-audit, via its own bundled Vale config/styles - entry: plugins/kyberforge/skills/agent-audit/scripts/vale-wrap.sh + entry: plugins/kyberforge/.apm/skills/agent-audit/scripts/vale-wrap.sh language: script files: '(^|/)agents/[^/]+\.md$|\.agent\.md$' diff --git a/AGENTS.md b/AGENTS.md index d169ff3..e5a9d43 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,9 +4,23 @@ This repo is the global AI development configuration repository — the authorit ## Structure -- `plugins/` — installable plugin units; each is self-contained (skills, agents, hooks, MCP servers, bundled assets); install separately via `claude plugin install @holocron` +- `plugins/` — installable plugin units; each is an apm package (`apm.yml` + `.apm/`) carrying skills, agents, hooks, MCP servers, and bundled assets; install separately via `claude plugin install @holocron` - `providers/claude-code/` — Claude Code adapter (deployed to `~/.claude/` via `install.sh`) +## Edit `.apm/`, never the flat mirror + +Inside a plugin, `plugins//.apm/` is the **only** hand-edited source for **plugin content** — the skills, agents, commands, instructions, extensions and hooks a host discovers. Everything in a plugin root that mirrors an `.apm/` primitive, plus both `plugin.json` manifests, is generated: + +- `scripts/sync-plugin-content.sh` generates the flat `plugins//{skills,agents,commands,instructions,extensions}/` directories and the merged `plugins//hooks/hooks.json` (ADR-0017) +- `apm pack` generates both per-plugin manifests — `plugins//.claude-plugin/plugin.json` and `plugins//.github/plugin/plugin.json` — and **two of the three** root marketplace manifests: `.claude-plugin/marketplace.json` (apm's `claude` output profile) and `.agents/plugins/marketplace.json` (its `codex` profile, a differently-shaped file) (ADR-0015) +- `scripts/sync-marketplace-mirror.sh` generates the third, `.github/plugin/marketplace.json` — Copilot CLI's legacy manifest path. **No apm output profile targets it**: apm ships exactly two marketplace output profiles, `claude` and `codex` (documented in `plugins/kyberforge/.apm/skills/apm-workflow/references/marketplace.md`). The mirror is a byte-identical copy of `.claude-plugin/marketplace.json`, gated by the `check-marketplace-mirror-sync` pre-push hook. Do not expect `apm pack` to refresh it — that assumption is exactly the drift this pair exists to prevent + +**A plugin root is not wholly generated.** Material that is not an `.apm/` primitive is hand-authored there and no compiler touches it: `README.md`, `docs/`, `bin/`, `sources.md`, `.mcp.json`, plus per-plugin extras like `plugins/git/config.example.json`, `plugins/gitea/references/` and `plugins/bin/evals/`. Edit those in place — they have no `.apm/` source, and looking for one wastes a search. The rule is per-path, not per-directory: `plugins//skills/` is generated, `plugins//docs/` is not. `docs/spec/architecture.md` carries the same carve-out. + +One qualification: "hand-authored, untouched" holds only at the plugin *root*. A file placed **inside** a mirrored directory is destroyed — `sync_dir` runs `rm -rf "$dst"` before every copy, so a `README.md` under `plugins//hooks/` or `plugins//skills/` is deleted on the next sync whether or not `.apm/` has a counterpart. Put root-level plugin documentation in `docs/`, never in a mirrored directory. + +Nothing labels a generated file as generated — `plugins/kyberforge/skills/forge/SKILL.md` is byte-identical to its `.apm/` original, with no marker in either. Check the path before you edit. An edit to the mirror is discarded by the next sync and is reported as drift by the `check-plugin-content-sync` pre-push hook, which is the earliest anyone finds out. Details in `docs/spec/architecture.md`. + ## Prefer plugin skills over raw shell This repo dogfoods its own plugins. Before shelling out to git, gitea, or lint tooling directly, check whether an installed skill already owns the operation — it usually does: @@ -21,10 +35,18 @@ Fall back to raw shell only when no skill covers it. ## Setup and testing -- Install git hooks via `git:pc-run`, wiring all three stages — this repo's `.pre-commit-config.yaml` has no `default_install_hook_types`, so a plain install silently skips `commit-msg` (Conventional Commits) and `pre-push` (tests, manifest check). -- Install the `vale` binary — required by the `vale-audit-prefilter-skill`/`-agent` pre-commit hooks, which run on every commit touching a `SKILL.md` or agent `.md` file. Without it the hooks fail with a bare "command not found" and no install pointer. `brew install vale` (macOS), `snap install vale` (Linux), `choco install vale` (Windows), or see https://vale.sh/docs/vale-cli/installation/. No `vale sync` needed — the `Kyberforge` styles are committed under `plugins/kyberforge/skills/{skill-audit,agent-audit}/assets/vale/styles/`, not downloaded packages (see ADR-0014). +- Install git hooks via `git:pc-run`, wiring all three stages — this repo's `.pre-commit-config.yaml` has no `default_install_hook_types`, so a plain install silently skips `commit-msg` (Conventional Commits) and `pre-push` (the 13-hook gate described below). +- Install the `apm` CLI — four pre-push hooks shell out to it: `apm-marketplace-check`, `apm-audit-ci`, `apm-pack-check-clean`, and `check-plugin-content-sync` (via `scripts/sync-plugin-content.sh`, which wraps `apm pack`). `apm-marketplace-check` and `apm-pack-check-clean` are bare `apm …` hook entries and `apm-audit-ci` is a `bash -c` loop calling `apm` once per package, so without it the push dies with an unhelpful "command not found". Use `kyberforge:apm-install`, or `curl -sSL https://aka.ms/apm-unix | sh`; verify with `apm --version`. +- Install `jq` — required by `scripts/check-manifests.sh` and `scripts/sync-plugin-content.sh`, both pre-push. These at least fail loudly (`Error: jq is required but not installed`). +- Install the `vale` binary — required by the `vale-audit-prefilter-skill`/`-agent` pre-commit hooks. Their `files:` patterns are `.apm/`-scoped: `^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$` and `^plugins/[^/]+/\.apm/agents/[^/]+\.agent\.md$`. Only the authoring source triggers them — a `SKILL.md` in the generated mirror matches neither pattern, so prose findings surface only when you edit the file you are supposed to be editing. Without the binary the hooks fail with a bare "command not found" and no install pointer. `brew install vale` (macOS), `snap install vale` (Linux), `choco install vale` (Windows), or see https://vale.sh/docs/vale-cli/installation/. No `vale sync` needed — the `Kyberforge` styles are committed under `plugins/kyberforge/.apm/skills/{skill-audit,agent-audit}/assets/vale/styles/`, not downloaded packages (see ADR-0014). +- `vale` is also a **pre-push** dependency, not only pre-commit. `check-vale-style-sync` runs six glob-coverage probes by invoking `vale --config` — they are the only assertions in it that catch a `.vale.ini` glob typo, the failure mode where every text-level check stays clean while vale lints zero files. Missing `vale` is therefore a hard failure there. The opt-out is `CHECK_VALE_STYLE_SYNC_ALLOW_MISSING_VALE=1`, and it is **not** `SKIP=`: the hook still runs and still asserts everything verifiable from file text, but the six probes do not, and its summary says so explicitly — `Vale style sync check passed (text-level only, vale unavailable): … 0 glob probe(s) verified`. Use it only on a machine that genuinely cannot install `vale`, and read that summary line as "the glob axis was not checked", not as a pass. - Run `bash tests/run-tests.sh` before considering any change done — it runs every `test-*.sh` script in the repo plus the bats suite (`--bats-only` for just bats). First run auto-initializes the bats submodules; no manual `git submodule update` needed. -- Pushing re-runs the full suite plus `scripts/check-manifests.sh` via the pre-push hook — same commands, so run them locally first. +- A suite that exits 77 because a dependency is missing is reported as SKIPPED, and does **not** fail an ad-hoc run. The pre-push hook invokes the same script as `--strict` (`RUN_TESTS_STRICT=1` is equivalent), where a skip **does** fail the push: at pre-push a skip means one of the dependencies above is absent on this machine, so the gate would otherwise report success having run fewer suites than it appears to. Without vale, for instance, three suites skip (`test-check-vale-style-sync.sh`, `test-vale-hooks-consumer.sh`, `test-vale-wrap.sh`) and the strict failure names each one and what to install. +- `tests/run-bats.sh` derives the set of `.bats` files it expects from `git ls-files`, so a `.bats` file deleted from the worktree but still tracked in the index fails the run rather than silently shrinking the suite. Remove one with `git rm` (or stage the deletion) when the removal is intentional; an untracked new `.bats` file is picked up and needs no ceremony. +- Pushing runs 13 repo-defined pre-push hooks, not just the test suite — `run-tests` and `check-manifests`, plus generated-content drift gates (`check-plugin-content-sync`, `check-marketplace-mirror-sync`, `check-vale-style-sync`, `check-scope-walkup-sync`), artifact validators (`check-apm-agents-valid`, which runs agent-audit's `validate.sh` over every real `plugins/*/.apm/agents/*.agent.md`), apm's own gates (`apm-marketplace-check`, `apm-audit-ci`, `apm-pack-check-clean`), host validators (`validate-plugins`, `validate-marketplace`, both needing the `claude` CLI), and `check-release-needed`. Run `pre-commit run --hook-stage pre-push --all-files` locally — one command, the whole gate. That command reports **15**, not 13: pre-commit's own `meta` hooks, `check-hooks-apply` and `check-useless-excludes`, declare no `stages:` and so run at every stage including this one. +- `apm-audit-ci` runs `apm audit --ci` once per manifest — the root one and each of the six plugin packages — because the root-only invocation audits the marketplace manifest and **nothing else**, and `apm-pack-check-clean` does not parse plugin `dependencies:` blocks either (verified: a malformed one passes `apm pack --check-versions --check-clean --dry-run` and fails `apm audit --ci` in that package's directory). It verifies two things and claims no more: each `apm.yml` parses as a valid APM manifest, and any package declaring dependencies has a consistent `apm.lock.yaml`. It does **not** enforce an org policy — apm discovers one from the git remote and only understands github.com and Azure DevOps, so against this repo's self-hosted Gitea remote it prints `No org policy found at unknown; enforcement skipped`. Do **not** "fix" that with `policy.fetch_failure_default: block` in `apm.yml`: it was tested and rejected, because with no reachable policy source it makes the hook exit 1 on every push forever. +- `check-apm-agents-valid` derives its expected agent-file set from `git ls-files` (same pattern as `tests/run-bats.sh`), so an agent file deleted from the worktree but still tracked fails the run, and discovering zero agent files is an error rather than a pass. An untracked new agent file is still validated — the derivation is one-directional on purpose, so uncommitted work is not blocked but also cannot bypass the gate. +- **Two** pre-push hooks need the network, for one shared reason: root `apm.yml`'s `marketplace.packages[]` contains exactly one remote entry (`mattpocock-skills`, `source: mattpocock/skills`), and resolving it needs a `git ls-remote`. `apm-marketplace-check` resolves every entry and is `always_run`, so it fails with `No cached refs (offline)`. `apm-pack-check-clean` (`apm pack --check-versions --check-clean --dry-run`) re-resolves the same entry and fails with `Error: Git network timeout during ls-remote`. Pinning the entry to an exact version does **not** remove the call — an exact pin still ls-remotes. `--offline` rescues neither. To push without a network, skip both using pre-commit's own mechanism: `SKIP=apm-marketplace-check,apm-pack-check-clean git push`. Skip those two alone — verified under `unshare -rn`, the other eleven pre-push hooks pass offline because they are real local checks, and adding one of them to `SKIP` disarms it silently. `apm-audit-ci` calls `apm` too but stays local: its org-policy discovery resolves nothing on this remote before any network call, so it does not join the pair above. - Author commits with `git:git-commits` — it validates Conventional Commits (enforced at `commit-msg`) for you. ## Key documents diff --git a/CONTEXT.md b/CONTEXT.md index ea48c76..02f4112 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -24,13 +24,13 @@ Before answering any design or architecture question, check for existing decisio A separate product (separate repo) for browsing, editing, and configuring AI development configs through a proper product UI. Git is the persistence layer, invisible to the user. The app is repo-agnostic — it works with any git repo that follows these conventions. This repo is the canonical default content (the official starter). See `docs/VISION.md` for the phased roadmap. ### Skills -Reusable slash commands for AI coding tools, defined as `SKILL.md` files following the [Agent Skills open standard](https://agentskills.io). Deployed via plugin — `plugins//skills//SKILL.md`, available after the plugin is installed (`claude plugin install @`). Skills are self-contained — they cannot reference files outside the plugin directory after install-time caching. +Reusable slash commands for AI coding tools, defined as `SKILL.md` files following the [Agent Skills open standard](https://agentskills.io). Deployed via plugin — `plugins//.apm/skills//SKILL.md`, available after the plugin is installed (`claude plugin install @`). Skills are self-contained — they cannot reference files outside the plugin directory after install-time caching. ### Plugin -The deployable unit in the plugin marketplace. A plugin bundles one or more skills, agents, hooks, prompts, MCP servers, and optionally a `bin/` directory into a single installable directory. Each plugin has two manifests: `.claude-plugin/plugin.json` (Claude Code) and `plugin.json` at the plugin root (Copilot CLI). Plugins are copied to a cache on install — they cannot reference files outside their own directory. In this repo, plugins live under `plugins//`. Install a plugin with `claude plugin install @`. These manifests are hand-authored today but are slated to become APM-compiled output per ADR-0015, pending issue #90 (not yet changed). +The deployable unit in the plugin marketplace. A plugin bundles one or more skills, agents, hooks, prompts, MCP servers, and optionally a `bin/` directory into a single installable directory. In this repo, plugins live under `plugins//`, each with its own `apm.yml` + `.apm/{skills,agents,hooks,...}` — this is the authoring source of truth for the plugin's content (ADR-0015). Two categories of tracked output are compiled from that source, never hand-edited: `.claude-plugin/plugin.json` (Claude Code) and `.github/plugin/plugin.json` (Copilot CLI) via `apm pack`/`apm compile`; and, alongside them, a flat `agents/`, `skills/`, `commands/`, `instructions/`, `extensions/` directory mirror at the plugin root plus a merged hooks file at `hooks/hooks.json`, generated by `scripts/sync-plugin-content.sh` — Claude Code's and Copilot's installers convention-scan only these flat paths (`hooks/hooks.json` is the convention path for hooks specifically; a root-level `hooks.json` is scanned by nothing and is deleted as stale by a sync — see ADR-0017's 2026-08-14 amendment) and have no awareness of `.apm/` nesting at all, so this mirror is what actually makes `.apm/` content discoverable at install time (ADR-0017). Plugins are copied to a cache on install — they cannot reference files outside their own directory. Install a plugin with `claude plugin install @`. ### Plugin marketplace -A Git repository with a `marketplace.json` manifest listing installable plugins. No backend, registry, or SaaS required — the Git repo is the marketplace. This repo is the `holocron` marketplace. The manifest lives at `.claude-plugin/marketplace.json` (read by both Claude Code and Copilot CLI) and is mirrored to `.github/plugin/marketplace.json`. This manifest is hand-authored today but is slated to become APM-compiled output per ADR-0015, pending issue #90 (not yet changed). +A Git repository with a `marketplace.json` manifest listing installable plugins. No backend, registry, or SaaS required — the Git repo is the marketplace. This repo is the `holocron` marketplace. The manifest at `.claude-plugin/marketplace.json` (read by both Claude Code and Copilot CLI) is **compiled output** of `apm pack`, generated from the root `apm.yml`'s `marketplace:` block (owner, build/output config, versioning strategy, and the `packages:` list of installable plugins) — it is not hand-edited. See ADR-0015. `.github/plugin/marketplace.json` is Copilot CLI's legacy manifest path; apm has no output profile for it (only `claude` and `codex`, and `codex`'s is a differently-shaped file at `.agents/plugins/marketplace.json`), so `scripts/sync-marketplace-mirror.sh` keeps it byte-identical to `.claude-plugin/marketplace.json`, checked at pre-push. Each listed package's `source:` still points at that plugin's own `plugins//` root, not at an `apm pack` build artifact — which is why that root also carries the flat `agents/`/`skills/`/`commands/`/`hooks/hooks.json` content mirror described under "Plugin" (ADR-0017): without it, an install from this marketplace finds a valid manifest but no discoverable content. ### HITL (human-in-the-loop) Agent pauses before a consequential action; human approves before execution. Required for irreversible or high-stakes actions (architecture changes, production deployments, security configuration). The agent drafts the change plan and waits — it does not proceed autonomously. Contrast with HOTL. @@ -49,10 +49,10 @@ The provider-agnostic always-on instruction entry point. Two files: Contains always-on rules in plain markdown with no provider-specific syntax (no `@import`). Provider-specific files (`CLAUDE.md`) are thin adapters that import the relevant `AGENTS.md` and add only Claude Code-specific syntax. This pattern means a single source of truth can serve multiple providers without duplication. See ADR-0003. ### Skill composition -A skill calling another skill by name to delegate a sub-task. The calling skill focuses on the orchestration decision ("when to do X"); the called skill owns the mechanics ("how to do X"). Established compositions: `grill-me` calls `write-adr` when a decision crystallises; `implement-feature` calls `tdd` as its implementation methodology; `forge` calls `grill-with-docs` to refine intent, classifies the target artifact type (skill / agent / plugin / marketplace entry), then routes to the matching `*-author` skill — which owns its own create/improve logic and, where applicable, its own inline audit closeout (`skill-author` runs `/skill-audit`, `agent-author` runs `kyberforge:agent-audit`, both in the same context as the authoring work). Reserve `forge` for genuinely undecided "which artifact type is this" questions — an already-fully-specified corrective edit (exact file, line, and fix already known) should call the target author skill directly instead (`skill-author`, `plugin-author`, `agentsmd-author`, etc.); routing a known fix through `forge`'s grill-and-classify layer adds unnecessary indirection and, in practice, has been observed to lose track of hard constraints handed down the chain (e.g. "don't commit yet," "edit in this worktree") because each hop re-derives instructions from a shorter brief. `forge` additionally runs its own independent recheck after a skill/agent route finishes: a clean-context subagent (not forked, no inherited context) re-runs the same audit skill against the finished artifact, as a distinct verification layer from the author skill's inline audit — the two can share blind spots since the inline audit runs in the same context as the work it checks. If the clean audit surfaces any unresolved finding, `forge` loops — re-invoke the author skill to resolve it, re-run the clean audit — until the clean audit comes back with nothing unresolved; only then is the route done. `plugin-author` and `marketplace-author` have no audit counterpart and get no recheck; their terminal check is `claude plugin validate`. Both are deprecated per ADR-0015, superseded by `apm-workflow`, pending issue #90 (not yet changed). +A skill calling another skill by name to delegate a sub-task. The calling skill focuses on the orchestration decision ("when to do X"); the called skill owns the mechanics ("how to do X"). Established compositions: `grill-me` calls `write-adr` when a decision crystallises; `implement-feature` calls `tdd` as its implementation methodology; `forge` calls `grill-with-docs` to refine intent, classifies the target artifact type (skill / agent / plugin / marketplace entry), then routes to the matching `*-author` skill — which owns its own create/improve logic and, where applicable, its own inline audit closeout (`skill-author` runs `/skill-audit`, `agent-author` runs `kyberforge:agent-audit`, both in the same context as the authoring work). Reserve `forge` for genuinely undecided "which artifact type is this" questions — an already-fully-specified corrective edit (exact file, line, and fix already known) should call the target author skill directly instead (`skill-author`, `apm-workflow`, `agentsmd-author`, etc.); routing a known fix through `forge`'s grill-and-classify layer adds unnecessary indirection and, in practice, has been observed to lose track of hard constraints handed down the chain (e.g. "don't commit yet," "edit in this worktree") because each hop re-derives instructions from a shorter brief. `forge` additionally runs its own independent recheck after a skill/agent route finishes: a clean-context subagent (not forked, no inherited context) re-runs the same audit skill against the finished artifact, as a distinct verification layer from the author skill's inline audit — the two can share blind spots since the inline audit runs in the same context as the work it checks. If the clean audit surfaces any unresolved finding, `forge` loops — re-invoke the author skill to resolve it, re-run the clean audit — until the clean audit comes back with nothing unresolved; only then is the route done. `plugin-author` and `marketplace-author` had no audit counterpart and got no recheck; their terminal check was `claude plugin validate`. Both were deprecated per ADR-0015, superseded by `apm-workflow`, and deleted entirely once issue #90 landed. ### Provider-agnostic issue tracker -Skills and workflows reference "linked issue" generically rather than a specific provider. Gitea is the canonical issue tracker for this repo (see ADR-0017). "Issue" is the cross-provider term (GitHub, GitLab, Gitea all use it). +Skills and workflows reference "linked issue" generically rather than a specific provider. Gitea is the canonical issue tracker for this repo (see ADR-0007). "Issue" is the cross-provider term (GitHub, GitLab, Gitea all use it). ### Provenance chain The three-stage traceability record linking a skill back to its research inputs: (1) `/research` produces topic docs and a `sources.md` in `plugins//docs/research/docs//`; (2) `/skill-author` reads those docs and records which sources informed which skill files in `references/sources.md` (including a `Research doc:` back-pointer to the upstream research file) and `source_keys` frontmatter on `SKILL.md` and `references/*.md`; (3) `skill-audit` validates the chain is complete and internally consistent via `validate-provenance.sh`. A skill with research input but no `references/sources.md`, or with `source_keys` that don't match `references/sources.md` slugs, has a broken provenance chain. @@ -67,14 +67,14 @@ A skill pair in the `core` plugin for writing, updating, and reviewing a repo's A companion skill (`core` plugin) that detects a target repo's provider-specific instruction file (`CLAUDE.md`, `.cursor/rules/*.mdc`, `copilot-instructions.md`, etc.) and, where it duplicates content AGENTS.md should own, converts it into a thin adapter that imports AGENTS.md — mirroring this repo's own ADR-0002/ADR-0003 two-tier adapter pattern. Self-validates via its own bundled deterministic script (`scripts/validate-adapter.sh`: checks for an import reference, no duplicated headings, size threshold) rather than a separate paired audit skill — the check is mechanical, so a script suffices per governance.md's "prefer deterministic code for repeatable tasks." `agentsmd-author` calls this skill via skill composition when it detects an existing provider file with overlapping content. ### lint plugin -A standalone, repo-agnostic plugin (`plugins/lint/`) for configuring and running linters — not scoped to kyberforge's own meta-tooling. First linter is Vale (prose style linting), split into two skills per the git/gitea per-concern pattern: `vale-config` (setup — `.vale.ini`, `StylesPath`, styles) and `vale-run` (invoke Vale, interpret/report findings). A `lint-runner` agent composes these for isolated-context lint sweeps; it is report-only (no `Edit` tool) — it flags findings, it does not rewrite prose. Vale's research docs (`docs/research/docs/vale/`) moved from `plugins/kyberforge/` to `plugins/lint/` to keep the provenance chain same-plugin. +A standalone, repo-agnostic plugin (`plugins/lint/`) for configuring and running linters — not scoped to kyberforge's own meta-tooling. First linter is Vale (prose style linting), split into two skills per the git/gitea per-concern pattern: `vale-config` (setup — `.vale.ini`, `StylesPath`, styles) and `vale-run` (invoke Vale, interpret/report findings). A `lint-runner` agent composes these for isolated-context lint sweeps; it is report-only **by instruction, not by capability** — its body states "You never edit files" and "Do not edit, fix, or rewrite any flagged content", but nothing enforces that. It previously carried `tools: Bash, Read, Grep, Glob`, which withheld `Edit` outright; plugin-scope APM agents cannot express a `tools:` field at all (ADR-0016 — `apm compile` copies frontmatter verbatim to both Claude Code and Copilot, whose `tools:` vocabularies are incompatible, so a value correct for one harness is wrong for the other), so `plugins/lint/.apm/agents/lint-runner.agent.md` now declares only `name`/`description`/`source_keys` and inherits every tool, `Edit` included. ADR-0016 accepted this loss of enforcement knowingly; the restriction survives as prose the agent is expected to follow. Vale's research docs (`docs/research/docs/vale/`) moved from `plugins/kyberforge/` to `plugins/lint/` to keep the provenance chain same-plugin. ### Vale audit prefilter (skill-audit / agent-audit) -Wiring Vale as a deterministic prefilter for `skill-audit`/`agent-audit`'s Description dimension (ADR motivation: issue #84) is repo-specific, not part of the generic `lint` plugin, so it doesn't live in `plugins/lint/` — but per ADR-0014 it also doesn't live at the repo root anymore. Two copies live inside `plugins/kyberforge/`, one per skill, since a plugin's cache-install only copies each skill's own files (no cross-skill sharing): `plugins/kyberforge/skills/agent-audit/assets/vale/` is canonical (`.vale.ini` plus a custom `Kyberforge` style covering description-opener banning ("This skill/agent..."), vague-capability wording ("helps with", "utilize", ...), and generic "see references/ for details" padding — and a `KyberforgeCopilot` style scoped only to `.agent.md` files for the Copilot-only "Use proactively has no effect" check), and `plugins/kyberforge/skills/skill-audit/assets/vale/` is a smaller duplicate (`Kyberforge` only, scoped to `SKILL.md`) kept in sync by `scripts/check-vale-style-sync.sh` (pre-push). A root-level `.pre-commit-hooks.yaml` exposes both copies (plus `skill-size-check`) so any external repo can enforce the same rules via `repo: , rev: ` in its own `.pre-commit-config.yaml` — pre-commit clones the pinned rev into its own cache, independent of whether Claude Code or the `kyberforge` plugin is installed at all, and the same mechanism covers CI (`pre-commit run --all-files`). This repo's own `vale-audit-prefilter-skill`/`-agent` pre-commit hooks consume the identical plugin-bundled copies via `repo: local` (not a third root copy, and not a pinned self-reference — a pinned self-reference would lint working-tree edits against the last tagged release rather than the change being made). Every rule is `level: error` and every alert is a FAIL — no ignorable tier, same as shellcheck, the test suite, and conventional-pre-commit. Graded severities do not work here: Vale's exit code keys on `error` alerts alone, so `warning`/`suggestion` rules exit 0 and pre-commit swallows the output of a passing hook, leaving them invisible and blocking nothing. `MinAlertLevel` and `--minAlertLevel` are correspondingly absent from `.vale.ini` and the hook, being no-ops under this model. Vale covers the pattern-matchable sub-checks named in issue #84 (imperative opener, vague filler, `Use proactively`, generic reference-pointer padding) plus, per ADR-0013, one body-wide prose-pattern check ("There is/are" sentence openers) — everything else about body discipline (defaults-vs-menus, why-rationale, non-pattern-matchable judgment calls), near-miss exclusion strength, and control calibration stays LLM judgment. +Wiring Vale as a deterministic prefilter for `skill-audit`/`agent-audit`'s Description dimension (ADR motivation: issue #84) is repo-specific, not part of the generic `lint` plugin, so it doesn't live in `plugins/lint/` — but per ADR-0014 it also doesn't live at the repo root anymore. Two copies live inside `plugins/kyberforge/`, one per skill, since a plugin's cache-install only copies each skill's own files (no cross-skill sharing): `plugins/kyberforge/.apm/skills/agent-audit/assets/vale/` is canonical (`.vale.ini` plus a custom `Kyberforge` style covering description-opener banning ("This skill/agent..."), vague-capability wording ("helps with", "utilize", ...), and generic "see references/ for details" padding — and a `KyberforgeCopilot` style scoped only to `.agent.md` files for the Copilot-only "Use proactively has no effect" check), and `plugins/kyberforge/.apm/skills/skill-audit/assets/vale/` is a smaller duplicate (`Kyberforge` only, scoped to `SKILL.md`) kept in sync by `scripts/check-vale-style-sync.sh` (pre-push). A root-level `.pre-commit-hooks.yaml` exposes both copies (plus `skill-size-check`) so any external repo can enforce the same rules via `repo: , rev: ` in its own `.pre-commit-config.yaml` — pre-commit clones the pinned rev into its own cache, independent of whether Claude Code or the `kyberforge` plugin is installed at all, and the same mechanism covers CI (`pre-commit run --all-files`). This repo's own `vale-audit-prefilter-skill`/`-agent` pre-commit hooks consume the identical plugin-bundled copies via `repo: local` (not a third root copy, and not a pinned self-reference — a pinned self-reference would lint working-tree edits against the last tagged release rather than the change being made). Every rule is `level: error` and every alert is a FAIL — no ignorable tier, same as shellcheck, the test suite, and conventional-pre-commit. Graded severities do not work here: Vale's exit code keys on `error` alerts alone, so `warning`/`suggestion` rules exit 0 and pre-commit swallows the output of a passing hook, leaving them invisible and blocking nothing. `MinAlertLevel` and `--minAlertLevel` are correspondingly absent from `.vale.ini` and the hook, being no-ops under this model. Vale covers the pattern-matchable sub-checks named in issue #84 (imperative opener, vague filler, `Use proactively`, generic reference-pointer padding) plus, per ADR-0013, one body-wide prose-pattern check ("There is/are" sentence openers) — everything else about body discipline (defaults-vs-menus, why-rationale, non-pattern-matchable judgment calls), near-miss exclusion strength, and control calibration stays LLM judgment. -Both skills' Step 1, and the `vale-audit-prefilter-skill`/`-agent` pre-commit hooks, call each copy's own `scripts/vale-wrap.sh` rather than `vale` directly — a workaround for a confirmed Vale 3.15.2 limitation (see `vale-config`'s Gotchas): `text.frontmatter.description` silently stops matching on most — not all — multi-line descriptions. Verified by reproduction, not assumed: `>` folded scalars, plain (unquoted) continuation lines, and single- or double-quoted multi-line scalars all yield 0 alerts and exit 0 on a deliberately-bad fixture, while a `|` literal block spanning the same 2+ lines lints normally (alerts fire, exit 1). The wrapper flattens those three broken forms to one physical line in a scratch copy (padding with blank lines so every other line number is unchanged) before handing off to real `vale`; `|` literal blocks and single-line descriptions pass through untouched, already linting correctly. The plain and quoted forms previously passed silently — unflattened and unmatched — so a bad description in either sailed through the prefilter. Handed no `--config` at all, the wrapper falls back to its own sibling `assets/vale/.vale.ini`, located from `${BASH_SOURCE[0]}` rather than from the cwd — which is why both manifests' `entry:` is now the bare script path with no argument after it. pre-commit prefixes only `entry[0]` with the hook-repo clone path (`cmd = (prefix.path(cmd[0]), *cmd[1:])`), so every later argument resolves against the *consuming* repo's root: a `--config` in `.pre-commit-hooks.yaml` pointed at a path no consumer has and hard-failed every external run with `E100 [--config] Runtime error`. `.pre-commit-config.yaml` drops the argument too, deliberately keeping the two entries identical — the local `repo: local` hook resolved its `--config` correctly only because the consuming repo *was* this repo, and that divergence is why three review rounds exercised a path no external consumer takes and missed the defect. An explicit `--config` still wins, in all three argv forms (`--config X`, `--config=/abs`, `--config=rel`), and a relative one still resolves against the caller's cwd, matching bare `vale`, not the repo root. Both audit skills' Step 1 now passes no `--config` either: it resolves the script relative to the skill's own directory so the call works from an installed plugin cache, but a relative `--config` alongside it would still resolve against the cwd, yielding `E100 Runtime error ... does not exist` and exit 2 — which both skills' fallback misreads as "vale unavailable" and silently downgrades to full LLM judgment. `tests/test-vale-wrap.sh` regression-tests this against skill-audit's copy specifically (its fixtures are all `SKILL.md`-shaped, and only skill-audit's `.vale.ini` has that glob section). Each `.vale.ini`'s section globs are path-agnostic (`[**/SKILL.md]` for skill-audit's copy; `[**/agents/*.md]`/`[**/*.agent.md]` for agent-audit's) and do no scoping on their own: Vale's `*` crosses `/`. Scoping comes from each pre-commit hook's own `files:` regex and from the audit skills passing one explicit file per invocation. The two manifests scope differently on purpose: this repo's `.pre-commit-config.yaml` pins its own layout — `^plugins/[^/]+/skills/[^/]+/SKILL\.md$` for `-skill`, `^plugins/[^/]+/agents/[^/]+\.md$` for `-agent` — while the shipped `.pre-commit-hooks.yaml` stays layout-agnostic for external consumers whose skills live anywhere, using `(^|/)SKILL\.md$` and `(^|/)agents/[^/]+\.md$|\.agent\.md$`. Both manifests split the prefilter into two hooks precisely because one combined hook pointed at only one copy would silently 0-file-skip the other file type. A `SKILL.md` outside `plugins/` (e.g. project-scope `.claude/skills/foo/SKILL.md`) still matches `[**/SKILL.md]` and gets linted normally — the globs constrain filename shape, not location. Vale reports 0 files only when the path it is handed matches no glob section at all: a differently-named file, or a directory argument holding nothing that matches. That run prints `✔ 0 errors ... in 0 files.` and exits 0, indistinguishable from a clean pass, so both audits treat a 0-file Vale run as NOT RUN and fall back to full LLM judgment. +Both skills' Step 1, and the `vale-audit-prefilter-skill`/`-agent` pre-commit hooks, call each copy's own `scripts/vale-wrap.sh` rather than `vale` directly — a workaround for a confirmed Vale 3.15.2 limitation (see `vale-config`'s Gotchas): `text.frontmatter.description` silently stops matching on most — not all — multi-line descriptions. Verified by reproduction, not assumed: `>` folded scalars, plain (unquoted) continuation lines, and single- or double-quoted multi-line scalars all yield 0 alerts and exit 0 on a deliberately-bad fixture, while a `|` literal block spanning the same 2+ lines lints normally (alerts fire, exit 1). The wrapper flattens those three broken forms to one physical line in a scratch copy (padding with blank lines so every other line number is unchanged) before handing off to real `vale`; `|` literal blocks and single-line descriptions pass through untouched, already linting correctly. The plain and quoted forms previously passed silently — unflattened and unmatched — so a bad description in either sailed through the prefilter. Handed no `--config` at all, the wrapper falls back to its own sibling `assets/vale/.vale.ini`, located from `${BASH_SOURCE[0]}` rather than from the cwd — which is why both manifests' `entry:` is now the bare script path with no argument after it. pre-commit prefixes only `entry[0]` with the hook-repo clone path (`cmd = (prefix.path(cmd[0]), *cmd[1:])`), so every later argument resolves against the *consuming* repo's root: a `--config` in `.pre-commit-hooks.yaml` pointed at a path no consumer has and hard-failed every external run with `E100 [--config] Runtime error`. `.pre-commit-config.yaml` drops the argument too, deliberately keeping the two entries identical — the local `repo: local` hook resolved its `--config` correctly only because the consuming repo *was* this repo, and that divergence is why three review rounds exercised a path no external consumer takes and missed the defect. An explicit `--config` still wins, in all three argv forms (`--config X`, `--config=/abs`, `--config=rel`), and a relative one still resolves against the caller's cwd, matching bare `vale`, not the repo root. Both audit skills' Step 1 now passes no `--config` either: it resolves the script relative to the skill's own directory so the call works from an installed plugin cache, but a relative `--config` alongside it would still resolve against the cwd, yielding `E100 Runtime error ... does not exist` and exit 2 — which both skills' fallback misreads as "vale unavailable" and silently downgrades to full LLM judgment. `tests/test-vale-wrap.sh` regression-tests this against skill-audit's copy specifically (its fixtures are all `SKILL.md`-shaped, and only skill-audit's `.vale.ini` has that glob section). Each `.vale.ini`'s section globs are path-agnostic (`[**/SKILL.md]` for skill-audit's copy; `[**/agents/*.md]`/`[**/*.agent.md]` for agent-audit's) and do no scoping on their own: Vale's `*` crosses `/`. Scoping comes from each pre-commit hook's own `files:` regex and from the audit skills passing one explicit file per invocation. The two manifests scope differently on purpose: this repo's `.pre-commit-config.yaml` pins its own layout — `^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$` for `-skill`, `^plugins/[^/]+/\.apm/agents/[^/]+\.agent\.md$` for `-agent` — while the shipped `.pre-commit-hooks.yaml` stays layout-agnostic for external consumers whose skills live anywhere, using `(^|/)SKILL\.md$` and `(^|/)agents/[^/]+\.md$|\.agent\.md$`. Both manifests split the prefilter into two hooks precisely because one combined hook pointed at only one copy would silently 0-file-skip the other file type. A `SKILL.md` outside `plugins/` (e.g. project-scope `.claude/skills/foo/SKILL.md`) still matches `[**/SKILL.md]` and gets linted normally — the globs constrain filename shape, not location. Vale reports 0 files only when the path it is handed matches no glob section at all: a differently-named file, or a directory argument holding nothing that matches. That run prints `✔ 0 errors ... in 0 files.` and exits 0, indistinguishable from a clean pass, so both audits treat a 0-file Vale run as NOT RUN and fall back to full LLM judgment. -This scope expands per ADR-0013: one cherry-picked low-noise `write-good`/`alex` rule landed in `styles/Kyberforge`, `Kyberforge.SentenceOpenerThereIs` (22 held-out hits, both in-corpus hits clean rewrites, zero suppressions). A second, `Kyberforge.VagueQualifier`, was cherry-picked and then deleted: 2 hits across the 41 skill/agent files, one marginal and one an unfixable false positive (`caveman/SKILL.md` quotes `of course` as an example of filler — a mention, not a use) that forced the repo's only Vale suppression comments. Also new is a sibling pre-commit hook, `skill-size-check` (`scripts/skill-size-check.sh`), enforcing agentskills.io's `SKILL.md` ceiling as two blocking gates: `MAX_LINES=500` and `MAX_WORDS=2770` (a word-count proxy for the 5,000-token limit, calibrated to the densest prose measured in this repo — 1.81 tokens per word — so even a worst-case `SKILL.md` at the ceiling stays under 5,000 tokens). Both are inclusive, and `skill-audit/scripts/validate.sh` checks the same pair on the same terms, so a `SKILL.md` can no longer pass its own audit yet be blocked by the commit hook. Scoped to `^plugins/[^/]+/skills/[^/]+/SKILL\.md$` only, same as `vale-audit-prefilter-skill`, so it never lints `docs/research/examples/` reference skills. It's also exposed in the root-level `.pre-commit-hooks.yaml` as `kyberforge-skill-size-check` — it has no external asset dependency, so it needed no relocation, only exposure to external consumers. File scope (`SKILL.md` + agent files) and enforcement model (rules land directly in `styles/Kyberforge`, blocking immediately, no trial tier) stay unchanged; governance.md/CONTROLS.md were evaluated and excluded as rule sources (nothing prose-pattern-matchable to mine). House convention: banned phrasing that must be mentioned rather than used goes in backticks or a fenced code block — Vale skips code spans and fences, so no suppression is needed; inline `` (HTML-comment form; the MDX `{/* */}` form does not work in plain Markdown) is the fallback only where backticking is impossible. +This scope expands per ADR-0013: one cherry-picked low-noise `write-good`/`alex` rule landed in `styles/Kyberforge`, `Kyberforge.SentenceOpenerThereIs` (22 held-out hits, both in-corpus hits clean rewrites, zero suppressions). A second, `Kyberforge.VagueQualifier`, was cherry-picked and then deleted: 2 hits across the 41 skill/agent files, one marginal and one an unfixable false positive (`caveman/SKILL.md` quotes `of course` as an example of filler — a mention, not a use) that forced the repo's only Vale suppression comments. Also new is a sibling pre-commit hook, `skill-size-check` (`scripts/skill-size-check.sh`), enforcing agentskills.io's `SKILL.md` ceiling as two blocking gates: `MAX_LINES=500` and `MAX_WORDS=2770` (a word-count proxy for the 5,000-token limit, calibrated to the densest prose measured in this repo — 1.81 tokens per word — so even a worst-case `SKILL.md` at the ceiling stays under 5,000 tokens). Both are inclusive, and `skill-audit/scripts/validate.sh` checks the same pair on the same terms, so a `SKILL.md` can no longer pass its own audit yet be blocked by the commit hook. Scoped to `^plugins/[^/]+/\.apm/skills/[^/]+/SKILL\.md$` only, same as `vale-audit-prefilter-skill`, so it never lints `docs/research/examples/` reference skills. It's also exposed in the root-level `.pre-commit-hooks.yaml` as `kyberforge-skill-size-check` — it has no external asset dependency, so it needed no relocation, only exposure to external consumers. File scope (`SKILL.md` + agent files) and enforcement model (rules land directly in `styles/Kyberforge`, blocking immediately, no trial tier) stay unchanged; governance.md/CONTROLS.md were evaluated and excluded as rule sources (nothing prose-pattern-matchable to mine). House convention: banned phrasing that must be mentioned rather than used goes in backticks or a fenced code block — Vale skips code spans and fences, so no suppression is needed; inline `` (HTML-comment form; the MDX `{/* */}` form does not work in plain Markdown) is the fallback only where backticking is impossible. ### LESSONS.md Long-loop feedback log for patterns observed across sessions. Three or more entries on the same pattern graduate to the relevant standing file (e.g. a coding convention, a governance rule). Updated by the session-handoff skill or directly by the human. Lives at the repo root. diff --git a/LESSONS.md b/LESSONS.md index 835a386..074bb83 100644 --- a/LESSONS.md +++ b/LESSONS.md @@ -2,7 +2,7 @@ Patterns observed during development of this repo. Three or more entries on the same pattern → promote to CONTEXT.md (or the relevant instruction file) as a standing rule. -**Graduation rule:** When three or more entries cover the same pattern, the human reviews and promotes it to the appropriate standing location: `CONTEXT.md` for domain-level principles, `core/instructions/coding.md` for coding conventions, `core/instructions/git.md` for git conventions, or `core/instructions/testing.md` for testing conventions. The graduated entries are marked `[graduated → target file]` rather than deleted (audit trail). +**Graduation rule:** When three or more entries cover the same pattern, the human reviews and promotes it to the appropriate standing location: `CONTEXT.md` for domain-level principles, `core/instructions/coding.md` for coding conventions, `core/instructions/testing.md` for testing conventions, or `core/instructions/subagent-orchestration.md` for delegation conventions. Those four are the whole set — `core/instructions/` holds `coding.md`, `governance.md`, `subagent-orchestration.md` and `testing.md`, and nothing else. Git conventions have no standing file of their own: promote them to `core/instructions/coding.md`, or create a new instruction file deliberately rather than assuming one exists. The graduated entries are marked `[graduated → target file]` rather than deleted (audit trail). **Who writes here:** The session-handoff skill (Chunk 3) prompts LESSONS.md extraction before closing a session. The human may also write directly. @@ -26,6 +26,8 @@ Issue files frequently referenced "the workflow defined in `docs/notes/skill-imp The repo CLAUDE.md instructs agents to read CONTEXT.md at session start, but agents skip this in practice — defaulting to reading only what's directly relevant to the immediate prompt (e.g. the skills folder). The governance.md works because `@import` is technically enforced by Claude Code. Fix: (1) add `@CONTEXT.md` to repo CLAUDE.md using `@import` to make it always-loaded; (2) add a "Key decisions" section to CONTEXT.md with one-line resolved-ADR summaries so locked choices are always in context. +**Status (2026-08-14): neither part landed.** Root `CLAUDE.md` imports `@AGENTS.md` only — no `@CONTEXT.md` — and `CONTEXT.md` has no "Key decisions" section. The behavioral hope this entry diagnosed is still the only mechanism in place: `AGENTS.md` carries the line "Read CONTEXT.md at the start of every session in this repo," which is loaded but is itself an instruction, not an import. The proposal above is open work, not a record of a completed change. + ## 2026-05-17 — Instruction rules lose to RLHF defaults without specificity Behavioral tests (2026-05-17) showed three communication/behavior rules failing: exploratory question format (gave verbose multi-bullet answer instead of 2-3 sentences), file edit intent (asked for clarification instead of stating intent and proceeding), and push confirmation (went straight to tool call instead of asking first). All three rules are present in `providers/claude-code/CLAUDE.md` as one-liner statements. The RLHF-trained defaults (thorough answers, risk-averse clarification seeking, fast execution) consistently outcompete thin rules. Fix: rewrite failing rules with specificity, a counter-example, and a boundary statement — not just a single-line imperative. @@ -124,7 +126,7 @@ Two forks independently fixed `references/sources.md` with different approaches ## 2026-06-28 — Implementation agents must invoke /skill-author, not write skill files directly -When briefing an agent to implement a new skill, the instinct is to tell it to write the SKILL.md and supporting files directly. This bypasses Step 5 of the skill-author process (provenance), which requires reading all research `sources.md` files and recording every `extracted` slug in META.md. The `validate-provenance.sh` script catches the gap — but only after the commit, requiring a fix round. This pattern recurred twice in one session (plugin-author and marketplace-author initial implementation, then again in the first round of fix agents). Fix: briefs for implementation agents must explicitly say "invoke `/skill-author` (read and follow `plugins/kyberforge/skills/skill-author/SKILL.md`)" — not "write the skill files." Invoking the skill is the only reliable way to ensure all process gates, including provenance, run. +When briefing an agent to implement a new skill, the instinct is to tell it to write the SKILL.md and supporting files directly. This bypasses Step 5 of the skill-author process (provenance), which requires reading all research `sources.md` files and recording every `extracted` slug in META.md. The `validate-provenance.sh` script catches the gap — but only after the commit, requiring a fix round. This pattern recurred twice in one session (plugin-author and marketplace-author initial implementation, then again in the first round of fix agents). Fix: briefs for implementation agents must explicitly say "invoke `/skill-author` (read and follow `plugins/kyberforge/.apm/skills/skill-author/SKILL.md`)" — not "write the skill files." Invoking the skill is the only reliable way to ensure all process gates, including provenance, run. ## 2026-07-05 — Repo root is a bare checkout; work happens in worktrees only @@ -165,3 +167,13 @@ Dropping the `--config` argument from `.pre-commit-hooks.yaml` was the right fix ## 2026-08-09 — A documented impossibility is a claim, not a constraint `vale-wrap.sh` flattens multi-line YAML `description:` scalars so Vale's `text.frontmatter.description` scope keeps matching. Its last-resort branch rewrote ASCII `'` to U+2019, justified at the emission site and in review as "the single combination no YAML scalar can carry verbatim" — an accepted-by-design residual, documented and test-covered, which is exactly why nobody retested it. The claim was false: a `|-` literal block with one indented content line carries `'`, `"`, `\` and `: ` verbatim, keeps the scope alive, and the wrapper's own header docstring already said literal blocks were unaffected. The cost of the unexamined claim was a silent underlint on 12 of 54 in-scope files — any rule whose token contained an apostrophe simply never fired, and the covering test (case 20) pinned only "the scope stays alive", so it passed either way. Fix: when a residual is accepted because something is "impossible", write down the specific claim in a falsifiable form and test *that*, not the workaround built on top of it. The tell here was that the residual and its justification were documented in the same breath by the same author — documentation records a belief, and a belief adjacent to a workaround is the one most worth attacking. Related: an assertion written to cover an accepted residual tends to assert the residual's *presence* rather than the behaviour it costs; case 20b asserted the scope survived flattening, never that a rule matching the rewritten characters still fired. + +## 2026-08-14 — A fix handed down with authority is the least-reviewed code in the change + +Across one review round, four fixes specified by the orchestrating reviewer were wrong, and every one would have shipped a guard that looked correct and caught nothing — the same defect class the guard was written to close. `nproc([[:space:]]|$)` does not match `$(nproc)`, the only spelling that occurs in real code. `grep -E ... | grep -Evq ...` under `set -o pipefail` returns 141 because `-q` exits on first match and SIGPIPEs the upstream, and 141 as an `if` condition reads as "no findings" — worse, it is *size-dependent*, so on the real 4-line `.vale.ini` the broken form behaves correctly and only fails once the input grows. `FUNCNAME` and `BASH_ARGC` were proposed as never-empty shell arrays to exempt from an unguarded-expansion scan; both are empty in reachable states (outside a function; `BASH_ARGC` measured 1 at top level and 0 inside a function), so exempting them suppresses a real bash 3.2 abort. `sed 's/#.*//'` as a comment-stripper truncates at the `#` in `${var#prefix}` — a form this repo actually uses at `check-manifests.sh:58` — reintroducing the exact blind spot being fixed. Each was caught only because the implementing agent re-derived the fix and measured, rather than applying what it was told; each had survived being written down confidently in a numbered finding with a reproduction attached. The asymmetry is the point: a finding arrives with evidence and gets scrutinised, while the fix beside it arrives with the same authority and gets implemented. Fix: state a proposed fix as a hypothesis with its own falsifiable check, and require the implementer to verify the fix mechanism independently of the defect reproduction — the two are different claims. The tell is a fix whose correctness depends on a regex boundary, a shell exit-status rule, or an "always/never" property of a builtin: measure it at the size, scope, and spelling it will actually meet, because the small case and the shipped case can disagree. + +## 2026-08-14 — Every assertion needs a revert it provably fails against [graduation candidate] + +Mutation testing a review round's own fixes found repeatedly that a passing test was pinning nothing. Deleting `sync_dir`'s stale-directory wipe, its check-mode stale branch, or three of five `MIRROR_DIRS` entries each left the suite at 18/18 green; so did replacing the hooks trailing-newline normalisation with plain `cp`. A pair of concurrency assertions written to guard a reentrancy defect caught it 0 times in 10 runs against the deliberately broken script — and one of them was structurally incapable of ever catching it, because the broken code wrote to the system temp dir while the assertion inspected `$TMPDIR`. A fixture-leak fix ran green with and without the fix, verified only by external observation. Two manifest fixtures passed with the canonicalisation they claimed to cover deleted, rescued by an unrelated name-matching axis. In each case the test named the right behaviour in its description and asserted something adjacent to it. The cheap discipline that finds all of these: for every assertion, construct the revert it is supposed to catch and confirm it fails — and when an assertion survives every revert you can think of, that is not reassurance, it is the finding (one test only revealed itself as decoration once a sixth, differently-targeted revert was built for it). Fix: treat "which revert does this fail against?" as a required answer at the time an assertion is written, and record it where the assertion lives, since a test's own description is exactly the artifact that made the gap invisible. + +Graduation candidate: this overlaps 2026-08-09's "an assertion written to cover an accepted residual tends to assert the residual's presence rather than the behaviour it costs" and the same date's "assert on the expected members, so a derivation whose input vanished fails loudly instead of quietly covering less." Three entries circling one pattern — human review for promotion to `core/instructions/testing.md`. diff --git a/apm.yml b/apm.yml new file mode 100644 index 0000000..4afd41e --- /dev/null +++ b/apm.yml @@ -0,0 +1,75 @@ +name: holocron +version: 0.3.4 +description: AI development skills for Claude Code and GitHub Copilot CLI — factory, design, implement, review, and cross-cutting workflows. +license: MIT +marketplace: + # apm's Claude marketplace mapper only emits description:/version: into the + # compiled marketplace.json when set explicitly here (an override) — the + # top-level apm.yml description:/version: above are NOT inherited into the + # compiled output despite being used elsewhere (e.g. by `apm audit`). + description: AI development skills for Claude Code and GitHub Copilot CLI — factory, design, implement, review, and cross-cutting workflows. + version: 0.3.4 + owner: + name: Defame1297 + email: defame1297@rkdr.net + url: https://git.dev.rkdr.net/Defame1297/ + + # Default tag pattern used to resolve version ranges for each package. + build: + tagPattern: "v{version}" + + # Output targets (map form). Each output writes to its profile default + # path; add 'path:' under a key to override. + # 'codex' requires every package below to declare 'category:' (satisfied). + outputs: + claude: {} + codex: {} + + # CI tip: build one or all formats with a machine-readable manifest: + # apm pack --marketplace=claude,codex --json | jq -r '.marketplace.outputs[].path' + + versioning: + strategy: per_package + + packages: + - name: kyberforge + description: Skills and agents for creating, maintaining, and managing a Claude Code / Copilot CLI plugin marketplace. + source: ./plugins/kyberforge + version: 1.4.1 + category: Developer Tools + + - name: bin + description: A place for things to be binned + source: ./plugins/bin + version: 1.1.2 + category: Utilities + + - name: git + description: Skills for working with Git — conventional commits, branch management, pull requests, and feature flow. + source: ./plugins/git + version: 1.3.3 + category: Version Control + + - name: gitea + description: Skills for managing Gitea repositories — issues, pull requests, milestones, releases, and wikis. + source: ./plugins/gitea + version: 1.3.4 + category: Version Control + + - name: core + description: Skills for authoring and auditing a repo's AGENTS.md and the provider adapter files that defer to it. + source: ./plugins/core + version: 1.1.1 + category: Productivity + + - name: mattpocock-skills + description: Skills for Real Engineers — planning, TDD, architecture, and debugging workflows from Matt Pocock's .claude directory. + source: mattpocock/skills + version: "1.2.3" + category: Productivity + + - name: lint + description: Skills and agents for configuring and running linters. + source: ./plugins/lint + version: 1.1.6 + category: Developer Tools diff --git a/docs/adr/0001-skills-in-agents-dir.md b/docs/adr/0001-skills-in-agents-dir.md index 05ffd41..528d0f6 100644 --- a/docs/adr/0001-skills-in-agents-dir.md +++ b/docs/adr/0001-skills-in-agents-dir.md @@ -1,5 +1,16 @@ # Skills are distributed via plugins, not monolithic repo deployment +**Superseded by:** ADR-0015 (Microsoft APM replaces the hand-authored plugin/marketplace model +as this repo's authoring source of truth) and, for plugin-scope agent files specifically, +ADR-0016 (plugin-scope `.apm/agents/*.agent.md` drops provider-specific fields). Since issue +#90's conversion executed, plugin content is authored under `plugins//apm.yml` + +`.apm/{skills,agents,hooks}/` — not the flat `skills/`/`agents/` layout this ADR describes — +and `.claude-plugin/plugin.json`/`.github/plugin/plugin.json` are compiled output of `apm pack`, +not hand-authored. This ADR's content is kept below as the historical record of the +pre-APM decision; it is no longer the current model. + +--- + Skills (slash commands) are authored and distributed as part of **plugins** — each plugin contains its own `skills/` directory alongside agents and other artifacts. Plugins are installed via `claude plugin install @holocron` rather than deployed from the repo's local tree. This decision decouples skill authoring cadence from core provider deployments and allows independent versioning per plugin. ## Context diff --git a/docs/adr/0006-plugin-version-parity.md b/docs/adr/0006-plugin-version-parity.md index 769da91..e677929 100644 --- a/docs/adr/0006-plugin-version-parity.md +++ b/docs/adr/0006-plugin-version-parity.md @@ -1,5 +1,18 @@ # version field is present in both plugin manifests +**Moot as of ADR-0015.** This ADR addressed drift risk between two independently +*hand-maintained* manifests. Since issue #90's conversion executed, `.claude-plugin/plugin.json` +and `.github/plugin/plugin.json` are both **compiled output** of `apm pack`, generated in the +same pass from a single `apm.yml` per plugin — there is no longer a second hand-authored file +that could drift out of parity. The invariant this ADR required (`version` present and +identical in both manifests) still holds in the compiled output, but structurally, not because +a skill enforces it: both files are derived from the same `apm.yml` `version:` field, so +divergence is no longer possible by construction. `plugin-author`, the skill that enforced this +invariant, is deleted per ADR-0015 rather than adapted. Kept below as the historical record of +the pre-APM decision. + +--- + Each plugin has two manifests: `plugin.json` (Copilot CLI) and `.claude-plugin/plugin.json` (Claude Code). Both tools support a `version` field. Prior to this decision, only the CC manifest carried `version`; the Copilot manifest omitted it. We now require `version` in both manifests, always identical. A reader of `plugin.json` alone should be able to determine the plugin version without consulting the CC manifest. The `plugin-author` skill enforces this invariant on every create, update, and release operation. diff --git a/docs/adr/0007-gitea-canonical-issue-tracker.md b/docs/adr/0007-gitea-canonical-issue-tracker.md index 18fe8c6..7cd7d7a 100644 --- a/docs/adr/0007-gitea-canonical-issue-tracker.md +++ b/docs/adr/0007-gitea-canonical-issue-tracker.md @@ -2,6 +2,8 @@ **Supersedes:** ADR-0011 (provider-agnostic issue tracker with file-based default — archived during refactoring) +> **Note on the ADR-0011 number.** Every "ADR-0011" on this page means the *archived* provider-agnostic issue tracker ADR, which no longer exists in `docs/adr/` — it was removed when it was superseded, and the number 0011 was later reused for an unrelated decision, `docs/adr/0011-gitea-skill-deep-modules.md` (the gitea skill's split into deep modules). That file is not the ADR referenced below. The number is not renumbered here: these ADRs are a published record and renumbering would break every citation that already points at either one. The archived text is recoverable from git history. + ADR-0011 established a provider-agnostic model with `docs/issues/NNNN-.md` as the file-based default, switching to Gitea MCP at runtime when available. The interim model was justified because Gitea would not be configured until after Chunk 3, and the repo needed to work before then. Gitea is now configured and in active use. The condition in ADR-0011 has been met. This ADR supersedes it. diff --git a/docs/adr/0010-agent-sources-relocated-outside-agents-dir.md b/docs/adr/0010-agent-sources-relocated-outside-agents-dir.md index 3860472..edbde4d 100644 --- a/docs/adr/0010-agent-sources-relocated-outside-agents-dir.md +++ b/docs/adr/0010-agent-sources-relocated-outside-agents-dir.md @@ -5,6 +5,21 @@ claim that "both files share a single `agents/sources.md` for provenance." The r ADR-0005 (dual-provider generation, scope detection, single-root script interface) is unaffected and remains in force. +**Path update per ADR-0016:** at plugin scope, agent files no longer live at +`/agents/.md`. The authoring source is now +`/.apm/agents/.agent.md` — a single vendor-neutral file (no dual Claude/ +Copilot pair) compiled to both targets via `apm pack`. See ADR-0016 for why (the field-dropping +rationale, `tools:` incompatibility, the compiled-output mechanics) — not restated here. This +ADR's own conclusion is unaffected by that move: the provenance file still belongs at +`/sources.md`, outside any directory `claude plugin validate --strict` +auto-scans, and `.apm/agents/` is, if anything, further removed from plugin-root than the old +flat `agents/` directory was, so the reasoning below still holds. References below to +`/agents/` describe the pre-APM layout in effect when this decision was made. +**Scope boundary (per ADR-0016):** this path change is plugin scope only. Project scope +(`.claude/agents/` + `.github/agents/`) and user scope (`~/.claude/agents/` + +`~/.copilot/agents/`) are unaffected — they are not APM packages and keep the dual-file +Claude+Copilot pair model this ADR originally described. + `claude plugin validate --strict` auto-discovers every `.md` file directly under a plugin's `agents/` directory and treats it as an agent definition requiring YAML frontmatter (`name`, `description`, etc.). A flat provenance file at `agents/sources.md` — no frontmatter, by diff --git a/docs/adr/0014-vale-prefilter-ships-from-the-plugin.md b/docs/adr/0014-vale-prefilter-ships-from-the-plugin.md index 91a6e91..b93ddcc 100644 --- a/docs/adr/0014-vale-prefilter-ships-from-the-plugin.md +++ b/docs/adr/0014-vale-prefilter-ships-from-the-plugin.md @@ -18,9 +18,9 @@ full LLM judgment every time outside this repo — the exact gap ADR-0013 named the plugin itself, following the no-cross-skill-path rule already established in `skill-author/references/deployment-modes.md` (a plugin's cache-install only copies each skill's own files; there is no plugin-level shared directory). `agent-audit` needs both `Kyberforge` and -`KyberforgeCopilot` (it lints `.agent.md` files), so `plugins/kyberforge/skills/agent-audit/assets/vale/` +`KyberforgeCopilot` (it lints `.agent.md` files), so `plugins/kyberforge/.apm/skills/agent-audit/assets/vale/` is the canonical, superset copy. `skill-audit` needs a second, smaller copy -(`plugins/kyberforge/skills/skill-audit/assets/vale/`, `Kyberforge` only) since it cannot +(`plugins/kyberforge/.apm/skills/skill-audit/assets/vale/`, `Kyberforge` only) since it cannot reference agent-audit's copy across the skill boundary. Both skills' Step 1 now resolve `scripts/vale-wrap.sh`/`assets/vale/.vale.ini` relative to their own directory, the same way `scripts/validate.sh ` already does — no new resolution mechanism, just applying the @@ -38,7 +38,7 @@ and gets all three, fully decoupled from Claude Code. CI is the identical `pre-c **This repo's own dev-time gate** consumes the same plugin-bundled copies instead of a third root-level copy — per explicit instruction, this repo should be set up like any other consumer would be, not dogfood a special root-only path. The existing `repo: local` hook is retargeted -(not removed): `entry:` now points at `plugins/kyberforge/skills/{skill-audit,agent-audit}/scripts/vale-wrap.sh`. +(not removed): `entry:` now points at `plugins/kyberforge/.apm/skills/{skill-audit,agent-audit}/scripts/vale-wrap.sh`. `repo: local` is kept rather than switching to a pinned self-reference (`repo: , rev: `) — a pinned self-reference would lint working-tree edits against the *last tagged release*, not the change actually being made, which is wrong for the repo that @@ -58,15 +58,20 @@ single hook at agent-audit's copy silently scanned 0 SKILL.md files.) **The hook `entry:` is the wrapper alone; the wrapper self-locates its config.** pre-commit prefixes only `entry[0]` with the hook-repo clone path (`cmd = (prefix.path(cmd[0]), *cmd[1:])`); every later argument is handed to the process untouched and so resolves against the *consuming* -repo's root. A `--config plugins/kyberforge/skills/…/assets/vale/.vale.ini` in +repo's root. A `--config plugins/kyberforge/.apm/skills/…/assets/vale/.vale.ini` in `.pre-commit-hooks.yaml` therefore named a path no consumer has, and every external run died with `E100 [--config] Runtime error`. The external-consumer contract this ADR exists to establish cannot be expressed as a `--config` argument at all — the config path has to be derived inside the process, from the script's own location. `vale-wrap.sh` accordingly defaults to its sibling `assets/vale/.vale.ini`, resolved from `${BASH_SOURCE[0]}`, whenever no `--config` is supplied; an explicit `--config` from any other caller still wins and still resolves against the caller's -cwd, so both audit skills' Step 1 (`--config assets/vale/.vale.ini`) is unaffected. Both -manifests now carry the identical argument-free `entry:`. Keeping them identical is part of the +cwd. Both audit skills' Step 1 passes no `--config` either, for the same reason and one more: a +relative `--config assets/vale/.vale.ini` resolves against the cwd, not against the skill +directory the wrapper path was resolved from, so it yields `E100 Runtime error … does not exist` +and exit 2 — which both skills' fallback misreads as "vale unavailable" and silently downgrades +to full LLM judgment, the exact failure the self-location exists to prevent. Both `SKILL.md` Step +1 sections say so explicitly ("Pass no `--config`"), and both manifests now carry the identical +argument-free `entry:`. Keeping them identical is part of the decision: the local `repo: local` hook resolved its `--config` correctly only because the consuming repo *was* this repo, and that one difference is why three review rounds exercised a code path no external consumer ever takes. @@ -105,8 +110,8 @@ doesn't wonder if it was overlooked. ## Consequences - Root `.vale.ini`, `styles/`, `scripts/vale-wrap.sh` are deleted. Two copies remain: - `plugins/kyberforge/skills/agent-audit/assets/vale/` (canonical, superset) and - `plugins/kyberforge/skills/skill-audit/assets/vale/` (subset, `Kyberforge` only). + `plugins/kyberforge/.apm/skills/agent-audit/assets/vale/` (canonical, superset) and + `plugins/kyberforge/.apm/skills/skill-audit/assets/vale/` (subset, `Kyberforge` only). - `plugins/kyberforge`'s `plugin.json` and `.claude-plugin/plugin.json` both patch-bump for every shipped content change (per ADR-0006's version-parity invariant): `1.2.5` for the relocation itself, `1.2.6` for the self-locating `vale-wrap.sh` that followed. diff --git a/docs/adr/0015-apm-replaces-plugin-marketplace-authoring.md b/docs/adr/0015-apm-replaces-plugin-marketplace-authoring.md index e64e07d..77b8a0e 100644 --- a/docs/adr/0015-apm-replaces-plugin-marketplace-authoring.md +++ b/docs/adr/0015-apm-replaces-plugin-marketplace-authoring.md @@ -1,7 +1,9 @@ # Microsoft APM replaces the hand-authored plugin/marketplace model as this repo's authoring source of truth -**Will supersede:** ADR-0001 ("Skills are distributed via plugins... each plugin contains its -own `skills/` directory") — once issue #90's conversion actually executes; not yet in effect. +**Status: executed (2026-08-12, issue #90).** All six plugins now carry `apm.yml` + `.apm/` as +their authoring source; `.claude-plugin/marketplace.json` and every plugin's `plugin.json` are +`apm pack`-compiled output. **Supersedes ADR-0001** ("Skills are distributed via plugins... each +plugin contains its own `skills/` directory") — in effect. This repo replaces its hand-maintained Claude Code plugin/marketplace authoring model (`.claude-plugin/marketplace.json` + per-plugin `plugin.json`) with Microsoft APM (`apm.yml` + @@ -31,10 +33,9 @@ new hand-maintained manifest format. and per-provider `plugin.json` files become **compiled output** via `apm compile`/`apm pack`, generated from `apm.yml` + `.apm/` per plugin, extensible to other `apm runtime`-supported providers without hand-maintaining a separate manifest per provider. -- **This will supersede ADR-0001** ("Skills are distributed via plugins... each plugin - contains its own `skills/` directory"), but not yet — supersession is pending on issue #90's - conversion. Once that real conversion executes, skills and agents physically move to - `plugins//.apm/skills/` and `plugins//.apm/agents/*.agent.md`. +- **This supersedes ADR-0001** ("Skills are distributed via plugins... each plugin + contains its own `skills/` directory"). Executed in issue #90: skills and agents physically moved + to `plugins//.apm/skills/` and `plugins//.apm/agents/*.agent.md`. - New operational tooling — `apm-install` (skill), `apm-workflow` (skill), `apm-orchestrate` (agent) — lands in `kyberforge`, tracked in issue #88 (https://git.dev.rkdr.net/Defame1297/holocron/issues/88). @@ -49,13 +50,12 @@ new hand-maintained manifest format. routing: `apm compile`/`apm pack` will generate `.claude-plugin/marketplace.json` and per-provider `plugin.json` directly from `apm.yml` + `.apm/`, so `apm-install`/`apm-workflow`/ `apm-orchestrate` (issue #88, already landed on this branch) fully replace what these two skills - did. Deleting `plugin-author`/`marketplace-author` is part of issue #90's execution, not #89's. -- Actually translating the existing plugins into `apm.yml` + `.apm/` and running the real - conversion is deferred to issue #90 - (https://git.dev.rkdr.net/Defame1297/holocron/issues/90). -- `CONTEXT.md`'s "Plugin"/"Plugin marketplace" glossary entries get a forward-pointer to this - ADR so a session-start read surfaces the pending change; their substantive definitions remain - accurate until issue #90's conversion actually executes — this ADR does not rewrite them. + did. `plugin-author`/`marketplace-author` were deleted in issue #90's execution. +- Translating the existing plugins into `apm.yml` + `.apm/` and running the real conversion was + executed under issue #90 (https://git.dev.rkdr.net/Defame1297/holocron/issues/90), which tracks + that work through to merge. +- `CONTEXT.md`'s "Plugin"/"Plugin marketplace" glossary entries were rewritten in issue #90 to + describe the compiled-output model directly, rather than carrying a forward-pointer to this ADR. ## Considered options @@ -89,27 +89,88 @@ correction) sorted what they document into three buckets: because of hand-authored dual manifests (ADR-0006's version-parity/patch-bump rule, the CC-vs-Copilot field-placement split, dual-file mirroring) are obsolete under `apm.yml`'s single-manifest model and were deliberately dropped. -- **Holocron policy choice — flagged for #90, not resolved here.** `marketplace-author`'s - catalog-version convention (minor bump for package add/remove, patch bump for field-only - updates) isn't an APM mechanic — `apm` doesn't enforce it — but it's still a meaningful holocron - policy worth re-applying to `apm.yml`'s `marketplace.packages[].version` field once #90 wires up - real marketplace authoring. Where/how that gets implemented is #90's decision, not this ADR's. +- **Holocron policy choice — resolved in #90.** `marketplace-author`'s catalog-version convention + (minor bump for package add/remove, patch bump for field-only updates) isn't an APM mechanic — + `apm` doesn't enforce it, and has no native version-bump automation at all — so rather than + building a new script, the convention is now documented as guidance inside `apm-workflow`'s + reference docs (`references/marketplace.md` for the root catalog version rule, + `references/configure.md` for the per-package version-bump-on-content-edit rule), applied + manually by whoever edits `apm.yml`. ## Consequences -- ADR-0001 is superseded once issue #90 executes. -- ADR-0006 (plugin-version-parity) becomes moot once #90 lands: `plugin.json`/`marketplace.json` - become compiled output of a single `apm.yml`, so there's no second hand-authored file left to - keep in parity, and `plugin-author` — the skill that enforced ADR-0006 — is deleted rather than - adapted (see "Content migration" above). Not resolved by this ADR. -- ADR-0010 (agent sources relocated outside agents dir) needs revisiting once agents move to - `plugins//.apm/agents/` — the directory path changes, not the pre-existing `.agent.md` - extension convention (ADR-0005/ADR-0010, unaffected) — not resolved by this ADR. -- ADR-0014 (Vale prefilter ships from the plugin) has hardcoded path regexes assuming - `plugins//skills/...`/`plugins//agents/...`; these will need updating once paths - move under `.apm/` — not resolved by this ADR. -- `kyberforge` gains three new artifacts (issue #88) before any conversion of existing content - happens. -- Two follow-up issues track the remaining work: #89 (`skill-author`/`agent-author` routing - adaptation) and #90 (the actual repo conversion, which also deletes `plugin-author`/ - `marketplace-author`). +- ADR-0001 is superseded (issue #90). +- ADR-0006 (plugin-version-parity) is moot (issue #90): `plugin.json`/`marketplace.json` are now + compiled output of a single `apm.yml`, so there's no second hand-authored file left to keep in + parity, and `plugin-author` — the skill that enforced ADR-0006 — was deleted rather than adapted + (see "Content migration" above). +- ADR-0010 (agent sources relocated outside agents dir) was updated (issue #90) for agents now + living at `plugins//.apm/agents/*.agent.md` — the directory path changed; the pre-existing + `.agent.md` extension convention (ADR-0005/ADR-0010) and project/user scope are unaffected, per + ADR-0016. +- ADR-0014 (Vale prefilter ships from the plugin) had its hardcoded `plugins//skills/...` + paths (the Vale prefilter is skill-scoped only; ADR-0014 never referenced a + `plugins//agents/...` path) updated for the `.apm/` nesting as part of issue #90's + execution. +- `kyberforge` gained three new artifacts (issue #88) before any conversion of existing content + happened, then lost two (`plugin-author`/`marketplace-author`, deleted once issue #90 verified + parity) — net version bump 1.3.1 → 1.4.0. The root marketplace catalog bumped 0.3.1 → 0.3.2 to + match. +- ADR-0016 (a narrower decision discovered while designing issue #89) turned out to gate how + issue #90 had to re-author plugin-scope agents: `.apm/agents/*.agent.md` compiles verbatim to + both Claude and Copilot, so those files carry only the fields in the `apm-agent-allowlist` section + of `plugins/kyberforge/.apm/skills/agent-audit/references/field-inventory.md` (as amended + 2026-08-14: `name`/`description`/`model`/`source_keys`/`disallowedTools`) — existing dual-file + `.md`+`.agent.md` pairs could not be raw-moved, only re-authored. +- Two follow-up issues tracked the remaining work: #89 (`skill-author`/`agent-author` routing + adaptation — closed, merged in #93) and #90 (the actual repo conversion, which also deleted + `plugin-author`/`marketplace-author` — tracked through to merge; treat #90's own state as the + authority on whether it has landed, not this line). +- **`displayName` is gone from all six compiled `plugin.json` files — accepted, not overlooked.** + `apm.yml` has no key that compiles to it: `synthesize_plugin_json_from_apm_yml` + (`apm_cli/deps/plugin_parser.py`) emits only `name`, `version`, `description`, `author`, + `license`, `homepage`, `repository` and `keywords`, and nothing in `plugin_manifest.py` adds + `displayName` afterwards. So every `plugins//.claude-plugin/plugin.json` now carries + `author`/`description`/`homepage`/`keywords`/`license`/`name`/`repository`/`version` (plus + `mcpServers` for `bin`) and no `displayName`. The field is optional — + `plugins/kyberforge/docs/research/docs/claude-code-plugins/api-reference.md:14` lists + `displayName` as `Required: No`, "Human-readable name shown in plugin manager" — which is why + `claude plugin validate --strict` still passes on all six. The visible cost is that the plugin + manager falls back to the bare `name` as each plugin's label. Accepted as the price of `apm.yml` + being the single authoring source: re-injecting `displayName` post-compile would mean a second + `reinject_*` workaround of the kind ADR-0017's amendment reserves for fields apm strips on a + factually wrong premise, and apm's premise here is simply that the key does not exist in its + schema. +- **`owner.email` was dropped by mistake and has been restored (2026-08-14).** An earlier revision + of this ADR listed `owner.email` alongside `displayName` as a field `apm.yml` "has no key that + compiles to." That was wrong. `apm_cli/marketplace/yml_schema.py:186` defines + `_AUTHOR_OBJECT_KEYS = frozenset({"name", "email", "url"})`, and an `email:` under root + `apm.yml`'s `marketplace.owner` block was empirically confirmed to compile straight through into + `.claude-plugin/marketplace.json`'s `owner`. The key is declared in root `apm.yml` again and the + compiled `owner` block is `{name, email, url}`. Only `displayName` is a genuine schema gap; this + one was a documentation error that removed working configuration. +- **`mattpocock-skills` is pinned to an exact version, and the pin is advanced by hand.** + Pre-conversion the entry was `{"repo": "mattpocock/skills", "source": "github"}` — an unpinned + reference that tracked the upstream default branch, so consumers got whatever was on it at + install time. The conversion first replaced that with `version: "^1.2.0"`, which was still not a + pin: a caret range has nothing to freeze it, because there is no lockfile for + `marketplace.packages[]`. `apm pack` re-resolved the range against upstream on **every** run, so + an upstream `v1.2.4` would immediately invalidate the committed `ref`/`sha` and fail + `apm-pack-check-clean` with exit 4 — blocking every push in the repo, triggered by a third party + at an unrelated moment, with no local change to explain it. Root `apm.yml` therefore declares an + exact `version: "1.2.3"`, which `apm pack` freezes into `.claude-plugin/marketplace.json` as + `ref: v1.2.3` + an explicit `sha`. Two consequences, both intended: the committed ref/sha is + genuinely reproducible and cannot move under the repo, and picking up a new upstream release is a + deliberate act — a human edits the `version:` string in root `apm.yml` and re-runs `apm pack`. + apm has no version-bump automation (established under "Versioning" in issue #90's plan), so an + ageing pin is the accepted cost of a push gate that only fires on this repo's own changes. + Note the pin does not make the entry offline-resolvable: an exact version still requires a + `git ls-remote`, which is why two pre-push hooks need the network (see `AGENTS.md`). +- **Caveat on "Status: executed" above:** issue #90's own execution comment flagged, before merge, + that Claude Code's ability to actually load content out of `.apm/` was unverified — that caveat + turned out to be a real defect, not a formality: the native installer has zero awareness of + `.apm/` and reported `Skills (0) Agents (0) Hooks (0)` on every plugin installed from this + marketplace. The manifest-compilation deliverable this ADR describes was genuinely complete; + runtime discoverability was not. Fixed in ADR-0017 (a second, compiled flat-directory content + mirror at each plugin root, generated by `scripts/sync-plugin-content.sh`) — see that ADR for + the root cause and the fix. diff --git a/docs/adr/0016-apm-agent-primitive-drops-provider-specific-fields.md b/docs/adr/0016-apm-agent-primitive-drops-provider-specific-fields.md index af4e0e2..46ce8c8 100644 --- a/docs/adr/0016-apm-agent-primitive-drops-provider-specific-fields.md +++ b/docs/adr/0016-apm-agent-primitive-drops-provider-specific-fields.md @@ -22,13 +22,17 @@ Code and Copilot CLI targets. This is unlike: Because the agent primitive ships the same frontmatter unchanged to both harnesses, two concrete incompatibilities surface: -1. **`tools:`** — Claude Code expects a space-separated tool-name string; Copilot CLI expects a - list drawn from its own alias vocabulary (`execute`/`read`/`edit`/`search`/`agent`/`web`). A - value correct for one harness is wrong for the other. +1. **`tools:`** — Claude Code expects tool names drawn from its own vocabulary, as a + comma-separated string or a YAML list (`agent-definition.md:37`); Copilot CLI expects a list + drawn from a different alias vocabulary (`execute`/`read`/`edit`/`search`/`agent`/`web`). The + incompatibility is the vocabulary, not the punctuation: a value correct for one harness names + tools the other does not have. 2. **Claude-only knobs with no Copilot equivalent** — `isolation`, `maxTurns`, `effort`, `memory`, `permissionMode`. Writing any of these means Copilot's copy carries frontmatter keys it doesn't recognize at all. Whether Copilot's agent loader ignores unknown keys or - errors on them is unconfirmed by research. + errors on them is unconfirmed by research. *(Still unconfirmed as of the 2026-08-14 amendment + below, which admits `disallowedTools` as an explicitly accepted risk rather than by resolving + this question.)* ## Decision @@ -36,6 +40,9 @@ At **plugin scope only** (destination package has an `apm.yml` at its root — a package compiled via `apm compile`), `.apm/agents/.agent.md` carries only `name`, `description`, `model`, and the prose body. No `tools:` field, no Claude-only fields, at all. +*(Narrowed by the 2026-08-14 amendment below: `disallowedTools` is admitted as a fifth allowed +field. `tools:` and every other Claude-only knob remain excluded on the reasoning given here.)* + Absent `tools:` means inherit-all-tools on both harnesses — the one value that is never wrong on either target, unlike a present, harness-specific value that is guaranteed wrong on at least one of them. @@ -70,11 +77,84 @@ harness. Tracking the breakage doesn't prevent it, and the chosen decision alrea equivalent visibility (a SUGGESTION finding) without ever shipping the wrong value in the first place. +## Amendment (2026-08-14): the write fence comes back as a denylist + +The decision above generalised from `tools:` to "no tool restriction at all". That over-reached. +The unportability argument is specific to the **allowlist**: Claude Code reads `tools:` as a +delimited string of its own tool names, Copilot CLI reads it as a list drawn from its +alias vocabulary (`execute`/`read`/`edit`/`search`/`agent`/`web`), so one value is wrong on one +harness. That reasoning stands, and `tools:` stays out of every plugin-scope agent. + +A **denylist** has no such conflict. The evidence for that splits three ways, and this amendment +states which part is which rather than asserting the whole as settled. + +**Confirmed — Claude Code honours it for plugin subagents.** +`plugins/kyberforge/docs/research/docs/claude-code-plugins/agent-definition.md:39` documents +`disallowedTools` as a "Denylist applied before `tools`… Takes precedence over `tools`", and — the +part that matters here — it is **not** in that document's plugin-subagent ignore list. Line 99 +names exactly three fields plugin agents silently ignore: `hooks`, `mcpServers`, `permissionMode`. +`disallowedTools` is absent from that list. Claude Code is also the harness where the fence is +actually wanted, so the field earns its place on this evidence alone. + +**Inferred — the field is very likely inert on Copilot CLI, but by analogy, not by documentation.** +`plugins/kyberforge/docs/research/docs/github-copilot-plugins/troubleshooting.md:50` and `:53` +record Copilot *silently ignoring* two agent frontmatter fields it does not process (`mcp-servers` +and `metadata` outside the cloud runtime) rather than erroring on them. That is a documented +tolerance for *known-but-unprocessed* keys, which is adjacent to, not identical to, tolerance for +an *unknown* key. No stronger evidence exists: a sweep of the vendored Copilot corpus +(`agent-definition.md`, `api-reference.md`, `troubleshooting.md`, `configuration.md`) documents +unknown-key handling nowhere. + +**Unverified — Copilot's loader behaviour on an unrecognised key.** Context item 2 above says this +is unconfirmed by research and that remains true; nothing found since changes it. An earlier +revision of this amendment claimed "an unrecognised frontmatter key is inert" as settled fact and +attributed it to apm's verbatim-copy behaviour. That attribution was a non-sequitur — verbatim copy +describes what *apm* does at compile time and says nothing about what *Copilot* does at load time — +and the claim contradicted this ADR's own Context section. + +**So this is an accepted risk, stated as one.** Blast radius if the inference is wrong and Copilot +errors on the key: the three affected plugin-scope agents fail to load under Copilot CLI. It is +loud, not silent; it is confined to three agents in three plugins; no other primitive and no Claude +Code path is affected; and the remedy is a one-line frontmatter deletion. What the denylist shape +*does* rule out categorically — independent of loader behaviour — is the failure mode that motivated +dropping `tools:` in the first place: a denied name the other harness does not recognise denies +nothing, so a mis-shaped value can never grant or misroute a capability. The risk is a load failure, +never a silent over-grant. That asymmetry is why the same verbatim copy that makes `tools:` +unshippable makes `disallowedTools` worth shipping. + +So the read-only orchestrator agents regain their write fence: `gitea-orchestrate`, +`apm-orchestrate` and `lint-runner` each carry `disallowedTools: Edit, Write, NotebookEdit` plus +explicit prose in the body stating the agent does not edit files. `git-orchestrate` is deliberately +excluded — it legitimately declared `edit` before the conversion and still needs to write. + +**Residual — the fence is partial, and the prose is doing more of the work than the field is.** +`disallowedTools: Edit, Write, NotebookEdit` denies exactly those three tools. It does not deny +`Bash`, and at plugin scope these agents carry no `tools:` and therefore inherit it, so +`bash -c 'echo … > f'` remains unfenced by frontmatter. Only the body prose covers that path. This +is not a regression introduced here — the pre-conversion `tools:` allowlists also granted `Bash`, +so the shell route was open then too — but the ADR should not credit the mechanism with more than +it delivers. Closing it would need a `disallowedTools` entry for `Bash`, which these agents cannot +take because they legitimately shell out. + +Net position: the allowlist stays dropped for the reason originally given, and the denylist is +admitted as the portable-by-construction half of what was lost. It restores a real, Claude-Code- +confirmed write fence against the tool-call path, not a complete write sandbox. The consequence +below is narrowed accordingly. + +Enforcement follows the decision: `agent-audit`'s plugin-scope validator reads its allowlist as +data from the `apm-agent-allowlist` section of +`plugins/kyberforge/.apm/skills/agent-audit/references/field-inventory.md`, and that line now reads +`name description model source_keys disallowedTools`. `disallowedTools` also stays in that file's +`claude-code-only-fields` list, which is not a contradiction — that list governs whether a field +may cross the CC/Copilot boundary in a real project/user-scope *pair*, a different question from +whether a field is safe under verbatim copy in a single vendor-neutral file. + ## Consequences -- Every plugin-scope APM agent loses per-agent tool restriction and any Claude-only capability +- Every plugin-scope APM agent loses per-agent tool *allowlisting* and any Claude-only capability (isolation, maxTurns, effort, memory, permissionMode) until APM ships a real per-target integrator for the agent primitive. This is a known, accepted regression, not an oversight. + Tool **denial** is not part of that loss — see the 2026-08-14 amendment above. - **ADR-0005 is partially superseded** — its plugin-scope clause ("directory containing `plugin.json` is plugin scope → both files land in `/agents/`") no longer applies. Plugin scope is now "directory containing `apm.yml` → single vendor-neutral file lands in @@ -86,7 +166,9 @@ place. lists from `references/field-inventory.md` rather than hardcoding them, with a `source_keys` provenance chain — survives and is reused. Only the *content shape* changes for plugin scope: `field-inventory.md` shifts from two side-by-side CC-only/Copilot-only blocklists to one - vendor-neutral allowlist (`name`/`description`/`model`/`source_keys` — the last for provenance - tracking, validated separately by `validate-provenance.sh` against `sources.md`, not a - provider-specific field) for plugin-scope agents, while - continuing to serve its original two-blocklist role for project/user-scope validation. + vendor-neutral allowlist for plugin-scope agents, while continuing to serve its original + two-blocklist role for project/user-scope validation. That file's `apm-agent-allowlist` section + is the authoritative list and is read as data by `validate.sh`; as amended on 2026-08-14 it holds + `name`/`description`/`model`/`source_keys`/`disallowedTools` — `source_keys` for provenance + tracking, validated separately by `validate-provenance.sh` against `sources.md` rather than being + a provider-specific field, and `disallowedTools` per the amendment above. diff --git a/docs/adr/0017-plugin-content-mirror-bridges-apm-to-host-discovery.md b/docs/adr/0017-plugin-content-mirror-bridges-apm-to-host-discovery.md new file mode 100644 index 0000000..ad7856a --- /dev/null +++ b/docs/adr/0017-plugin-content-mirror-bridges-apm-to-host-discovery.md @@ -0,0 +1,346 @@ +# Plugin roots gain a compiled flat-directory mirror of `.apm/` content so Claude Code can discover it + +This ADR is a follow-on correction to ADR-0015 (Microsoft APM replaces hand-authored +plugin/marketplace authoring), discovered during issue #90's post-execution review. It does not +restate ADR-0015's rationale for adopting `.apm/` as the authoring source of truth — see that ADR +for the parent decision. It resolves the one question ADR-0015's own execution flagged as open but +did not block on: whether Claude Code's installer can actually load content out of `.apm/`. It +could not. + +**Status: executed (2026-08-13, issue #90).** `scripts/sync-plugin-content.sh` has been run +against all 6 plugins; flat `agents/`, `skills/`, `commands/` (etc., wherever `.apm/` populates +them), and a merged hooks file now exist at each plugin root as tracked, generated files. The +merged hooks file lands at `hooks/hooks.json`, not at the plugin root itself — see the second +amendment below, which corrects the path this ADR originally recorded. + +## Context + +ADR-0015's execution comment on issue #90 (2026-08-12) flagged, before merge: "it's currently +unverified whether Claude Code can actually discover any skill/agent content in these plugins... +This needs to be checked... before treating this conversion as functionally complete, not just +manifest-complete." That caveat did not block ADR-0015 from shipping "Status: executed" — the +manifest-compilation deliverable (`.claude-plugin/marketplace.json`/`plugin.json` generated from +`apm.yml` + `.apm/`) was genuinely complete, and every automated gate (`apm audit --ci`, +`claude plugin validate --strict` ×6, `apm marketplace check`) passed clean — so the ADR merged +with the caveat noted but unresolved. + +The caveat turned out to be a real defect, not a formality. `claude plugin install` against all +three plugins tested (`git@holocron`, `gitea@holocron`, `kyberforge@holocron`) reported +`Skills (0) Agents (0) Hooks (0)`. Root cause, confirmed two independent ways: + +1. **Claude Code's installer scans flat convention directories only.** `strings` on the installed + `claude` binary finds zero references to `.apm/` or `apm.yml` anywhere. The installed plugin + cache (`~/.claude/plugins/cache/holocron/kyberforge/1.3.1/`) mirrors the pre-conversion flat + `skills/`/`agents/`/`hooks/` layout verbatim — that is what the installer actually copies and + reads. `plugins/kyberforge/docs/research/docs/claude-code-plugins/configuration.md`'s own + "Plugin Directory Layout" table documents the same flat convention (`skills//SKILL.md`, + `agents/`, `hooks/hooks.json`, all "at the plugin root, not inside `.claude-plugin/`") — this + was accurate before ADR-0015 and never stopped being accurate; ADR-0015 moved plugin content + without adding a bridge to it. +2. **apm's own manifest compiler has no `.apm/` → host-path bridge, by design.** + `apm_cli/core/plugin_manifest.py`'s `build_plugin_manifest` docstring states directly: + "Convention directories (`agents/`, `skills/`, `commands/`) are auto-discovered by the host, so + they are never listed explicitly in the manifest." apm's Claude/Copilot compiler assumes plugin + content already lives in those flat root-level directories; it has no model of `.apm/` nesting + being host-visible at all, so it never emits anything that would point a host at `.apm/`. + +Separately, `apm_cli/bundle/plugin_exporter.py`'s `export_plugin_bundle` (the engine behind +`apm pack --format plugin`) *does* implement the correct mapping — `.apm/agents` → `agents/`, +`.apm/skills` → `skills/` (subdirs preserved), `.apm/prompts` + `.apm/commands` → `commands/` +(`*.prompt.md` renamed to `*.md`), `.apm/instructions` → `instructions/`, `.apm/extensions` → +`extensions/`, and `.apm/hooks/*.json` merged into one `hooks.json`. But it was only ever wired to +produce a distributable bundle under `build/-/` — a path nothing in root +`apm.yml`'s per-package `marketplace.packages[].source:` fields (e.g. `./plugins/bin`) or +`marketplace.json`'s equivalent points at. The correct mapping existed in apm's own codebase the +whole time; it was simply never connected to the path this repo's marketplace actually installs +plugins from. + +## Decision + +Each plugin root gains a second, generated content category, produced by +`scripts/sync-plugin-content.sh` (wraps `apm pack --format plugin`, copies the resulting bundle's +`agents/`, `skills/`, `commands/`, `instructions/`, `extensions/`, and merged hooks file back to +the plugin root — the hooks file to `hooks/hooks.json`, per the second amendment below) — same +governance status as `.claude-plugin/plugin.json`/`marketplace.json`: +**compiled output of `.apm/`, never hand-edited.** + +- `.apm/` remains the sole hand-edited authoring source, unchanged from ADR-0015. +- The flat mirror is what Claude Code's (and Copilot's) installer actually convention-scans at + install time — it exists purely to satisfy the host's discovery contract, a contract apm's own + manifest compiler deliberately does not bridge. +- `plugin.json`/`apm.lock.yaml`/`.mcp.json` from the bundle are excluded from the copy: + `plugin.json` is already correctly generated by a separate, already-verified apm code path + (`build_plugin_manifest`, run in the same `apm pack` invocation); `.mcp.json` is hand-authored + at the plugin root per ADR-0015 and is not an `.apm/` primitive. +- Dev-fixture `tests/` directories are excluded too — they are dev-time fixtures no plugin host + ever needs to discover, and several reference their own repo root through a hardcoded relative + walk-up sized for `.apm/`-nested depth, so a copy one directory level shallower breaks the + duplicate and double-runs the original under repo-wide bats discovery. The exclusion is + **depth-scoped to `//tests`**, deliberately: a skill may legitimately ship a + directory literally named `tests` as a template asset it scaffolds *from* + (`skills/skill-author/assets/templates/tests`, at depth 4). A depth-agnostic `-name tests` + matched that too and stripped it, making the mirrored `new-skill.sh` die mid-run on + `sed: can't read .../tests/README.md` — the scaffolder seds its way through the template tree + file by file. Scaffolding assets survive; fixtures do not. +- Drift is enforced by a pre-push gate (`scripts/sync-plugin-content.sh --check --all`, wired into + `.pre-commit-config.yaml` as hook id `check-plugin-content-sync` by a parallel workstream on + issue #90) — the same enforcement model `check-manifests.sh` already applies to the other + compiled-output category. `--check` alone is not the gate: the script requires either `--all` or + an explicit list of plugin directories, and run bare it prints usage and exits 1. `--all` derives + its work list from `marketplace.json`, a generated file, so it asserts its own coverage against + that list: it fails if it verified fewer plugins than the marketplace declares, not merely if it + verified none. A listed plugin whose `.apm/` has gone missing is skipped by the per-plugin sync + and would otherwise let the gate report success over a shrinking work list. +- Verified two ways before landing: `claude plugin validate --strict` passes on all 6 real + (non-scratch) plugin directories, and a live behavioral test + (`claude --plugin-dir plugins/kyberforge -p "list your skills and agents"`) against the real + committed directory confirms `kyberforge:*` skills and the `kyberforge:apm-orchestrate` agent + are now actually discovered — they were not, before this fix. +- The stale root-level `plugins//plugin.json` files (a near-duplicate of + `.claude-plugin/plugin.json` that nothing read or wrote, flagged separately in issue #90's + review) were deleted across all 6 plugins as part of the same cleanup. + +## Considered options + +**Patch `plugin.json`'s content-pointer fields to point directly at `.apm/` paths (rejected).** +Claude Code's manifest schema documents these as legitimate override fields that accept custom +paths — `plugins/kyberforge/docs/research/docs/claude-code-plugins/configuration.md` shows a real +example (`"skills": "./custom/skills/"`, `"agents": ["./custom/agents/reviewer.md"]`), so the host +side of this would work. Rejected because apm never emits such a pointer and would have to be +worked around on every run to make it do so. + +Be precise about the mechanism, because an earlier revision of this ADR overstated it. apm 0.28.0's +`build_plugin_manifest` (`apm_cli/core/plugin_manifest.py`) does carry a strip loop, but its field +list is `("agents", "skills", "commands", "instructions")` — `hooks` is **not** in it, and +`instructions` **is**, which this ADR previously did not mention. More to the point, that loop can +never fire: the manifest it operates on comes from `synthesize_plugin_json_from_apm_yml` +(`apm_cli/deps/plugin_parser.py`), which only ever emits `name`, `version`, `description`, +`author`, `license`, `homepage`, `repository` and `keywords`. The pointer fields are absent from +apm's output because `apm.yml` has no schema for them, not because apm actively removes them — the +`pop` loop is defensive dead code against a manifest shape apm does not produce. + +The rejection is unaffected by that correction, only its framing. Honoring this option would still +mean post-processing apm's compiled output on every `apm pack` run to add fields apm's schema has +no way to express, rather than reusing `plugin_exporter.py`'s bundle-export mapping, which already +does the right thing and only needed its output redirected to a path the installer reads. What it +is *not* is a fight against a load-bearing apm code path — the honest statement is that apm has no +input for these fields, and inventing one downstream is a workaround this ADR did not need. + +**Point `marketplace.json`'s `source:` at `apm pack`'s `build/-/` output directly +(rejected).** Would reuse the bundle exporter's correct mapping without adding a new script. +Rejected: `build/` is a version-suffixed, regenerate-on-every-pack directory — pointing the +marketplace at it would mean either committing a moving-target build artifact to version control +(defeating the point of it being generated) or requiring every consumer's marketplace to run +`apm pack` before install, a build step Claude Code's installer has no hook for — it clones/fetches +source and scans directories; it does not execute a package manager's build command first. +Copying the relevant subset back to the stable `plugins//` path — where `marketplace.json` +already points — needed no change to the marketplace source model at all. + +## Amendment (2026-08-13, revised 2026-08-14): Copilot's `plugin.json` gets an `mcpServers` *path* + +PR #95's review (a follow-on to this same issue #90 workstream) found a second field apm's +compiler drops for the Copilot ecosystem: `build_plugin_manifest` runs +`manifest.pop("mcpServers", None)` on every Copilot-ecosystem `plugin.json`, its docstring stating +the field is "not part of the Copilot plugin manifest schema." That claim is contradicted by this +repo's own researched documentation — +`plugins/kyberforge/docs/research/docs/github-copilot-plugins/configuration.md:49` documents +`mcpServers` as a valid, optional `plugin.json` field, typed **"string or object — MCP server +config path or inline definitions."** + +This is not the same situation "Considered options" above rejected. There, apm emits no pointer +because its schema has no input for one and the host auto-discovers the directories anyway, so +nothing is missing. Here a field Copilot actually reads is actively removed on a premise that is +wrong against documented Copilot behavior, and there is no auto-discovery mechanism that makes it +redundant. Shipping the manifest as apm produces it would ship a manifest known to be incomplete. + +`scripts/sync-plugin-content.sh`'s `reinject_mcp_servers()`, called from `sync_one()`, therefore +sets `mcpServers` on `.github/plugin/plugin.json` after `apm pack` runs — to the **string +`".mcp.json"`**, the path form of the documented type, not the resolved server objects. Only when +the plugin's `.mcp.json` declares at least one server, matching apm's own Claude-ecosystem builder, +which omits the field entirely rather than emitting `mcpServers: {}`. + +**The payload is a path because an inlined object is a credential-leak path.** The original +implementation copied `.mcp.json`'s resolved `mcpServers` object into the manifest with `jq`. That +route bypasses apm's own `_sanitize_mcp_servers()` (`apm_cli/core/plugin_manifest.py`), which +strips credential keys and redacts secret values out of `.mcp.json` precisely because — in its own +words — "copying them verbatim into a committed `plugin.json` would exfiltrate them into the +distributed artefact." Today's `.mcp.json` files here carry no `env` block, so nothing leaked; the +first one that did would have written a live token into a tracked, published manifest, with the +sanitizer sitting one code path away and never invoked. A path reference cannot carry a secret at +all: the manifest names a file, and resolution happens in the host at load time. This also matches +apm's documented posture for MCP secrets — `microsoft-apm/configuration.md:96-98` requires `${VAR}` +indirection so secrets are "never committed to the manifest." + +**Both modes re-inject**, not just real syncs: real mode writes into the plugin root directly, +`--check` into its throwaway copy first, so the manifest diff compares against the same content a +real sync would actually produce (see the script's own header). A check-mode re-injection is what +keeps `--check` from reporting permanent phantom drift on every plugin that ships an `.mcp.json`. + +This remains scoped to one field found to be incorrectly dropped. It does not reopen the +content-pointer option rejected above: those fields stay absent because apm has no schema input for +them and the host needs no pointer, which is a different situation from a documented field being +actively removed. + +Consequence: if a future apm release corrects the Copilot `mcpServers` omission, `reinject_mcp_servers()` +and its call site become dead code and should be deleted — nothing else in this ADR depends on the +reinjection existing beyond working around this specific upstream gap. + +Line numbers are deliberately omitted above. An earlier revision of this amendment cited +`reinject_mcp_servers()` at line 190 and its call site at line 269; both had already moved by the +next review round of the same PR, and moved again with the edits recorded in the amendment below. +A function name is stable enough to grep for; a line number in an ADR is stale by the next commit. + +## Amendment (2026-08-14): the merged hooks file lands at `hooks/hooks.json`, not the plugin root + +As originally executed, `sync-plugin-content.sh` wrote the merged hooks file to +`plugins//hooks.json`. That path is scanned by nothing. Claude Code convention-scans +`hooks/hooks.json`, and the "Plugin Directory Layout" table this ADR's own root-cause analysis +quotes above says so: +`plugins/kyberforge/docs/research/docs/claude-code-plugins/configuration.md:100` is the row naming +`hooks/hooks.json`, six lines below the table's preamble at `:94` — "All content directories must +be at the plugin root, not inside `.claude-plugin/`". The two are not the same line; an earlier +revision of this amendment said they were. The implementation read the preamble's "at the plugin +root" and dropped the file there, without reading the row that names the path. So this ADR shipped +with the contract quoted correctly in its diagnosis and violated in its output — the flat mirror +bridged skills and agents into discovery and left hooks exactly as undiscoverable as before the +fix. + +The merged file therefore moves to `plugins//hooks/hooks.json`. A root-level `hooks.json` +left over from a prior sync is stale output: a real sync deletes it, `--check` reports it as +drift. The real sync produced exactly these working-tree changes — `plugins/kyberforge/hooks.json` +and `plugins/lint/hooks.json` deleted, `plugins/kyberforge/hooks/hooks.json` and +`plugins/lint/hooks/hooks.json` created. Only those two plugins have an `.apm/hooks/` tree, so +only those two grow a mirrored hooks file at all. + +This does **not** reopen the "patch `plugin.json` pointer fields" option rejected above. The move +needs no `hooks` pointer in `plugin.json`: `hooks/hooks.json` *is* the convention path, so the +host finds it by auto-discovery, exactly as it finds `skills/` and `agents/`. The rejection stands +for the reason it was made, once stated accurately — apm emits no pointer field for any of these, +because `apm.yml` has no key that produces one, and none is needed when content sits at the +convention path. (`hooks` was never in `build_plugin_manifest`'s strip list at all; see the +corrected mechanism note under "Considered options".) Writing to the convention path is what makes +the no-pointer premise true here rather than something to work around. + +Read "the host finds it by auto-discovery" above as **Claude Code**, not both hosts. Copilot has no +default for `hooks` and so discovers none — a real gap, examined and deliberately left open in the +next amendment. + +## Amendment (2026-08-14): no `hooks` pointer is re-injected for Copilot — the gap stays documented + +PR #95's review found a third field, and it looks like the `mcpServers` amendment's exact twin: +`plugins/kyberforge/docs/research/docs/github-copilot-plugins/configuration.md:47` types `hooks` as +a `plugin.json` field, **"string or object"**, with **no default** — so Copilot has no convention +path to scan — and `jq 'has("hooks")'` returns `false` for all six `plugins/*/.github/plugin/plugin.json`. +Copilot therefore resolves **zero hooks from every plugin in this repo**. The facts are not in +dispute; the remedy is. + +State the mechanism correctly first, because it differs from `mcpServers` and the amendment above +depends on that distinction. `mcpServers` is *actively removed* — `build_plugin_manifest` runs +`manifest.pop("mcpServers", None)` on every Copilot manifest. `hooks` was **never in that strip +list** (its field list is `("agents", "skills", "commands", "instructions")`, and the loop is dead +code besides — see "Considered options"). This is an absence apm never fills, not a removal to +reverse. + +**Decision: do not re-inject. Document the gap.** The `mcpServers` exception was granted on three +conditions, and `hooks` meets only two of them: + +1. *A documented host schema field.* Met — `hooks` is in Copilot's own field table. +2. *apm has no input that produces it.* Met — `apm.yml` has no key for it. +3. *The payload is correct for the host regardless of content.* **Not met**, and this is the whole + difference. `.mcp.json` is one host-agnostic format that both ecosystems read, so the string + `".mcp.json"` is a true statement about the file no matter what is in it. Hooks have no such + shared format: Claude Code reads + `{"hooks": {"PreToolUse": [{"matcher": ..., "hooks": [...]}]}}` while Copilot requires + `{"version": 1, "hooks": {"sessionStart": [{"type": "command", "bash": ..., "powershell": ...}]}}` + — a mandatory `version`, lowercase and differently-named lifecycle events, and per-shell script + keys. apm's exporter merges `.apm/hooks/*.json` into **exactly one** `hooks.json` with no + per-target shaping (`_collect_hooks_from_apm`, `apm_cli/bundle/plugin_exporter.py`), and that one + file also sits at Claude Code's convention path, where Claude Code will read it whatever it + contains. So there is exactly one file and two incompatible readers of it. + +A `hooks` pointer would therefore assert that a Claude-shaped file is Copilot-shaped. That trades an +*incomplete* manifest for a *wrong* one, which is the opposite of the `mcpServers` amendment's +reasoning ("shipping the manifest as apm produces it would ship a manifest known to be incomplete"). + +The "it changes nothing today, so it is zero-risk and correct-by-construction for the first real +hook" argument does not survive the same check, in both halves. It is not inert today: both +`hooks/hooks.json` files are `{"hooks": {}}`, which lacks the `version: 1` Copilot's schema +requires, so a pointer would name a file invalid against the schema it is being pointed at from — +a change from "declares no hooks" to "declares hooks, at an invalid file". And it is not +correct-by-construction later: whoever writes the first real hook writes it in one of the two +shapes, and the pointer is wrong in the Claude-shaped case (the case that actually happens, since +Claude Code auto-discovers the same file and is what these hooks are authored against) while the +Copilot-shaped case breaks Claude Code instead. No content makes both readers correct. + +What would change this decision is upstream, not local: apm emitting a per-target hooks file (at +which point a pointer names a file genuinely shaped for its reader), or the two hook schemas +converging. Until then the honest artifact is a documented gap, recorded for authors in +`plugins/kyberforge/docs/hooks.md` and pinned by a test asserting the Copilot manifest carries no +`hooks` key — so that adding one is a deliberate act that has to confront the schema mismatch, +rather than a plausible-looking one-liner nobody re-derives. + +This does not weaken the `mcpServers` amendment. That exception was narrow on purpose, and this is +what its third condition was for. + +## Amendment (2026-08-14): symlinks under `.apm/` are dropped, and are now reported + +apm's bundle exporter filters symlinks out of the bundle entirely — `f.is_file() and not +f.is_symlink()` in `_collect_flat` and `_collect_recursive`, and the same test in +`_collect_hooks_from_apm` (`apm_cli/bundle/plugin_exporter.py`). It emits no warning. A symlink +placed under a plugin's `.apm/` therefore never reaches the mirror, and until now nothing said so. + +This was **silent content loss, not drift**, and that distinction is why no existing gate caught it. +Every other check in `sync-plugin-content.sh` compares the live mirror against a freshly synced +copy — and both sides are built from that same bundle. The symlink is absent from both, they agree, +and `--check` exits 0. There is no mismatch to detect, only an absence with nothing left to +mismatch against. Reproduced on a fixture: `ln -s real.md link.md` under `.apm/skills/hello/` +produced a mirror with no `link.md` and a `--check` at exit 0. + +`check_apm_symlinks()` therefore reads the `.apm/` **source** tree directly — the only place the +loss is visible — and reports each symlink in both modes, failing the run. It is reported rather +than resolved: dereferencing and copying the target would make a real sync emit content the bundle +does not contain, which is precisely the "reimplement apm's mapping outside apm" this ADR rejects. +Telling the author is the in-contract half. + +The scan covers only the `.apm/` directories apm's exporter actually reads +(`agents`, `skills`, `prompts`, `commands`, `instructions`, `extensions`, `hooks`), and carves out +`//tests` to match the mirror's own exclusion — that subtree is not mirrored whether +or not it holds a symlink, so nothing is lost there. The carve-out is depth-scoped for the same +reason the `tests/` exclusion is: a symlink under `assets/templates/tests` sits in content the +mirror does carry, and is reported. + +## Consequences + +- Git now tracks real, visible duplication: `.apm/skills//SKILL.md` and + `skills//SKILL.md` both exist and must match, likewise `.apm/agents/*.agent.md` vs. + `agents/*.agent.md`, and `.apm/hooks/*.json` vs. the merged `hooks/hooks.json` (see the + 2026-08-14 amendment above for that path). This is an accepted + tradeoff of bridging a gap apm itself doesn't close, not a bug — `.apm/` stays the single + hand-edited source, and the drift gate (`check-plugin-content-sync`) is what keeps the mirror + honest rather than trusting authors to remember to regenerate it by hand. +- `scripts/check-manifests.sh`'s existing blind spot (flagged in the same issue #90 review round: + it validated `plugin.json` fields that ADR-0015 already stopped populating, so a plugin shipping + zero content could pass it silently) is fixed as part of the same workstream: those field checks + are removed (nothing to check — the fields are correctly absent by design), and the + content-presence question they were standing in for is now answered by + `check-plugin-content-sync`, not re-implemented inside `check-manifests.sh`. +- ADR-0015's "Status: executed" now carries a pointer to this ADR (see that ADR's Consequences) + rather than being rewritten — the manifest-compilation half of its execution was correct and + stands; this ADR fixes the second, previously-unverified half. +- `CONTEXT.md`'s "Plugin" and "Plugin marketplace" glossary entries are updated to describe the + flat mirror as a second compiled-output category, alongside the existing + `.claude-plugin/plugin.json`/`marketplace.json` description. +- A future apm release that ships a native `.apm/`-aware plugin.json compiler (closing this gap + upstream) would let `sync-plugin-content.sh` and its drift gate be deleted outright — nothing in + this ADR's decision depends on the flat mirror existing beyond satisfying the current installer's + convention-scan contract. +- **Reproduction note (2026-08-13):** the live behavioral test cited in "Decision" above + (`claude --plugin-dir plugins/kyberforge -p "list your skills and agents"`) is only a clean + kyberforge-only signal when run from a working directory outside this repo. Run literally as + written, from this repo's root, this repo's own project-level `.claude/settings.json` sets + `enabledPlugins` to true for all 6 holocron plugins (kyberforge, git, gitea, core, lint, bin), so + Claude Code loads all 6 plugins' skills/agents, not just kyberforge's — conflating kyberforge's + discoverability with the other 5 plugins' already-enabled content. To isolate the signal, run + from a neutral cwd outside `/root/ai-development` with an absolute `--plugin-dir` path, e.g. + `cd /some/neutral/dir && claude --plugin-dir /root/ai-development/plugins/kyberforge -p "list your skills and agents"`. +- Reference: issue #90 (https://git.dev.rkdr.net/Defame1297/holocron/issues/90). diff --git a/docs/spec/architecture.md b/docs/spec/architecture.md index 8fbeb3b..2c4a4ad 100644 --- a/docs/spec/architecture.md +++ b/docs/spec/architecture.md @@ -25,7 +25,16 @@ Skills are **not** deployed by `install.sh`. They are distributed as plugins and ## Plugin model -Skills, agents, MCP servers, and hooks are distributed as self-contained plugin units under `plugins/`. Each plugin has a `plugin.json` manifest and is installed independently via `claude plugin install`. +Skills, agents, MCP servers, and hooks are distributed as self-contained plugin units under `plugins/`, installed independently via `claude plugin install @holocron`. Each plugin is an **apm package**: `plugins//apm.yml` plus a hand-authored `plugins//.apm/{skills,agents,hooks,commands,instructions,extensions}/` tree (ADR-0015). There is no hand-maintained `plugin.json` — every manifest and every host-visible content directory is compiled from that source. + +Two compilers produce the plugin roots you see in the tree: + +- **`apm pack` compiles the manifests** (ADR-0015). Per plugin: `.claude-plugin/plugin.json` and `.github/plugin/plugin.json`, both generated from `plugins//apm.yml`. Repo-wide, from the root `apm.yml`'s `marketplace:` block: `.claude-plugin/marketplace.json` (apm's `claude` output profile) and `.agents/plugins/marketplace.json` (its `codex` profile, a differently-shaped file). Those two are the only marketplace outputs apm has profiles for — the third root manifest, `.github/plugin/marketplace.json` (Copilot CLI's legacy path), is a byte-identical mirror of the Claude one maintained by `scripts/sync-marketplace-mirror.sh` and gated by the `check-marketplace-mirror-sync` pre-push hook. +- **`scripts/sync-plugin-content.sh` compiles the content mirror** (ADR-0017). It wraps `apm pack --format plugin` and copies the resulting bundle's flat `agents/`, `skills/`, `commands/`, `instructions/`, `extensions/`, and merged `hooks/hooks.json` back to the plugin root. Claude Code's installer convention-scans those flat paths and has no `.apm/` awareness whatsoever, so the mirror exists solely to satisfy the host's discovery contract. + +`.apm/` is the sole hand-edited authoring source for plugin content. An edit made in the flat mirror is discarded by the next sync and is reported as drift by the `check-plugin-content-sync` pre-push hook. Hand-authored material that is not an `.apm/` primitive — `README.md`, `docs/`, `bin/`, `sources.md`, `.mcp.json`, and per-plugin extras such as `plugins/git/config.example.json`, `plugins/gitea/references/` and `plugins/bin/evals/` — lives at the plugin **root** and is untouched by either compiler. + +That immunity is positional, not by filename. Anything placed *inside* a mirrored directory is destroyed regardless of what it is: `sync_dir` runs `rm -rf "$dst"` before every copy, and `sync_hooks_json` does the same to `hooks/`. A hand-written `README.md` under `plugins//hooks/` or `plugins//skills/` is deleted by the next sync with no drift report, because a file with no `.apm/` counterpart is simply absent from the regenerated tree. This has already cost the repo one document — `plugins/kyberforge/hooks/README.md`, since restored to `plugins/kyberforge/docs/hooks.md`. Plugin-root documentation belongs in `docs/`. ## Governance layer @@ -37,14 +46,16 @@ The governance layer has two phases: ## AGENTS.md pattern -This repo uses two `AGENTS.md` files as the provider-agnostic source of always-on rules (ADR-0012): +This repo uses two `AGENTS.md` files as the provider-agnostic source of always-on rules (ADR-0003): - **Repo-level `AGENTS.md`** — instructions for agents working inside this repo (structure, key rules). Imported by repo `CLAUDE.md` via `@AGENTS.md`. - **Global `core/AGENTS.md`** — Communication and Behavior rules that apply across all projects. Deployed to `~/.agents/AGENTS.md`; imported by `~/.claude/CLAUDE.md` via `@~/.agents/AGENTS.md`. Both `CLAUDE.md` files are thin adapters: they import from their respective `AGENTS.md` and add only Claude Code-specific syntax (`@import`, content index paths). They carry no original always-on content. -This repo also has a `CLAUDE.md` at its root — the Claude Code entry point for working in this repo. It imports `AGENTS.md` and `CONTEXT.md`, nothing more. This is distinct from `providers/claude-code/CLAUDE.md`, which is the global config deployed to `~/.claude/`. +This repo also has a `CLAUDE.md` at its root — the Claude Code entry point for working in this repo. It imports `AGENTS.md` and nothing else; there is no `@CONTEXT.md` import. It is not import-only either: below the import sits a fenced `` … `` block carrying the RTK command-prefix convention, which is tool-specific content with no `AGENTS.md` source. This is distinct from `providers/claude-code/CLAUDE.md`, which is the global config deployed to `~/.claude/`. + +`CONTEXT.md` is therefore **not** always-loaded. `AGENTS.md` instructs agents to read it at session start, which is a behavioural instruction, not an `@import` guarantee — `LESSONS.md`'s 2026-05-17 entry proposed adding the import and it was never applied. Treat that entry as open work rather than a record of a landed change. ## Provider model @@ -52,4 +63,4 @@ This repo also has a `CLAUDE.md` at its root — the Claude Code entry point for ## Architectural decisions -Key hard-to-reverse decisions are recorded as ADRs in `docs/adr/`. See the index there for rationale on choices like the pull distribution model, copy-not-symlink coupling, and the two-tier CLAUDE.md structure. +Key hard-to-reverse decisions are recorded as ADRs in `docs/adr/`. There is no index file — the directory holds 17 numbered ADRs whose filenames state their decision, so `ls docs/adr/` is the index. Read a superseding ADR before the one it supersedes: ADR-0015 (apm as the authoring source of truth) supersedes ADR-0001 and moots ADR-0006, and ADR-0017 corrects ADR-0015's host-discovery gap. Entry points for the structure described on this page: ADR-0002 (two-tier CLAUDE.md), ADR-0003 (AGENTS.md as the provider-agnostic entry point), ADR-0015 and ADR-0017 (the two compilers behind the plugin roots). diff --git a/plugins/bin/.apm/skills/caveman/SKILL.md b/plugins/bin/.apm/skills/caveman/SKILL.md new file mode 100644 index 0000000..85770a3 --- /dev/null +++ b/plugins/bin/.apm/skills/caveman/SKILL.md @@ -0,0 +1,49 @@ +--- +name: caveman +description: > + Ultra-compressed communication mode. Cuts token usage ~75% by dropping + filler, articles, and pleasantries while keeping full technical accuracy. + Use when user says "caveman mode", "talk like caveman", "use caveman", + "less tokens", "be brief", or invokes /caveman. +--- + +Respond terse like smart caveman. All technical substance stay. Only fluff die. + +## Persistence + +ACTIVE EVERY RESPONSE once triggered. No revert after many turns. No filler drift. Still active if unsure. Off only when user says "stop caveman" or "normal mode". + +## Rules + +Drop: articles (a/an/the), filler (just/really/basically/actually/simply), pleasantries (sure/certainly/of course/happy to), hedging. Fragments OK. Short synonyms (big not extensive, fix not "implement a solution for"). Abbreviate common terms (DB/auth/config/req/res/fn/impl). Strip conjunctions. Use arrows for causality (X -> Y). One word when one word enough. + +Technical terms stay exact. Code blocks unchanged. Errors quoted exact. + +Pattern: `[thing] [action] [reason]. [next step].` + +Not: "Sure! I'd be happy to help you with that. The issue you're experiencing is likely caused by..." +Yes: "Bug in auth middleware. Token expiry check use `<` not `<=`. Fix:" + +### Examples + +**"Why React component re-render?"** + +> Inline obj prop -> new ref -> re-render. `useMemo`. + +**"Explain database connection pooling."** + +> Pool = reuse DB conn. Skip handshake -> fast under load. + +## Auto-Clarity Exception + +Drop caveman temporarily for: security warnings, irreversible action confirmations, multi-step sequences where fragment order risks misread, user asks to clarify or repeats question. Resume caveman after clear part done. + +Example -- destructive op: + +> **Warning:** This will permanently delete all rows in the `users` table and cannot be undone. +> +> ```sql +> DROP TABLE users; +> ``` +> +> Caveman resume. Verify backup exist first. diff --git a/plugins/bin/.apm/skills/diagnose/SKILL.md b/plugins/bin/.apm/skills/diagnose/SKILL.md new file mode 100644 index 0000000..ed55bda --- /dev/null +++ b/plugins/bin/.apm/skills/diagnose/SKILL.md @@ -0,0 +1,117 @@ +--- +name: diagnose +description: Disciplined diagnosis loop for hard bugs and performance regressions. Reproduce → minimise → hypothesise → instrument → fix → regression-test. Use when user says "diagnose this" / "debug this", reports a bug, says something is broken/throwing/failing, or describes a performance regression. +--- + +# Diagnose + +A discipline for hard bugs. Skip phases only when explicitly justified. + +When exploring the codebase, use the project's domain glossary to get a clear mental model of the relevant modules, and check ADRs in the area you're touching. + +## Phase 1 — Build a feedback loop + +**This is the skill.** Everything else is mechanical. If you have a fast, deterministic, agent-runnable pass/fail signal for the bug, you will find the cause — bisection, hypothesis-testing, and instrumentation all just consume that signal. If you don't have one, no amount of staring at code will save you. + +Spend disproportionate effort here. **Be aggressive. Be creative. Refuse to give up.** + +### Ways to construct one — try them in roughly this order + +1. **Failing test** at whatever seam reaches the bug — unit, integration, e2e. +2. **Curl / HTTP script** against a running dev server. +3. **CLI invocation** with a fixture input, diffing stdout against a known-good snapshot. +4. **Headless browser script** (Playwright / Puppeteer) — drives the UI, asserts on DOM/console/network. +5. **Replay a captured trace.** Save a real network request / payload / event log to disk; replay it through the code path in isolation. +6. **Throwaway harness.** Spin up a minimal subset of the system (one service, mocked deps) that exercises the bug code path with a single function call. +7. **Property / fuzz loop.** If the bug is "sometimes wrong output", run 1000 random inputs and look for the failure mode. +8. **Bisection harness.** If the bug appeared between two known states (commit, dataset, version), automate "boot at state X, check, repeat" so you can `git bisect run` it. +9. **Differential loop.** Run the same input through old-version vs new-version (or two configs) and diff outputs. +10. **HITL bash script.** Last resort. If a human must click, drive _them_ with `scripts/hitl-loop.template.sh` so the loop is still structured. Captured output feeds back to you. + +Build the right feedback loop, and the bug is 90% fixed. + +### Iterate on the loop itself + +Treat the loop as a product. Once you have _a_ loop, ask: + +- Can I make it faster? (Cache setup, skip unrelated init, narrow the test scope.) +- Can I make the signal sharper? (Assert on the specific symptom, not "didn't crash".) +- Can I make it more deterministic? (Pin time, seed RNG, isolate filesystem, freeze network.) + +A 30-second flaky loop is barely better than no loop. A 2-second deterministic loop is a debugging superpower. + +### Non-deterministic bugs + +The goal is not a clean repro but a **higher reproduction rate**. Loop the trigger 100×, parallelise, add stress, narrow timing windows, inject sleeps. A 50%-flake bug is debuggable; 1% is not — keep raising the rate until it's debuggable. + +### When you genuinely cannot build a loop + +Stop and say so explicitly. List what you tried. Ask the user for: (a) access to whatever environment reproduces it, (b) a captured artifact (HAR file, log dump, core dump, screen recording with timestamps), or (c) permission to add temporary production instrumentation. Do **not** proceed to hypothesise without a loop. + +Do not proceed to Phase 2 until you have a loop you believe in. + +## Phase 2 — Reproduce + +Run the loop. Watch the bug appear. + +Confirm: + +- [ ] The loop produces the failure mode the **user** described — not a different failure that happens to be nearby. Wrong bug = wrong fix. +- [ ] The failure is reproducible across multiple runs (or, for non-deterministic bugs, reproducible at a high enough rate to debug against). +- [ ] You have captured the exact symptom (error message, wrong output, slow timing) so later phases can verify the fix actually addresses it. + +Do not proceed until you reproduce the bug. + +## Phase 3 — Hypothesise + +Generate **3–5 ranked hypotheses** before testing any of them. Single-hypothesis generation anchors on the first plausible idea. + +Each hypothesis must be **falsifiable**: state the prediction it makes. + +> Format: "If is the cause, then will make the bug disappear / will make it worse." + +If you cannot state the prediction, the hypothesis is a vibe — discard or sharpen it. + +**Show the ranked list to the user before testing.** They often have domain knowledge that re-ranks instantly ("we just deployed a change to #3"), or know hypotheses they've already ruled out. Cheap checkpoint, big time saver. Don't block on it — proceed with your ranking if the user is AFK. + +## Phase 4 — Instrument + +Each probe must map to a specific prediction from Phase 3. **Change one variable at a time.** + +Tool preference: + +1. **Debugger / REPL inspection** if the env supports it. One breakpoint beats ten logs. +2. **Targeted logs** at the boundaries that distinguish hypotheses. +3. Never "log everything and grep". + +**Tag every debug log** with a unique prefix, e.g. `[DEBUG-a4f2]`. Cleanup at the end becomes a single grep. Untagged logs survive; tagged logs die. + +**Perf branch.** For performance regressions, logs are usually wrong. Instead: establish a baseline measurement (timing harness, `performance.now()`, profiler, query plan), then bisect. Measure first, fix second. + +## Phase 5 — Fix + regression test + +Write the regression test **before the fix** — but only if there is a **correct seam** for it. + +A correct seam is one where the test exercises the **real bug pattern** as it occurs at the call site. If the only available seam is too shallow (single-caller test when the bug needs multiple callers, unit test that can't replicate the chain that triggered the bug), a regression test there gives false confidence. + +**If no correct seam exists, that itself is the finding.** Note it. The codebase architecture is preventing the bug from being locked down. Flag this for the next phase. + +If a correct seam exists: + +1. Turn the minimised repro into a failing test at that seam. +2. Watch it fail. +3. Apply the fix. +4. Watch it pass. +5. Re-run the Phase 1 feedback loop against the original (un-minimised) scenario. + +## Phase 6 — Cleanup + post-mortem + +Required before declaring done: + +- [ ] Original repro no longer reproduces (re-run the Phase 1 loop) +- [ ] Regression test passes (or absence of seam is documented) +- [ ] All `[DEBUG-...]` instrumentation removed (`grep` the prefix) +- [ ] Throwaway prototypes deleted (or moved to a clearly-marked debug location) +- [ ] The hypothesis that turned out correct is stated in the commit / PR message — so the next debugger learns + +**Then ask: what would have prevented this bug?** If the answer involves architectural change (no good test seam, tangled callers, hidden coupling) hand off to the `/improve-codebase-architecture` skill with the specifics. Make the recommendation **after** the fix is in, not before — you have more information now than when you started. diff --git a/plugins/bin/.apm/skills/diagnose/scripts/hitl-loop.template.sh b/plugins/bin/.apm/skills/diagnose/scripts/hitl-loop.template.sh new file mode 100644 index 0000000..40afc46 --- /dev/null +++ b/plugins/bin/.apm/skills/diagnose/scripts/hitl-loop.template.sh @@ -0,0 +1,41 @@ +#!/usr/bin/env bash +# Human-in-the-loop reproduction loop. +# Copy this file, edit the steps below, and run it. +# The agent runs the script; the user follows prompts in their terminal. +# +# Usage: +# bash hitl-loop.template.sh +# +# Two helpers: +# step "" → show instruction, wait for Enter +# capture VAR "" → show question, read response into VAR +# +# At the end, captured values are printed as KEY=VALUE for the agent to parse. + +set -euo pipefail + +step() { + printf '\n>>> %s\n' "$1" + read -r -p " [Enter when done] " _ +} + +capture() { + local var="$1" question="$2" answer + printf '\n>>> %s\n' "$question" + read -r -p " > " answer + printf -v "$var" '%s' "$answer" +} + +# --- edit below --------------------------------------------------------- + +step "Open the app at http://localhost:3000 and sign in." + +capture ERRORED "Click the 'Export' button. Did it throw an error? (y/n)" + +capture ERROR_MSG "Paste the error message (or 'none'):" + +# --- edit above --------------------------------------------------------- + +printf '\n--- Captured ---\n' +printf 'ERRORED=%s\n' "$ERRORED" +printf 'ERROR_MSG=%s\n' "$ERROR_MSG" diff --git a/plugins/bin/.apm/skills/grill-me/SKILL.md b/plugins/bin/.apm/skills/grill-me/SKILL.md new file mode 100644 index 0000000..bd04394 --- /dev/null +++ b/plugins/bin/.apm/skills/grill-me/SKILL.md @@ -0,0 +1,10 @@ +--- +name: grill-me +description: Interview the user relentlessly about a plan or design until reaching shared understanding, resolving each branch of the decision tree. Use when user wants to stress-test a plan, get grilled on their design, or mentions "grill me". +--- + +Interview me relentlessly about every aspect of this plan until we reach a shared understanding. Walk down each branch of the design tree, resolving dependencies between decisions one-by-one. For each question, provide your recommended answer. + +Ask the questions one at a time. + +If a question can be answered by exploring the codebase, explore the codebase instead. diff --git a/plugins/bin/.apm/skills/grill-with-docs/ADR-FORMAT.md b/plugins/bin/.apm/skills/grill-with-docs/ADR-FORMAT.md new file mode 100644 index 0000000..da7e78e --- /dev/null +++ b/plugins/bin/.apm/skills/grill-with-docs/ADR-FORMAT.md @@ -0,0 +1,47 @@ +# ADR Format + +ADRs live in `docs/adr/` and use sequential numbering: `0001-slug.md`, `0002-slug.md`, etc. + +Create the `docs/adr/` directory lazily — only when the first ADR is needed. + +## Template + +```md +# {Short title of the decision} + +{1-3 sentences: what's the context, what did we decide, and why.} +``` + +That's it. An ADR can be a single paragraph. The value is in recording *that* a decision was made and *why* — not in filling out sections. + +## Optional sections + +Only include these when they add genuine value. Most ADRs won't need them. + +- **Status** frontmatter (`proposed | accepted | deprecated | superseded by ADR-NNNN`) — useful when decisions are revisited +- **Considered Options** — only when the rejected alternatives are worth remembering +- **Consequences** — only when non-obvious downstream effects need to be called out + +## Numbering + +Scan `docs/adr/` for the highest existing number and increment by one. + +## When to offer an ADR + +All three of these must be true: + +1. **Hard to reverse** — the cost of changing your mind later is meaningful +2. **Surprising without context** — a future reader will look at the code and wonder "why on earth did they do it this way?" +3. **The result of a real trade-off** — there were genuine alternatives and you picked one for specific reasons + +If a decision is easy to reverse, skip it — you'll just reverse it. If it's not surprising, nobody will wonder why. If there was no real alternative, there's nothing to record beyond "we did the obvious thing." + +### What qualifies + +- **Architectural shape.** "We're using a monorepo." "The write model is event-sourced, the read model is projected into Postgres." +- **Integration patterns between contexts.** "Ordering and Billing communicate via domain events, not synchronous HTTP." +- **Technology choices that carry lock-in.** Database, message bus, auth provider, deployment target. Not every library — just the ones that would take a quarter to swap out. +- **Boundary and scope decisions.** "Customer data is owned by the Customer context; other contexts reference it by ID only." The explicit no-s are as valuable as the yes-s. +- **Deliberate deviations from the obvious path.** "We're using manual SQL instead of an ORM because X." Anything where a reasonable reader would assume the opposite. These stop the next engineer from "fixing" something that was deliberate. +- **Constraints not visible in the code.** "We can't use AWS because of compliance requirements." "Response times must be under 200ms because of the partner API contract." +- **Rejected alternatives when the rejection is non-obvious.** If you considered GraphQL and picked REST for subtle reasons, record it — otherwise someone will suggest GraphQL again in six months. diff --git a/plugins/bin/.apm/skills/grill-with-docs/CONTEXT-FORMAT.md b/plugins/bin/.apm/skills/grill-with-docs/CONTEXT-FORMAT.md new file mode 100644 index 0000000..ddfa247 --- /dev/null +++ b/plugins/bin/.apm/skills/grill-with-docs/CONTEXT-FORMAT.md @@ -0,0 +1,77 @@ +# CONTEXT.md Format + +## Structure + +```md +# {Context Name} + +{One or two sentence description of what this context is and why it exists.} + +## Language + +**Order**: +{A concise description of the term} +_Avoid_: Purchase, transaction + +**Invoice**: +A request for payment sent to a customer after delivery. +_Avoid_: Bill, payment request + +**Customer**: +A person or organization that places orders. +_Avoid_: Client, buyer, account + +## Relationships + +- An **Order** produces one or more **Invoices** +- An **Invoice** belongs to exactly one **Customer** + +## Example dialogue + +> **Dev:** "When a **Customer** places an **Order**, do we create the **Invoice** immediately?" +> **Domain expert:** "No — an **Invoice** is only generated once a **Fulfillment** is confirmed." + +## Flagged ambiguities + +- "account" was used to mean both **Customer** and **User** — resolved: these are distinct concepts. +``` + +## Rules + +- **Be opinionated.** When multiple words exist for the same concept, pick the best one and list the others as aliases to avoid. +- **Flag conflicts explicitly.** If a term is used ambiguously, call it out in "Flagged ambiguities" with a clear resolution. +- **Keep definitions tight.** One sentence max. Define what it IS, not what it does. +- **Show relationships.** Use bold term names and express cardinality where obvious. +- **Only include terms specific to this project's context.** General programming concepts (timeouts, error types, utility patterns) don't belong even if the project uses them extensively. Before adding a term, ask: is this a concept unique to this context, or a general programming concept? Only the former belongs. +- **Group terms under subheadings** when natural clusters emerge. If all terms belong to a single cohesive area, a flat list is fine. +- **Write an example dialogue.** A conversation between a dev and a domain expert that demonstrates how the terms interact naturally and clarifies boundaries between related concepts. + +## Single vs multi-context repos + +**Single context (most repos):** One `CONTEXT.md` at the repo root. + +**Multiple contexts:** A `CONTEXT-MAP.md` at the repo root lists the contexts, where they live, and how they relate to each other: + +```md +# Context Map + +## Contexts + +- [Ordering](./src/ordering/CONTEXT.md) — receives and tracks customer orders +- [Billing](./src/billing/CONTEXT.md) — generates invoices and processes payments +- [Fulfillment](./src/fulfillment/CONTEXT.md) — manages warehouse picking and shipping + +## Relationships + +- **Ordering → Fulfillment**: Ordering emits `OrderPlaced` events; Fulfillment consumes them to start picking +- **Fulfillment → Billing**: Fulfillment emits `ShipmentDispatched` events; Billing consumes them to generate invoices +- **Ordering ↔ Billing**: Shared types for `CustomerId` and `Money` +``` + +The skill infers which structure applies: + +- If `CONTEXT-MAP.md` exists, read it to find contexts +- If only a root `CONTEXT.md` exists, single context +- If neither exists, create a root `CONTEXT.md` lazily when the first term is resolved + +When multiple contexts exist, infer which one the current topic relates to. If unclear, ask. diff --git a/plugins/bin/.apm/skills/grill-with-docs/SKILL.md b/plugins/bin/.apm/skills/grill-with-docs/SKILL.md new file mode 100644 index 0000000..6dad6ad --- /dev/null +++ b/plugins/bin/.apm/skills/grill-with-docs/SKILL.md @@ -0,0 +1,88 @@ +--- +name: grill-with-docs +description: Grilling session that challenges your plan against the existing domain model, sharpens terminology, and updates documentation (CONTEXT.md, ADRs) inline as decisions crystallise. Use when user wants to stress-test a plan against their project's language and documented decisions. +--- + + + +Interview me relentlessly about every aspect of this plan until we reach a shared understanding. Walk down each branch of the design tree, resolving dependencies between decisions one-by-one. For each question, provide your recommended answer. + +Ask the questions one at a time, waiting for feedback on each question before continuing. + +If a question can be answered by exploring the codebase, explore the codebase instead. + + + + + +## Domain awareness + +During codebase exploration, also look for existing documentation: + +### File structure + +Most repos have a single context: + +``` +/ +├── CONTEXT.md +├── docs/ +│ └── adr/ +│ ├── 0001-event-sourced-orders.md +│ └── 0002-postgres-for-write-model.md +└── src/ +``` + +If a `CONTEXT-MAP.md` exists at the root, the repo has multiple contexts. The map points to where each one lives: + +``` +/ +├── CONTEXT-MAP.md +├── docs/ +│ └── adr/ ← system-wide decisions +├── src/ +│ ├── ordering/ +│ │ ├── CONTEXT.md +│ │ └── docs/adr/ ← context-specific decisions +│ └── billing/ +│ ├── CONTEXT.md +│ └── docs/adr/ +``` + +Create files lazily — only when you have something to write. If no `CONTEXT.md` exists, create one when the first term is resolved. If no `docs/adr/` exists, create it when the first ADR is needed. + +## During the session + +### Challenge against the glossary + +When the user uses a term that conflicts with the existing language in `CONTEXT.md`, call it out immediately. "Your glossary defines 'cancellation' as X, but you seem to mean Y — which is it?" + +### Sharpen fuzzy language + +When the user uses vague or overloaded terms, propose a precise canonical term. "You're saying 'account' — do you mean the Customer or the User? Those are different things." + +### Discuss concrete scenarios + +When domain relationships are being discussed, stress-test them with specific scenarios. Invent scenarios that probe edge cases and force the user to be precise about the boundaries between concepts. + +### Cross-reference with code + +When the user states how something works, check whether the code agrees. If you find a contradiction, surface it: "Your code cancels entire Orders, but you just said partial cancellation is possible — which is right?" + +### Update CONTEXT.md inline + +When a term is resolved, update `CONTEXT.md` right there. Don't batch these up — capture them as they happen. Use the format in [CONTEXT-FORMAT.md](./CONTEXT-FORMAT.md). + +Don't couple `CONTEXT.md` to implementation details. Only include terms that are meaningful to domain experts. + +### Offer ADRs sparingly + +Only offer to create an ADR when all three are true: + +1. **Hard to reverse** — the cost of changing your mind later is meaningful +2. **Surprising without context** — a future reader will wonder "why did they do it this way?" +3. **The result of a real trade-off** — there were genuine alternatives and you picked one for specific reasons + +If any of the three is missing, skip the ADR. Use the format in [ADR-FORMAT.md](./ADR-FORMAT.md). + + diff --git a/plugins/bin/.apm/skills/improve-codebase-architecture/DEEPENING.md b/plugins/bin/.apm/skills/improve-codebase-architecture/DEEPENING.md new file mode 100644 index 0000000..ecaf5d7 --- /dev/null +++ b/plugins/bin/.apm/skills/improve-codebase-architecture/DEEPENING.md @@ -0,0 +1,37 @@ +# Deepening + +How to deepen a cluster of shallow modules safely, given its dependencies. Assumes the vocabulary in [LANGUAGE.md](LANGUAGE.md) — **module**, **interface**, **seam**, **adapter**. + +## Dependency categories + +When assessing a candidate for deepening, classify its dependencies. The category determines how the deepened module is tested across its seam. + +### 1. In-process + +Pure computation, in-memory state, no I/O. Always deepenable — merge the modules and test through the new interface directly. No adapter needed. + +### 2. Local-substitutable + +Dependencies that have local test stand-ins (PGLite for Postgres, in-memory filesystem). Deepenable if the stand-in exists. The deepened module is tested with the stand-in running in the test suite. The seam is internal; no port at the module's external interface. + +### 3. Remote but owned (Ports & Adapters) + +Your own services across a network boundary (microservices, internal APIs). Define a **port** (interface) at the seam. The deep module owns the logic; the transport is injected as an **adapter**. Tests use an in-memory adapter. Production uses an HTTP/gRPC/queue adapter. + +Recommendation shape: *"Define a port at the seam, implement an HTTP adapter for production and an in-memory adapter for testing, so the logic sits in one deep module even though it's deployed across a network."* + +### 4. True external (Mock) + +Third-party services (Stripe, Twilio, etc.) you don't control. The deepened module takes the external dependency as an injected port; tests provide a mock adapter. + +## Seam discipline + +- **One adapter means a hypothetical seam. Two adapters means a real one.** Don't introduce a port unless at least two adapters are justified (typically production + test). A single-adapter seam is just indirection. +- **Internal seams vs external seams.** A deep module can have internal seams (private to its implementation, used by its own tests) as well as the external seam at its interface. Don't expose internal seams through the interface just because tests use them. + +## Testing strategy: replace, don't layer + +- Old unit tests on shallow modules become waste once tests at the deepened module's interface exist — delete them. +- Write new tests at the deepened module's interface. The **interface is the test surface**. +- Tests assert on observable outcomes through the interface, not internal state. +- Tests should survive internal refactors — they describe behaviour, not implementation. If a test has to change when the implementation changes, it's testing past the interface. diff --git a/plugins/bin/.apm/skills/improve-codebase-architecture/INTERFACE-DESIGN.md b/plugins/bin/.apm/skills/improve-codebase-architecture/INTERFACE-DESIGN.md new file mode 100644 index 0000000..3197723 --- /dev/null +++ b/plugins/bin/.apm/skills/improve-codebase-architecture/INTERFACE-DESIGN.md @@ -0,0 +1,44 @@ +# Interface Design + +When the user wants to explore alternative interfaces for a chosen deepening candidate, use this parallel sub-agent pattern. Based on "Design It Twice" (Ousterhout) — your first idea is unlikely to be the best. + +Uses the vocabulary in [LANGUAGE.md](LANGUAGE.md) — **module**, **interface**, **seam**, **adapter**, **leverage**. + +## Process + +### 1. Frame the problem space + +Before spawning sub-agents, write a user-facing explanation of the problem space for the chosen candidate: + +- The constraints any new interface would need to satisfy +- The dependencies it would rely on, and which category they fall into (see [DEEPENING.md](DEEPENING.md)) +- A rough illustrative code sketch to ground the constraints — not a proposal, just a way to make the constraints concrete + +Show this to the user, then immediately proceed to Step 2. The user reads and thinks while the sub-agents work in parallel. + +### 2. Spawn sub-agents + +Spawn 3+ sub-agents in parallel using the Agent tool. Each must produce a **radically different** interface for the deepened module. + +Prompt each sub-agent with a separate technical brief (file paths, coupling details, dependency category from [DEEPENING.md](DEEPENING.md), what sits behind the seam). The brief is independent of the user-facing problem-space explanation in Step 1. Give each agent a different design constraint: + +- Agent 1: "Minimize the interface — aim for 1–3 entry points max. Maximise leverage per entry point." +- Agent 2: "Maximise flexibility — support many use cases and extension." +- Agent 3: "Optimise for the most common caller — make the default case trivial." +- Agent 4 (if applicable): "Design around ports & adapters for cross-seam dependencies." + +Include both [LANGUAGE.md](LANGUAGE.md) vocabulary and CONTEXT.md vocabulary in the brief so each sub-agent names things consistently with the architecture language and the project's domain language. + +Each sub-agent outputs: + +1. Interface (types, methods, params — plus invariants, ordering, error modes) +2. Usage example showing how callers use it +3. What the implementation hides behind the seam +4. Dependency strategy and adapters (see [DEEPENING.md](DEEPENING.md)) +5. Trade-offs — where leverage is high, where it's thin + +### 3. Present and compare + +Present designs sequentially so the user can absorb each one, then compare them in prose. Contrast by **depth** (leverage at the interface), **locality** (where change concentrates), and **seam placement**. + +After comparing, give your own recommendation: which design you think is strongest and why. If elements from different designs would combine well, propose a hybrid. Be opinionated — the user wants a strong read, not a menu. diff --git a/plugins/bin/.apm/skills/improve-codebase-architecture/LANGUAGE.md b/plugins/bin/.apm/skills/improve-codebase-architecture/LANGUAGE.md new file mode 100644 index 0000000..530c276 --- /dev/null +++ b/plugins/bin/.apm/skills/improve-codebase-architecture/LANGUAGE.md @@ -0,0 +1,53 @@ +# Language + +Shared vocabulary for every suggestion this skill makes. Use these terms exactly — don't substitute "component," "service," "API," or "boundary." Consistent language is the whole point. + +## Terms + +**Module** +Anything with an interface and an implementation. Deliberately scale-agnostic — applies equally to a function, class, package, or tier-spanning slice. +_Avoid_: unit, component, service. + +**Interface** +Everything a caller must know to use the module correctly. Includes the type signature, but also invariants, ordering constraints, error modes, required configuration, and performance characteristics. +_Avoid_: API, signature (too narrow — those refer only to the type-level surface). + +**Implementation** +What's inside a module — its body of code. Distinct from **Adapter**: a thing can be a small adapter with a large implementation (a Postgres repo) or a large adapter with a small implementation (an in-memory fake). Reach for "adapter" when the seam is the topic; "implementation" otherwise. + +**Depth** +Leverage at the interface — the amount of behaviour a caller (or test) can exercise per unit of interface they have to learn. A module is **deep** when a large amount of behaviour sits behind a small interface. A module is **shallow** when the interface is nearly as complex as the implementation. + +**Seam** _(from Michael Feathers)_ +A place where you can alter behaviour without editing in that place. The *location* at which a module's interface lives. Choosing where to put the seam is its own design decision, distinct from what goes behind it. +_Avoid_: boundary (overloaded with DDD's bounded context). + +**Adapter** +A concrete thing that satisfies an interface at a seam. Describes *role* (what slot it fills), not substance (what's inside). + +**Leverage** +What callers get from depth. More capability per unit of interface they have to learn. One implementation pays back across N call sites and M tests. + +**Locality** +What maintainers get from depth. Change, bugs, knowledge, and verification concentrate at one place rather than spreading across callers. Fix once, fixed everywhere. + +## Principles + +- **Depth is a property of the interface, not the implementation.** A deep module can be internally composed of small, mockable, swappable parts — they just aren't part of the interface. A module can have **internal seams** (private to its implementation, used by its own tests) as well as the **external seam** at its interface. +- **The deletion test.** Imagine deleting the module. If complexity vanishes, the module wasn't hiding anything (it was a pass-through). If complexity reappears across N callers, the module was earning its keep. +- **The interface is the test surface.** Callers and tests cross the same seam. If you want to test *past* the interface, the module is probably the wrong shape. +- **One adapter means a hypothetical seam. Two adapters means a real one.** Don't introduce a seam unless something actually varies across it. + +## Relationships + +- A **Module** has exactly one **Interface** (the surface it presents to callers and tests). +- **Depth** is a property of a **Module**, measured against its **Interface**. +- A **Seam** is where a **Module**'s **Interface** lives. +- An **Adapter** sits at a **Seam** and satisfies the **Interface**. +- **Depth** produces **Leverage** for callers and **Locality** for maintainers. + +## Rejected framings + +- **Depth as ratio of implementation-lines to interface-lines** (Ousterhout): rewards padding the implementation. We use depth-as-leverage instead. +- **"Interface" as the TypeScript `interface` keyword or a class's public methods**: too narrow — interface here includes every fact a caller must know. +- **"Boundary"**: overloaded with DDD's bounded context. Say **seam** or **interface**. diff --git a/plugins/bin/.apm/skills/improve-codebase-architecture/SKILL.md b/plugins/bin/.apm/skills/improve-codebase-architecture/SKILL.md new file mode 100644 index 0000000..05984a6 --- /dev/null +++ b/plugins/bin/.apm/skills/improve-codebase-architecture/SKILL.md @@ -0,0 +1,71 @@ +--- +name: improve-codebase-architecture +description: Find deepening opportunities in a codebase, informed by the domain language in CONTEXT.md and the decisions in docs/adr/. Use when the user wants to improve architecture, find refactoring opportunities, consolidate tightly-coupled modules, or make a codebase more testable and AI-navigable. +--- + +# Improve Codebase Architecture + +Surface architectural friction and propose **deepening opportunities** — refactors that turn shallow modules into deep ones. The aim is testability and AI-navigability. + +## Glossary + +Use these terms exactly in every suggestion. Consistent language is the point — don't drift into "component," "service," "API," or "boundary." Full definitions in [LANGUAGE.md](LANGUAGE.md). + +- **Module** — anything with an interface and an implementation (function, class, package, slice). +- **Interface** — everything a caller must know to use the module: types, invariants, error modes, ordering, config. Not just the type signature. +- **Implementation** — the code inside. +- **Depth** — leverage at the interface: a lot of behaviour behind a small interface. **Deep** = high leverage. **Shallow** = interface nearly as complex as the implementation. +- **Seam** — where an interface lives; a place behaviour can be altered without editing in place. (Use this, not "boundary.") +- **Adapter** — a concrete thing satisfying an interface at a seam. +- **Leverage** — what callers get from depth. +- **Locality** — what maintainers get from depth: change, bugs, knowledge concentrated in one place. + +Key principles (see [LANGUAGE.md](LANGUAGE.md) for the full list): + +- **Deletion test**: imagine deleting the module. If complexity vanishes, it was a pass-through. If complexity reappears across N callers, it was earning its keep. +- **The interface is the test surface.** +- **One adapter = hypothetical seam. Two adapters = real seam.** + +This skill is _informed_ by the project's domain model. The domain language gives names to good seams; ADRs record decisions the skill should not re-litigate. + +## Process + +### 1. Explore + +Read the project's domain glossary and any ADRs in the area you're touching first. + +Then use the Agent tool with `subagent_type=Explore` to walk the codebase. Don't follow rigid heuristics — explore organically and note where you experience friction: + +- Where does understanding one concept require bouncing between many small modules? +- Where are modules **shallow** — interface nearly as complex as the implementation? +- Where have pure functions been extracted just for testability, but the real bugs hide in how they're called (no **locality**)? +- Where do tightly-coupled modules leak across their seams? +- Which parts of the codebase are untested, or hard to test through their current interface? + +Apply the **deletion test** to anything you suspect is shallow: would deleting it concentrate complexity, or just move it? A "yes, concentrates" is the signal you want. + +### 2. Present candidates + +Present a numbered list of deepening opportunities. For each candidate: + +- **Files** — which files/modules are involved +- **Problem** — why the current architecture is causing friction +- **Solution** — plain English description of what would change +- **Benefits** — explained in terms of locality and leverage, and also in how tests would improve + +**Use CONTEXT.md vocabulary for the domain, and [LANGUAGE.md](LANGUAGE.md) vocabulary for the architecture.** If `CONTEXT.md` defines "Order," talk about "the Order intake module" — not "the FooBarHandler," and not "the Order service." + +**ADR conflicts**: if a candidate contradicts an existing ADR, only surface it when the friction is real enough to warrant revisiting the ADR. Mark it clearly (e.g. _"contradicts ADR-0007 — but worth reopening because…"_). Don't list every theoretical refactor an ADR forbids. + +Do NOT propose interfaces yet. Ask the user: "Which of these would you like to explore?" + +### 3. Grilling loop + +Once the user picks a candidate, drop into a grilling conversation. Walk the design tree with them — constraints, dependencies, the shape of the deepened module, what sits behind the seam, what tests survive. + +Side effects happen inline as decisions crystallize: + +- **Naming a deepened module after a concept not in `CONTEXT.md`?** Add the term to `CONTEXT.md` — same discipline as `/grill-with-docs` (see [CONTEXT-FORMAT.md](../grill-with-docs/CONTEXT-FORMAT.md)). Create the file lazily if it doesn't exist. +- **Sharpening a fuzzy term during the conversation?** Update `CONTEXT.md` right there. +- **User rejects the candidate with a load-bearing reason?** Offer an ADR, framed as: _"Want me to record this as an ADR so future architecture reviews don't re-suggest it?"_ Only offer when the reason would actually be needed by a future explorer to avoid re-suggesting the same thing — skip ephemeral reasons ("not worth it right now") and self-evident ones. See [ADR-FORMAT.md](../grill-with-docs/ADR-FORMAT.md). +- **Want to explore alternative interfaces for the deepened module?** See [INTERFACE-DESIGN.md](INTERFACE-DESIGN.md). diff --git a/plugins/bin/.apm/skills/prototype/LOGIC.md b/plugins/bin/.apm/skills/prototype/LOGIC.md new file mode 100644 index 0000000..526ecb1 --- /dev/null +++ b/plugins/bin/.apm/skills/prototype/LOGIC.md @@ -0,0 +1,79 @@ +# Logic Prototype + +A tiny interactive terminal app that lets the user drive a state model by hand. Use this when the question is about **business logic, state transitions, or data shape** — the kind of thing that looks reasonable on paper but only feels wrong once you push it through real cases. + +## When this is the right shape + +- "I'm not sure if this state machine handles the edge case where X then Y." +- "Does this data model actually let me represent the case where..." +- "I want to feel out what the API should look like before writing it." +- Anything where the user wants to **press buttons and watch state change**. + +If the question is "what should this look like" — wrong branch. Use [UI.md](UI.md). + +## Process + +### 1. State the question + +Before writing code, write down what state model and what question you're prototyping. One paragraph, in the prototype's README or a comment at the top of the file. A logic prototype that answers the wrong question is pure waste — make the question explicit so it can be checked later, whether the user is watching now or returning to it AFK. + +### 2. Pick the language + +Use whatever the host project uses. If the project has no obvious runtime (e.g. a docs repo), ask. + +Match the project's existing conventions for tooling — don't add a new package manager or runtime just for the prototype. + +### 3. Isolate the logic in a portable module + +Put the actual logic — the bit that's answering the question — behind a small, pure interface that could be lifted out and dropped into the real codebase later. The TUI around it is throwaway; the logic module shouldn't be. + +The right shape depends on the question: + +- **A pure reducer** — `(state, action) => state`. Good when actions are discrete events and state is a single value. +- **A state machine** — explicit states and transitions. Good when "which actions are even legal right now" is part of the question. +- **A small set of pure functions** over a plain data type. Good when there's no implicit current state — just transformations. +- **A class or module with a clear method surface** when the logic genuinely owns ongoing internal state. + +Pick whichever shape best fits the question being asked, *not* whichever is easiest to wire to a TUI. Keep it pure: no I/O, no terminal code, no `console.log` for control flow. The TUI imports it and calls into it; nothing flows the other direction. + +This is what makes the prototype useful past its own lifetime. When the question's been answered, the validated reducer / machine / function set can be lifted into the real module — the TUI shell gets deleted. + +### 4. Build the smallest TUI that exposes the state + +Build it as a **lightweight TUI** — on every tick, clear the screen (`console.clear()` / `print("\033[2J\033[H")` / equivalent) and re-render the whole frame. The user should always see one stable view, not an ever-growing scrollback. + +Each frame has two parts, in this order: + +1. **Current state**, pretty-printed and diff-friendly (one field per line, or formatted JSON). Use **bold** for field names or section headers and **dim** for less important context (timestamps, IDs, derived values). Native ANSI escape codes are fine — `\x1b[1m` bold, `\x1b[2m` dim, `\x1b[0m` reset. No need to pull in a styling library unless one is already in the project. +2. **Keyboard shortcuts**, listed at the bottom: `[a] add user [d] delete user [t] tick clock [q] quit`. Bold the key, dim the description, or vice-versa — whatever reads cleanly. + +Behaviour: + +1. **Initialise state** — a single in-memory object/struct. Render the first frame on start. +2. **Read one keystroke (or one line)** at a time, dispatch to a handler that mutates state. +3. **Re-render** the full frame after every action — don't append, replace. +4. **Loop until quit.** + +The whole frame should fit on one screen. + +### 5. Make it runnable in one command + +Add a script to the project's existing task runner (`package.json` scripts, `Makefile`, `justfile`, `pyproject.toml`). The user should run `pnpm run ` or equivalent — never need to remember a path. + +If the host project has no task runner, just put the command at the top of the prototype's README. + +### 6. Hand it over + +Give the user the run command. They'll drive it themselves; the interesting moments are when they say "wait, that shouldn't be possible" or "huh, I assumed X would be different" — those are the bugs in the _idea_, which is the whole point. If they want new actions added, add them. Prototypes evolve. + +### 7. Capture the answer + +When the prototype has done its job, the answer to the question is the only thing worth keeping. If the user is around, ask what it taught them. If not, leave a `NOTES.md` next to the prototype so the answer can be filled in (or filled in by you, if you've watched the session) before the prototype gets deleted. + +## Anti-patterns + +- **Don't add tests.** A prototype that needs tests is no longer a prototype. +- **Don't wire it to the real database.** Use an in-memory store unless the question is specifically about persistence. +- **Don't generalise.** No "what if we wanted to support X later." The prototype answers one question. +- **Don't blur the logic and the TUI together.** If the reducer / state machine references `console.log`, prompts, or terminal escape codes, it's no longer portable. Keep the TUI as a thin shell over a pure module. +- **Don't ship the TUI shell into production.** The shell is optimised for being driven by hand from a terminal. The logic module behind it is the bit worth keeping. diff --git a/plugins/bin/.apm/skills/prototype/SKILL.md b/plugins/bin/.apm/skills/prototype/SKILL.md new file mode 100644 index 0000000..84478e0 --- /dev/null +++ b/plugins/bin/.apm/skills/prototype/SKILL.md @@ -0,0 +1,30 @@ +--- +name: prototype +description: Build a throwaway prototype to flush out a design before committing to it. Routes between two branches — a runnable terminal app for state/business-logic questions, or several radically different UI variations toggleable from one route. Use when the user wants to prototype, sanity-check a data model or state machine, mock up a UI, explore design options, or says "prototype this", "let me play with it", "try a few designs". +--- + +# Prototype + +A prototype is **throwaway code that answers a question**. The question decides the shape. + +## Pick a branch + +Identify which question is being answered — from the user's prompt, the surrounding code, or by asking if the user is around: + +- **"Does this logic / state model feel right?"** → [LOGIC.md](LOGIC.md). Build a tiny interactive terminal app that pushes the state machine through cases that are hard to reason about on paper. +- **"What should this look like?"** → [UI.md](UI.md). Generate several radically different UI variations on a single route, switchable via a URL search param and a floating bottom bar. + +The two branches produce fundamentally different artifacts — getting this wrong wastes the whole prototype. If the question is genuinely ambiguous and the user isn't reachable, default to whichever branch better matches the surrounding code (a backend module → logic; a page or component → UI) and state the assumption at the top of the prototype. + +## Rules that apply to both + +1. **Throwaway from day one, and clearly marked as such.** Locate the prototype code close to where it will actually be used (next to the module or page it's prototyping for) so context is obvious — but name it so a casual reader can see it's a prototype, not production. For throwaway UI routes, obey whatever routing convention the project already uses; don't invent a new top-level structure. +2. **One command to run.** Whatever the project's existing task runner supports — `pnpm `, `python `, `bun `, etc. The user must be able to start it without thinking. +3. **No persistence by default.** State lives in memory. Persistence is the thing the prototype is *checking*, not something it should depend on. If the question explicitly involves a database, hit a scratch DB or a local file with a clear "PROTOTYPE — wipe me" name. +4. **Skip the polish.** No tests, no error handling beyond what makes the prototype *runnable*, no abstractions. The point is to learn something fast and then delete it. +5. **Surface the state.** After every action (logic) or on every variant switch (UI), print or render the full relevant state so the user can see what changed. +6. **Delete or absorb when done.** When the prototype has answered its question, either delete it or fold the validated decision into the real code — don't leave it rotting in the repo. + +## When done + +The *answer* is the only thing worth keeping from a prototype. Capture it somewhere durable (commit message, ADR, issue, or a `NOTES.md` next to the prototype) along with the question it was answering. If the user is around, that capture is a quick conversation; if not, leave the placeholder so they (or you, on the next pass) can fill in the verdict before deleting the prototype. diff --git a/plugins/bin/.apm/skills/prototype/UI.md b/plugins/bin/.apm/skills/prototype/UI.md new file mode 100644 index 0000000..f3b6e64 --- /dev/null +++ b/plugins/bin/.apm/skills/prototype/UI.md @@ -0,0 +1,112 @@ +# UI Prototype + +Generate **several radically different UI variations** on a single route, switchable from a floating bottom bar. The user flips between variants in the browser, picks one (or steals bits from each), then throws the rest away. + +If the question is about logic/state rather than what something looks like — wrong branch. Use [LOGIC.md](LOGIC.md). + +## When this is the right shape + +- "What should this page look like?" +- "I want to see a few options for this dashboard before committing." +- "Try a different layout for the settings screen." +- Any time the user would otherwise spend a day picking between three vague mockups in their head. + +## Two sub-shapes — strongly prefer sub-shape A + +A UI prototype is much easier to judge when it's **butting up against the rest of the app** — real header, real sidebar, real data, real density. A throwaway route on its own is a vacuum: every variant looks fine in isolation. Default to sub-shape A whenever there's a plausible existing page to host the variants. Only reach for sub-shape B if the prototype genuinely has no nearby home. + +### Sub-shape A — adjustment to an existing page (preferred) + +The route already exists. Variants are rendered **on the same route**, gated by a `?variant=` URL search param. The existing data fetching, params, and auth all stay — only the rendering swaps. This is the default; pick it unless there's a specific reason not to. + +If the prototype is for something that doesn't yet have a page but *would naturally live inside one* (a new section of the dashboard, a new card on the settings screen, a new step in an existing flow) — that's still sub-shape A. Mount the variants inside the host page. + +### Sub-shape B — a new page (last resort) + +Only use this when the thing being prototyped genuinely has no existing page to live inside — e.g. an entirely new top-level surface, or a flow that can't be embedded anywhere sensible. + +Create a **throwaway route** following whatever routing convention the project already uses — don't invent a new top-level structure. Name it so it's obviously a prototype (e.g. include the word `prototype` in the path or filename). Same `?variant=` pattern. + +Before committing to sub-shape B, sanity-check: is there really no existing page this could be embedded in? An empty route hides design problems that a populated one would expose. + +In both sub-shapes the floating bottom bar is identical. + +## Process + +### 1. State the question and pick N + +Default to **3 variants**. More than 5 stops being radically different and starts being noise — cap there. + +Write down the plan in one line, in the prototype's location or a top-of-file comment: + +> "Three variants of the settings page, switchable via `?variant=`, on the existing `/settings` route." + +This works whether the user is here to push back or not. + +### 2. Generate radically different variants + +Draft each variant. Hold each one to: + +- The page's purpose and the data it has access to. +- The project's component library / styling system (TailwindCSS, shadcn, MUI, plain CSS, whatever). +- A clear exported component name, e.g. `VariantA`, `VariantB`, `VariantC`. + +Variants must be **structurally different** — different layout, different information hierarchy, different primary affordance, not just different colours. Three slightly-tweaked card grids isn't a UI prototype, it's wallpaper. If two drafts come out too similar, redo one with explicit "do not use a card grid" guidance. + +### 3. Wire them together + +Create a single switcher component on the route: + +```tsx +// pseudo-code — adapt to the project's framework +const variant = searchParams.get('variant') ?? 'A'; +return ( + <> + {variant === 'A' && } + {variant === 'B' && } + {variant === 'C' && } + + +); +``` + +For sub-shape A (existing page): keep all the existing data fetching above the switcher; only the rendered subtree changes per variant. + +For sub-shape B (new page): the throwaway route under `/prototype/` mounts the same switcher. + +### 4. Build the floating switcher + +A small fixed-position bar at the bottom-centre of the screen with three pieces: + +- **Left arrow** — cycles to the previous variant (wraps around). +- **Variant label** — shows the current variant key and, if the variant exports a name, that name too. e.g. `B — Sidebar layout`. +- **Right arrow** — cycles forward (wraps around). + +Behaviour: + +- Clicking an arrow updates the URL search param (use the framework's router — `router.replace` on Next, `navigate` on React Router, etc) so the variant is shareable and reload-stable. +- Keyboard: `←` and `→` arrow keys also cycle. Don't intercept arrow keys when an ``, `