From f48f3d99262449e25f8a264f945f1e5b633bdcf7 Mon Sep 17 00:00:00 2001 From: Defame1297 Date: Mon, 21 Sep 2026 07:23:30 +0000 Subject: [PATCH] docs(adr): rewrite ADR-0027 to match what the branch does and what is verified The first draft claimed omitting allowed-tools grants spawning and that the list was a restriction. The repo's own sources describe the field as pre-approval, and the code now keeps the list. Rewrite the ADR to say the #116 defect was step text disclaiming spawning, that per-target behaviour for an unlisted tool is unverified, that the spawn tool is left out because its name is sourced for Claude Code only, and that the orchestrator-writes mitigation is prose, with the unmitigated security cost recorded. Rename to fit the new decision. Refs #116 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01EGHFJextYtVQseaHPDDhxB --- ...l-body-and-its-allowed-tools-must-agree.md | 48 ------------------- ...fan-out-restored-and-its-tool-list-kept.md | 46 ++++++++++++++++++ 2 files changed, 46 insertions(+), 48 deletions(-) delete mode 100644 docs/adr/0027-a-skill-body-and-its-allowed-tools-must-agree.md create mode 100644 docs/adr/0027-research-fan-out-restored-and-its-tool-list-kept.md 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 deleted file mode 100644 index 6774732..0000000 --- a/docs/adr/0027-a-skill-body-and-its-allowed-tools-must-agree.md +++ /dev/null @@ -1,48 +0,0 @@ -# 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/docs/adr/0027-research-fan-out-restored-and-its-tool-list-kept.md b/docs/adr/0027-research-fan-out-restored-and-its-tool-list-kept.md new file mode 100644 index 0000000..0df6d3f --- /dev/null +++ b/docs/adr/0027-research-fan-out-restored-and-its-tool-list-kept.md @@ -0,0 +1,46 @@ +# `research` gets its fan-out back and keeps its tool list; a body must not disclaim spawning + +**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` listed nothing that spawns. `WebFetch` was listed, so nothing hard-failed: +the skill 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 quietly stopped +meaning anything. The #99 retrofit rewrote steps 4 and 5 as serial reads and said in the text that +no subagent tool was granted (#116). + +**What #116 did not establish.** It read the missing tool as the cause. The repo's own sources +describe `allowed-tools` as pre-approval, not restriction: `skill-author/references/create.md:113` +("space-separated pre-approved tools; reduces permission prompts"), the agentskills.io +specification, and the Copilot plugin docs. On that reading an unlisted spawn tool would prompt, not +fail. What Claude Code, Copilot and Codex actually do with an unlisted tool is **not verified +here**, and neither is whether omitting the field grants anything. What is documented is that the +serial behaviour followed the step text, which told the agent to go serial. + +**Decision.** `research` keeps its `allowed-tools` list and gets its parallel fan-out back in steps +4 and 5, with the "subagents read and summarise; the orchestrator writes every file" gotcha +restored (version 1.0.1 → 1.1.0). A skill body that instructs spawning must not be paired with text +saying spawning is unavailable. + +The spawn tool is **not** added to the list. Its name is sourced for Claude Code (`Agent`) only; the +Copilot and Codex names are not known. On Claude Code, spawns therefore prompt instead of being +pre-approved. Add the tool once its name is sourced for each target. + +**Corpus facts, with limits.** `write-docs`, `improve-codebase-architecture` and `forge` all omit +`allowed-tools` and instruct spawning subagents — `forge` from `references/author-routes.md` and +`references/version-bump.md`, not from its `SKILL.md`. That shows they spawn, not that a run +succeeded. `skill-author/SKILL.md:24` forbids spawning a subagent to recheck one's own work, which +is a different question and unaffected here. `CONTEXT.md` says a plugin-scope agent delegates to +skills because it cannot disclose to itself; nothing there bans a skill from delegating. + +**The security cost is real and not mitigated.** "The orchestrator alone writes files" is prose, +not enforcement. The subagents read untrusted web pages, and nothing restricts what tools they +hold. Not done, by decision: an instruction to treat fetched page content as data, a cap on the +number of subagents (user-supplied URLs are uncapped, and the step 5 page cap bounds less once +reads run in parallel), and read-only subagents. `docs/research/ai-coding-factory/ +ai-coding-factory-principles.md:53` recommends applying `allowed-tools` restrictions, which is why +the list was kept. + +Rejected: dropping `allowed-tools` on the premise that it blocked spawning (unsupported by the +repo's own sources, and it widens the tool surface for nothing), and banning spawning in skills +(three skills instruct it, and `CONTEXT.md` does not forbid it).