Skip to content

fix(subagent): lean substrate — no skills by default + opt-in skills param (#32) - #42

Merged
rz1989s merged 2 commits into
mainfrom
fix/issue-32-lean-substrate-no-skills-default
Aug 5, 2026
Merged

fix(subagent): lean substrate — no skills by default + opt-in skills param (#32)#42
rz1989s merged 2 commits into
mainfrom
fix/issue-32-lean-substrate-no-skills-default

Conversation

@rz1989s

@rz1989s rz1989s commented Aug 5, 2026

Copy link
Copy Markdown
Member

What

Root-cause fix for #32: buildChildLoader's skillsOverride loaded ALL installed skills (~42 → ~570K tokens, ~59% of a 976K context window from turn 1) when an agent declared no skills field. Both builtin agents (general-purpose, general-purpose-cc) declare none → every dispatch paid the full substrate cost before doing any work.

Default chosen per RECTOR: (a) none — lean substrate, caller opts in.

Changes

  • src/engine/child-loader.ts — extracted resolveChildSkills() (pure, testable); the no-skills branch now returns [] (not cur.skills). An agent that declares specific skills still loads only those (truthy branch unchanged).
  • src/tools/subagent.ts — added a skills param (Type.Array(Type.String())); callers opt in to specific skills. Threaded as skillsOverride to the direct spawn path + (params.skills ?? o.skills) to the lifecycle spawn adapter so a caller can override a lifecycle phase's bundle. Added a promptGuidelines note.

Blast radius (challenge-step verified)

Path Before After
general-purpose / general-purpose-cc direct dispatches all 42 skills (~570K) 0 skills (lean) — opt in via skills
default lifecycle phases (all declare explicit skill bundles) filter to declared unchanged (truthy branch)
verify.ts gate reviewer (passed skills:[]) all 42 (old else-branch bug) 0 — bonus lean-up, no regression

No silent skill loss: lifecycle phases declare skills explicitly, so they hit the truthy branch.

Tests (+6)

  • test/child-loader.test.mtsresolveChildSkills: no-skills → []; specific-skills → filtered; explicit-empty → []; unknown names dropped gracefully.
  • test/subagent-tool.test.mtsskills param threads to the cloned child agent.skills; absent → stays undefined (lean).

Verification

  • pnpm typecheck — clean
  • pnpm test:run621/621 pass (was 615; +6 new)

Not in this PR

The other #32 directions — memory-hydration trim, system-prompt compaction, widget work-progress UX (overlaps #23) — remain tracked in #32. The actual substrate-token reduction will be confirmed via the release-gate smoke (real pi dispatch) at tag time; this PR proves the skill-loading logic only.

Addresses #32 (the skill-loading root cause). The remaining #32 optimization directions stay open.

rz1989s added 2 commits August 6, 2026 06:32
…param (#32)

Root cause (#32): buildChildLoader's skillsOverride loaded ALL installed skills
(~42 → ~570K tokens, ~59% of a 976K context window from turn 1) when an agent
declared no 'skills' field. Both builtin agents (general-purpose, general-
purpose-cc) declare none → every dispatch paid the full substrate cost before
doing any work.

Fix (default (a) — lean, caller opts in):
- src/engine/child-loader.ts: extract resolveChildSkills() (pure, testable);
  the no-skills branch now returns [] (not cur.skills). An agent that declares
  specific skills still loads only those (truthy branch unchanged).
- src/tools/subagent.ts: add a 'skills' param (Type.Array(Type.String())) —
  callers opt in to specific skills. Threaded as skillsOverride to the direct
  spawn path + (params.skills ?? o.skills) to the lifecycle spawn adapter so a
  caller can override a lifecycle phase's bundle.

Blast radius (challenge-step verified):
- general-purpose / general-purpose-cc direct dispatches → now load 0 skills
  (lean substrate). Callers pass 'skills' to opt in.
- default lifecycle phases all declare explicit skill bundles → unaffected
  (truthy branch, filters to declared skills).
- verify.ts gate reviewer passed skills:[] → was also loading all 42 (the
  old else-branch); now correctly loads 0. Bonus lean-up, no regression.

Tests (+6): child-loader.test.mts — resolveChildSkills for no-skills → [],
specific-skills filter, explicit-empty → [], unknown-names dropped. subagent-
tool.test.mts — skills param threads to child agent.skills; absent → undefined.

621/621 pass, typecheck clean. Substrate reduction to be confirmed via the
release-gate smoke (real pi dispatch) at tag time; the other #32 directions
(memory-hydration trim, prompt compaction, widget UX) remain tracked in #32.
…de, EOF newlines)

Review findings addressed (APPROVE w/ nits):

- src/tools/subagent.ts: lifecycle spawn adapter switched from
  'params.skills ?? o.skills' (REPLACE — a caller passing skills:["tdd"] with
  lifecycle:"default" would strip 'brainstorming' from the brainstorm phase)
  to mergeLifecycleSkills(o.skills, params.skills) — ADDITIVE + deduped. The
  phase's designed skills always load; a caller can add extras but cannot
  strip phase skills. Extracted mergeLifecycleSkills() as a pure testable helper.
- src/tools/subagent.ts: clarified the skills param description — direct
  dispatch replaces frontmatter (pass [] for zero); lifecycle dispatch is
  additive. Fixes the description-accuracy nit.
- src/engine/child-loader.ts + src/tools/subagent.ts: added EOF newlines
  (AGENTS.md convention; the two source files were missing them).

Locked in the [] override behavior (a reviewer worried skills:[] was silently
ignored — it isn't: [] is truthy, the clone happens, resolveChildSkills
returns []). Added a test for it on an agent WITH frontmatter skills.

Tests (+2): skills:[] overrides frontmatter to zero (direct);
mergeLifecycleSkills additive + deduped (5 cases).
@rz1989s
rz1989s merged commit 9d0d3d8 into main Aug 5, 2026
1 check passed
@rz1989s
rz1989s deleted the fix/issue-32-lean-substrate-no-skills-default branch August 5, 2026 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant