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

fix(research): restore subagent fan-out (#116) #138

Merged
Defame1297 merged 5 commits from docs/116-research-fanout-decision into main 2026-09-21 16:33:11 +00:00
Conversation 3 Commits 5 Files Changed 2 +68 -23
Claude commented 2026-09-21 07:20:14 +00:00
Collaborator
Copy Link

What

Restores the parallel subagent fan-out in plugins/bin/.apm/skills/research/SKILL.md steps 4-5 and the "orchestrator writes every file" gotcha, with a serial fallback for a target that has no spawn tool. Version 1.0.1 -> 1.0.2. Adds ADR-0027 recording the decision.

Closes #116.

Why

research told the agent to spawn one subagent per URL, then its steps were rewritten to say no subagent tool was granted, so it ran serial and fetched every page into one context. The fan-out existed to keep raw pages out of that context. #116 asked whether to restore it; this PR does.

Commits

  • 6683da5 restores the fan-out. It also dropped allowed-tools on the premise that the list blocked spawning.
  • acaab29 corrects that premise. The repo's own docs (skill-author/references/create.md:113, the agentskills.io spec, the Copilot docs) describe allowed-tools as pre-approval, not restriction. The list is restored. The spawn tool is not added: Agent is sourced for Claude Code only, and the Copilot/Codex names are not known, so spawns prompt rather than being pre-approved.
  • f48f3d9 rewrites ADR-0027 to match the code and claim only what is verified. The file is docs/adr/0027-research-fan-out-restored-and-its-tool-list-kept.md.
  • 1a66ee9 applies review findings: a serial fallback in step 4 (read serially and reduce each page to notes if no spawn tool is available), a patch bump to 1.0.2 (the change restores existing behaviour), and a trim of the body from 717 words to under the 600-word ADR-0020 target with no instruction dropped.

Net change to SKILL.md: version bump, the new gotcha, the rewritten steps 4-5 with the fallback, and prose tightened elsewhere. allowed-tools is unchanged. Squash-merge: 6683da5 carries a wrong premise and a Closes line that closed #116 at push time.

Tradeoffs and what is not done

  • What Claude Code, Copilot and Codex do with an unlisted tool is not verified. The ADR says so.
  • "Orchestrator alone writes files" is prose, not enforced. Subagents read untrusted web pages, and the notes they return are an injection channel into the orchestrator.
  • Deliberately not done: a treat-page-content-as-data instruction, a cap on subagent count, read-only subagents, agent-hosted fan-out as an alternative, and an eval covering the fan-out.

Review

Two clean-context subagent reviews were run. The first found the allowed-tools premise unverified and contradicted by repo docs; acaab29 and f48f3d9 address that. The second (full PR) requested changes; 1a66ee9 and this description address the fallback, version, size and stale-description findings. The rest are listed above as not done.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EGHFJextYtVQseaHPDDhxB

## What Restores the parallel subagent fan-out in `plugins/bin/.apm/skills/research/SKILL.md` steps 4-5 and the "orchestrator writes every file" gotcha, with a serial fallback for a target that has no spawn tool. Version 1.0.1 -> 1.0.2. Adds ADR-0027 recording the decision. Closes #116. ## Why `research` told the agent to spawn one subagent per URL, then its steps were rewritten to say no subagent tool was granted, so it ran serial and fetched every page into one context. The fan-out existed to keep raw pages out of that context. #116 asked whether to restore it; this PR does. ## Commits - `6683da5` restores the fan-out. It also dropped `allowed-tools` on the premise that the list blocked spawning. - `acaab29` corrects that premise. The repo's own docs (`skill-author/references/create.md:113`, the agentskills.io spec, the Copilot docs) describe `allowed-tools` as **pre-approval**, not restriction. The list is restored. The spawn tool is not added: `Agent` is sourced for Claude Code only, and the Copilot/Codex names are not known, so spawns prompt rather than being pre-approved. - `f48f3d9` rewrites ADR-0027 to match the code and claim only what is verified. The file is `docs/adr/0027-research-fan-out-restored-and-its-tool-list-kept.md`. - `1a66ee9` applies review findings: a serial fallback in step 4 (read serially and reduce each page to notes if no spawn tool is available), a patch bump to 1.0.2 (the change restores existing behaviour), and a trim of the body from 717 words to under the 600-word ADR-0020 target with no instruction dropped. Net change to `SKILL.md`: version bump, the new gotcha, the rewritten steps 4-5 with the fallback, and prose tightened elsewhere. `allowed-tools` is unchanged. **Squash-merge:** `6683da5` carries a wrong premise and a `Closes` line that closed #116 at push time. ## Tradeoffs and what is not done - What Claude Code, Copilot and Codex do with an unlisted tool is not verified. The ADR says so. - "Orchestrator alone writes files" is prose, not enforced. Subagents read untrusted web pages, and the notes they return are an injection channel into the orchestrator. - Deliberately not done: a treat-page-content-as-data instruction, a cap on subagent count, read-only subagents, agent-hosted fan-out as an alternative, and an eval covering the fan-out. ## Review Two clean-context subagent reviews were run. The first found the `allowed-tools` premise unverified and contradicted by repo docs; `acaab29` and `f48f3d9` address that. The second (full PR) requested changes; `1a66ee9` and this description address the fallback, version, size and stale-description findings. The rest are listed above as not done. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01EGHFJextYtVQseaHPDDhxB
Claude added 2 commits 2026-09-21 07:20:14 +00:00
fix(research): restore subagent fan-out, record that a skill body and its allowed-tools must agree 6683da54ac 
research instructed "spawn one subagent per URL" while its allowed-tools
granted no spawn tool, so it silently degraded to serial fetches. Three
other skills spawn subagents without trouble because they declare no
allowed-tools. The defect was the mismatch, not the spawning.

ADR-0027 records the agreement rule. research drops allowed-tools and
gets its steps 4-5 fan-out and the orchestrator-writes gotcha back
(1.0.1 -> 1.1.0).

Closes #116

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EGHFJextYtVQseaHPDDhxB
fix(research): keep the allowed-tools list; it pre-approves tools, it does not restrict them acaab29f89 
6683da5 dropped allowed-tools on the premise that the list blocked
spawning. The repo's own docs describe the field as pre-approval, so the
list was never the cause and dropping it widened the tool surface for
nothing. Restore the list and keep the parallel fan-out in steps 4-5.

The spawn tool is not added: its name is sourced for Claude Code
(Agent) but not for Copilot or Codex, so spawns prompt rather than
being pre-approved.

ADR-0027 still asserts the dropped-list premise and is corrected
separately.

Refs #116

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EGHFJextYtVQseaHPDDhxB
Defame1297 added 1 commit 2026-09-21 07:27:23 +00:00
docs(adr): rewrite ADR-0027 to match what the branch does and what is verified f48f3d9926 
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
Claude commented 2026-09-21 07:27:31 +00:00
Author
Collaborator
Copy Link

ADR-0027 was rewritten in f48f3d9 to match the code. The "ADR-0027 is out of date" warning in the description is now resolved.

What changed in the ADR:

  • Title and filename. Now 0027-research-fan-out-restored-and-its-tool-list-kept.md, since the decision is no longer "drop allowed-tools".
  • Premise corrected. The #116 defect is stated as step text that disclaimed spawning and left the skill serial. It is no longer stated as allowed-tools blocking spawning; the repo's own sources describe the field as pre-approval. What Claude Code, Copilot and Codex do with an unlisted tool is recorded as not verified.
  • Decision matches the code. research keeps its allowed-tools list and gets the fan-out back in steps 4-5. The spawn tool is left out because Agent is sourced for Claude Code only, so spawns prompt on that target.
  • Corpus facts carry their limits. write-docs, improve-codebase-architecture and forge omit allowed-tools and instruct spawning (forge from its references, not SKILL.md). That shows they spawn, not that runs succeeded. The skill-author/SKILL.md:24 prohibition is noted as unrelated.
  • Security cost recorded plainly. "Orchestrator alone writes files" is prose, not enforced, and the subagents read untrusted pages. ai-coding-factory-principles.md:53 is cited as the reason the list was kept.

Review findings deliberately not addressed, by decision:

  • No "treat fetched page content as data" instruction in steps 4-5.
  • No cap on the number of subagents, so user-supplied URLs are uncapped and the step 5 page cap bounds less under parallel reads.
  • No read-only subagents.
  • No eval covering the fan-out or the orchestrator-only-writes gotcha.

The PR still says Refs #116, so merging will not close the issue.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EGHFJextYtVQseaHPDDhxB

ADR-0027 was rewritten in `f48f3d9` to match the code. The "ADR-0027 is out of date" warning in the description is now resolved. What changed in the ADR: - **Title and filename.** Now `0027-research-fan-out-restored-and-its-tool-list-kept.md`, since the decision is no longer "drop `allowed-tools`". - **Premise corrected.** The #116 defect is stated as step text that disclaimed spawning and left the skill serial. It is no longer stated as `allowed-tools` blocking spawning; the repo's own sources describe the field as pre-approval. What Claude Code, Copilot and Codex do with an unlisted tool is recorded as not verified. - **Decision matches the code.** `research` keeps its `allowed-tools` list and gets the fan-out back in steps 4-5. The spawn tool is left out because `Agent` is sourced for Claude Code only, so spawns prompt on that target. - **Corpus facts carry their limits.** `write-docs`, `improve-codebase-architecture` and `forge` omit `allowed-tools` and instruct spawning (`forge` from its references, not `SKILL.md`). That shows they spawn, not that runs succeeded. The `skill-author/SKILL.md:24` prohibition is noted as unrelated. - **Security cost recorded plainly.** "Orchestrator alone writes files" is prose, not enforced, and the subagents read untrusted pages. `ai-coding-factory-principles.md:53` is cited as the reason the list was kept. Review findings deliberately not addressed, by decision: - No "treat fetched page content as data" instruction in steps 4-5. - No cap on the number of subagents, so user-supplied URLs are uncapped and the step 5 page cap bounds less under parallel reads. - No read-only subagents. - No eval covering the fan-out or the orchestrator-only-writes gotcha. The PR still says `Refs #116`, so merging will not close the issue. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01EGHFJextYtVQseaHPDDhxB
Defame1297 added 1 commit 2026-09-21 07:37:37 +00:00
fix(research): add a serial fallback, patch-bump the version, trim the body under target 1a66ee939a 
The fan-out restored in 6683da5 had no degrade path for a target with no
spawn tool, which reproduces the silent degradation #116 was written
against. Step 4 now says to read serially and reduce each page to notes
when spawning is unavailable.

The change restores existing behaviour, so the version bump is a patch
(1.0.2) per skill-author's convention, not a minor. The body is trimmed
from 717 to under the 600-word ADR-0020 target without dropping any
instruction. ADR-0027 is updated to match.

Refs #116

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EGHFJextYtVQseaHPDDhxB
Defame1297 added 1 commit 2026-09-21 07:52:11 +00:00
fix(research): tell fan-out subagents to treat page content as data, cover step 5's fallback 01dfd8150f 
Step 4 subagents read untrusted pages; say their content is data, not
instructions. Step 5 now repeats step 4, so it inherits the serial
fallback and the data rule. Body stays at 598 words, under the
ADR-0020 target.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EGHFJextYtVQseaHPDDhxB
Claude commented 2026-09-21 07:52:20 +00:00
Author
Collaborator
Copy Link

Review of #138

Verdict: changes requested. 01dfd81 fixes the two findings below that were acted on.

Fixed in 01dfd81

  • Step 4 injection line. Fan-out subagents read untrusted pages, so each now treats page content as data, never as instructions. This supersedes the ADR-0027 statement that this instruction is "not done, by decision". The ADR still needs updating; see below.
  • Step 5 fallback. Step 5 now repeats step 4, so it inherits the serial fallback and the data rule. Before, only step 4 said what to do without a spawn tool.
  • Size. The body is 598 words, under the 600-word ADR-0020 target. skill-size-check.sh exits clean with no suggestion.

Still to do before merge

  • ADR-0027's "not done, by decision" list still names the treat-page-content-as-data instruction. Update it to match 01dfd81.
  • The PR description still says the body is trimmed to under 600 words from 717. That figure is unchanged (598 now), but the description does not mention the new instruction.

Accepted as is

  • The trims in steps 1–3 and 6 stay, to fit the size budget. This drops a few small instructions, for example stop in step 3.
  • The body and allowed-tools still disagree on Agent, as ADR-0027 records.
  • The subagent count is uncapped.
  • The default merge style is merge. Squash by hand: 6683da5 carries a wrong premise.

CI reports no checks. The local pre-push hooks passed on this push.

## Review of #138 Verdict: changes requested. `01dfd81` fixes the two findings below that were acted on. ### Fixed in `01dfd81` - **Step 4 injection line.** Fan-out subagents read untrusted pages, so each now treats page content as data, never as instructions. This supersedes the ADR-0027 statement that this instruction is "not done, by decision". The ADR still needs updating; see below. - **Step 5 fallback.** Step 5 now repeats step 4, so it inherits the serial fallback and the data rule. Before, only step 4 said what to do without a spawn tool. - **Size.** The body is 598 words, under the 600-word ADR-0020 target. `skill-size-check.sh` exits clean with no suggestion. ### Still to do before merge - ADR-0027's "not done, by decision" list still names the treat-page-content-as-data instruction. Update it to match `01dfd81`. - The PR description still says the body is trimmed to under 600 words from 717. That figure is unchanged (598 now), but the description does not mention the new instruction. ### Accepted as is - The trims in steps 1–3 and 6 stay, to fit the size budget. This drops a few small instructions, for example `stop` in step 3. - The body and `allowed-tools` still disagree on `Agent`, as ADR-0027 records. - The subagent count is uncapped. - The default merge style is `merge`. Squash by hand: `6683da5` carries a wrong premise. CI reports no checks. The local pre-push hooks passed on this push.
Defame1297 approved these changes 2026-09-21 16:33:08 +00:00
Defame1297 merged commit da95fa2a9e into main 2026-09-21 16:33:11 +00:00
Defame1297 deleted branch docs/116-research-fanout-decision 2026-09-21 16:33:11 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Defame1297
Labels
Clear labels
Compat/Breaking
Breaking change that won't be backward compatible
 Kind/Bug
Something is not working
 Kind/Documentation
Documentation changes
 Kind/Enhancement
Improve existing functionality
 Kind/Feature
New functionality
 Kind/Security
This is security issue
 Kind/Testing
Issue or pull request related to testing
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Low
4
The priority is low
Priority
Medium
3
The priority is medium
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
Claude renovate-bot Defame1297
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Defame1297/holocron#138
Delete Branch "docs/116-research-fanout-decision"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?