Defame1297/holocron
0
0
Fork 0
Code Issues 14 Pull Requests Actions Packages Projects Releases 3 Wiki Activity

docs(adr): rewrite ADR-0027 to match what the branch does and what is verified

Browse Source 
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EGHFJextYtVQseaHPDDhxB
This commit is contained in:
Defame1297
2026-09-21 07:23:30 +00:00
parent acaab29f89
commit f48f3d9926
2 changed files with 46 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
docs/adr/0027-a-skill-body-and-its-allowed-tools-must-agree.md
View File

@@ -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).

46
docs/adr/0027-research-fan-out-restored-and-its-tool-list-kept.md Normal file
View File

@@ -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).