diff --git a/AGENTS.md b/AGENTS.md index 5165438..d27c759 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -29,7 +29,7 @@ Copilot, and by Cursor, Codex, and Claude (via `CLAUDE.md` → `@AGENTS.md`). └── update-agent-skills.yaml # Daily gh skill update --all; opens a PR when upstream skills drift plugins/ └── / - ├── plugin.json # Plugin manifest (kebab-case name, description, version, skills: "skills/") + ├── plugin.json # Plugin manifest (kebab-case name, description, version; resources auto-discovered — no skills/agents path fields) └── skills/ └── /SKILL.md # An installed skill copied from upstream, with metadata.github-* provenance scripts/ @@ -102,12 +102,18 @@ membership) is authored here. `name`/`description`/`version`/`source`; CI enforces the diff. Edit both together. 2. **Plugin layout.** A plugin is a directory under `plugins/` with a `plugin.json` (kebab-case `name` matching `^[a-z0-9-]+$`, a `description`, a `version`) that declares **at least one resource**: - a `skills/` subdirectory (`"skills": "skills/"`), a bundled `.mcp.json` (MCP servers), and/or an - `agents/` directory (`"agents": "agents/"`). Skill dirs sit at `plugins//skills//` and - each holds a conformant `SKILL.md` (CI discovers them at depth 4). A bundled `.mcp.json` is a - `{ "mcpServers": { … } }` map whose every server carries a `command` (stdio) or `url` (remote). A - bundled `agents/` directory holds ≥1 `agents/*.md`, each with YAML frontmatter carrying a non-empty - `name` and `description` (the neutral cross-tool core). See + a `skills/` subdirectory, a bundled `.mcp.json` (MCP servers), and/or an `agents/` directory. Every + resource is **auto-discovered from its directory** — the `plugin.json` carries **no** component-path + fields. Both Claude Code and Copilot CLI default to `skills/` and `agents/` when the field is + omitted, and **Claude Code rejects the bare-string form** (`"skills": "skills/"` → + `skills: Invalid input`), which breaks `claude plugin install`; the portable manifest therefore omits + it (the field is only valid as a `string[]` path list, never a plain string). CI's + `validate-manifests.sh` enforces this — it counts resources by their on-disk directories and fails + any `plugin.json` that sets `skills`/`agents` to a non-array. Skill dirs sit at + `plugins//skills//` and each holds a conformant `SKILL.md` (CI discovers them at + depth 4). A bundled `.mcp.json` is a `{ "mcpServers": { … } }` map whose every server carries a + `command` (stdio) or `url` (remote). A bundled `agents/` directory holds ≥1 `agents/*.md`, each with + YAML frontmatter carrying a non-empty `name` and `description` (the neutral cross-tool core). See [ADR 0001](docs/adr/0001-bundling-mcp-servers-and-custom-agents.md) for the cross-tool delivery model. 3. **agentskills.io spec.** Every bundled `SKILL.md` must validate against the [`agentskills.io`](https://agentskills.io) spec — CI validates each discovered skill in a matrix. diff --git a/docs/adr/0001-bundling-mcp-servers-and-custom-agents.md b/docs/adr/0001-bundling-mcp-servers-and-custom-agents.md index 0b6b264..e80e7eb 100644 --- a/docs/adr/0001-bundling-mcp-servers-and-custom-agents.md +++ b/docs/adr/0001-bundling-mcp-servers-and-custom-agents.md @@ -7,6 +7,15 @@ > 🤖 Generated by the Daily AI Assistant +> **Correction (2026-07-03).** This ADR originally asserted that the component-path field +> `"skills": "skills/"` (and `"agents": "agents/"`) is a plain-string field "shared by both the Claude +> Code and Copilot CLI plugin schemas". That is **wrong for Claude Code**: its `plugin.json` schema +> types `skills`/`agents` as `string[]` (or omitted), so the bare-string form is rejected with +> `skills: Invalid input` and **`claude plugin install` fails**. Copilot CLI tolerates the string, but +> the portable contract is to **omit the fields entirely** — both tools auto-discover the default +> `skills/` and `agents/` directories. The tables and CI notes below are corrected accordingly; the +> resource model (§D1) and the rest of the decision stand. + ## Context `AGENTS.md` and the README already commit this marketplace to being **tool-neutral and not @@ -94,20 +103,20 @@ that VS Code is the sole consume-via-config surface, not imply that Copilot need ### D1 — Resource model: a plugin may provide skills, MCP servers, and/or custom agents -A plugin is valid if it provides **at least one** recognized resource. The three resource types and -their on-disk conventions use the component-path fields **shared by both the Claude Code and Copilot CLI -plugin schemas** (same field names, same defaults — no tool-specific invention): +A plugin is valid if it provides **at least one** recognized resource. Every resource is +**auto-discovered from its on-disk directory**; `plugin.json` carries **no** component-path fields +(both Claude Code and Copilot CLI default to `skills/` / `agents/` when the field is omitted, and the +bare-string form breaks Claude Code — see the correction note above): | Resource | On-disk | `plugin.json` | |---|---|---| -| Skills | `skills//SKILL.md` (+ provenance frontmatter) | `"skills": "skills/"` (default `skills/` in both schemas) | -| MCP servers | `.mcp.json` at plugin root (`{ "mcpServers": { … } }`) | optional `"mcpServers": ".mcp.json"` (Copilot resolves `mcpServers`→`.mcp.json`/`.github/mcp.json`; Claude Code auto-discovers `.mcp.json`) | -| Custom agents | `agents/.md` (md + YAML frontmatter; `.agent.md` for Copilot) | optional `"agents": "agents/"` (default `agents/` in both schemas) | +| Skills | `skills//SKILL.md` (+ provenance frontmatter) | omitted — auto-discovered from `skills/` (the field is only valid as a `string[]`, never `"skills/"`) | +| MCP servers | `.mcp.json` at plugin root (`{ "mcpServers": { … } }`) | omitted — Copilot resolves `mcpServers`→`.mcp.json`/`.github/mcp.json`; Claude Code auto-discovers `.mcp.json` | +| Custom agents | `agents/.md` (md + YAML frontmatter; `.agent.md` for Copilot) | omitted — auto-discovered from `agents/` (the field is only valid as a `string[]`, never `"agents/"`) | -`plugin.json` keeps `name` / `description` / `version` / `author`. The `"skills": "skills/"` field -becomes **conditional** — present only when the plugin actually ships skills (both tools treat the -component-path fields as optional and fall back to the default dirs, so an absent field is safe). The -component-path fields are explicit here so the manifest is self-describing for the CI gate (D3). +`plugin.json` keeps only `name` / `description` / `version` / `author` (+ `keywords`). The component-path +fields are **omitted**, so the manifest is portable across both tools; the CI gate (D3) is therefore +directory-based — it counts resources by their on-disk directories, not by a manifest field. ### D2 — Canonical MCP model → bundled (Claude Code + Copilot CLI) + documented (VS Code) @@ -135,9 +144,10 @@ check is added for MCP well-formedness: 1. **`validate_plugin_json`** — replace the unconditional `"skills" == "skills/"` + non-empty `skills/` requirement with: *the plugin must ship ≥1 recognized resource.* Per resource present, keep/add: - - `skills/` present → keep all existing skill checks (the `skills/` field equals `"skills/"`, ≥1 - `SKILL.md`, and the existing per-skill provenance + agentskills.io spec checks). **Unchanged for - every plugin shipping today.** + - `skills/` present → detected **on-disk** (a `skills//SKILL.md` exists), with the existing + per-skill provenance + agentskills.io spec checks. The old `.skills == "skills/"` field assertion + is **dropped** (the field is omitted now); in its place a guard rejects any `skills`/`agents` field + that is set to a non-array, so the Claude-Code-breaking bare-string form can never return. - `.mcp.json` present → it must be valid JSON with a non-empty `.mcpServers` object, and each server must have a `command` (stdio) **or** a `url` (remote). (New check.) - `agents/` present → ≥1 `agents/*.md`, each with YAML frontmatter carrying `name` + `description`. diff --git a/plugins/agentic-engineering/plugin.json b/plugins/agentic-engineering/plugin.json index db8f43e..2dccd5b 100644 --- a/plugins/agentic-engineering/plugin.json +++ b/plugins/agentic-engineering/plugin.json @@ -17,6 +17,5 @@ "mcp", "skills", "prompt-engineering" - ], - "skills": "skills/" + ] } diff --git a/plugins/engineering-practices/plugin.json b/plugins/engineering-practices/plugin.json index a1f3f3a..bf237f7 100644 --- a/plugins/engineering-practices/plugin.json +++ b/plugins/engineering-practices/plugin.json @@ -15,6 +15,5 @@ "test-driven-development", "code-quality", "engineering" - ], - "skills": "skills/" + ] } diff --git a/plugins/frontend-design/plugin.json b/plugins/frontend-design/plugin.json index 0d50e44..0a1ed1b 100644 --- a/plugins/frontend-design/plugin.json +++ b/plugins/frontend-design/plugin.json @@ -15,6 +15,5 @@ "css", "accessibility", "design" - ], - "skills": "skills/" + ] } diff --git a/plugins/github/plugin.json b/plugins/github/plugin.json index 7944218..e87db9a 100644 --- a/plugins/github/plugin.json +++ b/plugins/github/plugin.json @@ -16,6 +16,5 @@ "workflows", "issues", "ci-cd" - ], - "skills": "skills/" + ] } diff --git a/plugins/gitops-kubernetes/plugin.json b/plugins/gitops-kubernetes/plugin.json index c6ac846..de3f3fa 100644 --- a/plugins/gitops-kubernetes/plugin.json +++ b/plugins/gitops-kubernetes/plugin.json @@ -18,7 +18,5 @@ "multi-tenancy", "helm", "kustomize" - ], - "skills": "skills/", - "agents": "agents/" + ] } diff --git a/plugins/go/plugin.json b/plugins/go/plugin.json index d019766..724d160 100644 --- a/plugins/go/plugin.json +++ b/plugins/go/plugin.json @@ -14,6 +14,5 @@ "interfaces", "testing", "microservices" - ], - "skills": "skills/" + ] } diff --git a/scripts/validate-manifests.sh b/scripts/validate-manifests.sh index 570fcd9..bfe6320 100755 --- a/scripts/validate-manifests.sh +++ b/scripts/validate-manifests.sh @@ -140,18 +140,27 @@ validate_plugin_json() { echo "::error::$pj: missing or empty 'version' field" ok=0 fi - # Skills resource: when '.skills' is declared it must be "skills/" and contain at - # least one /SKILL.md. (Existing check — unchanged, only made conditional.) - if [ "$(jq -e 'has("skills")' "$pj")" = "true" ]; then - if [ "$(jq -r '.skills // ""' "$pj")" != "skills/" ]; then - echo "::error::$pj: 'skills' must be \"skills/\"" + # Component-path fields (skills/agents), when present, MUST be arrays. Claude Code rejects + # the bare-string form ('"skills": "skills/"' → 'skills: Invalid input'), which breaks + # 'claude plugin install' even though Copilot CLI tolerates it. Both tools auto-discover + # the default skills/ and agents/ dirs when the field is omitted, so omitting it is the + # portable form and what these plugins do — this guard just stops the broken string form + # from returning. + for field in skills agents; do + if [ "$(jq -e --arg f "$field" 'has($f)' "$pj")" = "true" ] \ + && [ "$(jq -r --arg f "$field" '.[$f] | type' "$pj")" != "array" ]; then + echo "::error::$pj: '$field' must be an array of paths, or omitted to auto-discover $field/ (Claude Code rejects the bare-string form)" ok=0 - elif ! find "$plugin_dir/skills" -mindepth 2 -maxdepth 2 -name SKILL.md -print -quit 2>/dev/null | grep -q .; then - echo "::error::$plugin_dir: 'skills/' must contain at least one /SKILL.md" - ok=0 - else - resource_count=$((resource_count + 1)) fi + done + # Skills resource (ADR 0001 §D3): auto-discovered from the on-disk skills/ directory — + # both tools default to skills/ when the manifest omits the field — so detection is + # directory-based, not field-based. A skills/ dir must hold >=1 /SKILL.md to count. + if find "$plugin_dir/skills" -mindepth 2 -maxdepth 2 -name SKILL.md -print -quit 2>/dev/null | grep -q .; then + resource_count=$((resource_count + 1)) + elif [ -d "$plugin_dir/skills" ]; then + echo "::error::$plugin_dir: 'skills/' present but contains no /SKILL.md" + ok=0 fi # MCP resource: a bundled .mcp.json at the plugin root must validate. if [ -f "$plugin_dir/.mcp.json" ]; then diff --git a/scripts/validate-manifests.test.sh b/scripts/validate-manifests.test.sh index f0bc573..b654e98 100755 --- a/scripts/validate-manifests.test.sh +++ b/scripts/validate-manifests.test.sh @@ -58,12 +58,14 @@ metadata: --- Example skill. EOF + # No "skills" field: skills are auto-discovered from the on-disk skills/ dir by both + # Claude Code and Copilot CLI. Claude Code rejects the bare-string "skills": "skills/" + # form, so the portable manifest omits it — the fixture mirrors the real plugins. cat > "$root/plugins/$name/plugin.json" < "$d/tmp" && mv "$d/tmp" "$d/plugins/alpha/plugin.json" check_fail "missing plugin.json version fails" "missing or empty 'version'" "$d" -d=$(fresh); jq '.skills = "wrong/"' "$d/plugins/alpha/plugin.json" > "$d/tmp" && mv "$d/tmp" "$d/plugins/alpha/plugin.json" -check_fail "wrong 'skills' value fails" "'skills' must be" "$d" +# The bare-string 'skills'/'agents' form is exactly what breaks 'claude plugin install' +# ('skills: Invalid input'); the guard must reject it and demand the array-or-omitted form. +d=$(fresh); jq '.skills = "skills/"' "$d/plugins/alpha/plugin.json" > "$d/tmp" && mv "$d/tmp" "$d/plugins/alpha/plugin.json" +check_fail "bare-string 'skills' field fails" "'skills' must be an array of paths" "$d" +d=$(fresh); jq '.agents = "agents/"' "$d/plugins/alpha/plugin.json" > "$d/tmp" && mv "$d/tmp" "$d/plugins/alpha/plugin.json" +check_fail "bare-string 'agents' field fails" "'agents' must be an array of paths" "$d" + +# The array form is accepted (auto-discovery still finds the on-disk skills either way). +d=$(fresh); jq '.skills = ["skills/example-skill"]' "$d/plugins/alpha/plugin.json" > "$d/tmp" && mv "$d/tmp" "$d/plugins/alpha/plugin.json" +check_pass "array 'skills' field passes" "$d" + +# A skills/ dir present but holding no /SKILL.md is a broken bundle. +d=$(fresh); rm -f "$d/plugins/alpha/skills/example-skill/SKILL.md" +check_fail "skills/ dir with no SKILL.md fails" "'skills/' present but contains no /SKILL.md" "$d" + +# A plugin declaring no resource at all (no skills/, no .mcp.json, no agents/) is invalid. d=$(fresh); rm -rf "$d/plugins/alpha/skills" -check_fail "plugin with no SKILL.md fails" "must contain at least one /SKILL.md" "$d" +check_fail "plugin with no resource fails" "must declare at least one resource" "$d" # --- check 4: manifest <-> plugins lockstep --- d=$(fresh)