diff --git a/docs/adr/0027-a-skill-body-and-its-allowed-tools-must-agree.md b/docs/adr/0027-a-skill-body-and-its-allowed-tools-must-agree.md new file mode 100644 index 0000000..6774732 --- /dev/null +++ b/docs/adr/0027-a-skill-body-and-its-allowed-tools-must-agree.md @@ -0,0 +1,48 @@ +# A skill's body and its `allowed-tools` must agree — a spawning step needs a spawn tool or no list + +**Status:** Accepted (2026-09-21) + +`plugins/bin/.apm/skills/research/SKILL.md` once told the agent to "spawn one subagent per URL" +while its `allowed-tools` granted nothing that spawns. `WebFetch` was granted, so it never +hard-failed: it degraded to serial fetches in the orchestrator's own context, and the "in +parallel" wording, the page cap and the "subagents summarise, orchestrator writes" gotcha all +quietly stopped meaning anything. The #99 retrofit rewrote steps 4 and 5 as honest serial reads +with a real page cap and per-page reduction to notes (#116). + +The defect was a **mismatch between what the body instructs and what `allowed-tools` permits**. It +was not that a skill spawned subagents. Three skills spawn today and work: `write-docs` (Reader +Testing sub-agent), `improve-codebase-architecture` (`Explore`, and 3+ parallel sub-agents in +`references/interface-design.md`) and `forge` (fork and clean-context subagents). None declares +`allowed-tools`, so all inherit every tool, spawning included. `research` was the only one that +both restricted the list and instructed spawning. + +**Decision: a skill may instruct spawning subagents, provided its `allowed-tools` agrees with its +body.** Either: + +- omit `allowed-tools`, so the skill inherits every tool on every target; or +- list the spawn tool — only once its per-target name is known, since `allowed-tools` is a flat list + and `claude`, `copilot` and `codex` name it differently. + +A step that needs a tool the list does not grant must be rewritten as a step that does not need it. +That is what the #99 retrofit did to `research`, and it stays correct until one of the two options +above is taken. + +Fan-out is not confined to agents. `CONTEXT.md` says a plugin-scope agent *delegates to skills* +because it cannot disclose to itself; it does not say skills may not delegate. + +## Consequence for `research` + +Its fan-out is restored, and `allowed-tools` is dropped to do it (version 1.0.1 → 1.1.0). That is +the first of the two options above; the second is unavailable until the per-target spawn tool names +are known. + +The cost is stated rather than hidden: `research` fetches arbitrary web pages, and inheriting every +tool widens what an injected page could reach for. Two things bound it. The subagents only read and +summarise, and the orchestrator alone writes files, so the write surface is unchanged in intent. And +the least-privilege list was never enforceable across targets anyway, since it could not name a +spawn tool. If a per-target form of `allowed-tools` appears, restore a list that includes the spawn +tool. + +Rejected: banning spawning in skills (contradicted by three working skills, and unsupported by +`CONTEXT.md`), and guessing a per-target spawn tool name in `allowed-tools` (no per-target form +exists, and a wrong guess reproduces the defect silently). diff --git a/plugins/bin/.apm/skills/research/SKILL.md b/plugins/bin/.apm/skills/research/SKILL.md index b25bb47..941ae36 100644 --- a/plugins/bin/.apm/skills/research/SKILL.md +++ b/plugins/bin/.apm/skills/research/SKILL.md @@ -6,17 +6,8 @@ description: >- documentation written from existing code or specs -> `write-docs`. Not a bug or incident -> `diagnose`. metadata: - version: "1.0.1" + version: "1.1.0" category: research -allowed-tools: - - Grep - - Glob - - Read - - Write - - WebSearch - - WebFetch - - mcp__context7__resolve-library-id - - mcp__context7__query-docs model: sonnet --- @@ -25,6 +16,7 @@ model: sonnet - Never infer the output path. A run writes a directory's worth of files, and a guessed destination scatters them through someone's source tree. If the user named no path, stop and ask. - Write nothing outside the given output path. A file placed beside the agreed directory is one the user never asked for and will not think to look for. - Never write an empty topic file. A stub `troubleshooting.md` reads downstream as researched and closed. +- Subagents read and summarise; the orchestrator writes every file. A subagent that writes has no view of the other subagents' notes, so its files collide with theirs. - A Context7 response that is a "no results" message, a redirect notice, or header-only boilerplate is not coverage. A topic area counts as covered only when the response carries at least one substantive paragraph. ## Step 1 — Scope against the working directory @@ -52,11 +44,11 @@ If nothing usable comes back, stop and report what was searched, then ask for st ## Step 4 — Read the sources -`WebFetch` each URL in turn. No subagent tool is granted here, so the reads are serial and every fetched page lands in this context: reduce each page to notes by topic area, plus the links worth deepening, before fetching the next one. +Spawn one subagent per URL, in parallel. Each fetches its page with `WebFetch` and returns notes by topic area plus the links worth deepening — never the raw page. The pages stay out of this context; only the notes come back. ## Step 5 — Deepen -`WebFetch` the links worth following, still one at a time and still reducing each page to notes. Stop a branch once its content turns repetitive or leaves the topic, and cap the whole step at roughly ten additional pages — serial reads make that cap a real budget, not a formality. +Spawn one further subagent per link worth following, again in parallel and again returning notes only. Stop a branch once its content turns repetitive or leaves the topic, and cap the whole step at roughly ten additional pages. ## Step 6 — Write