Skip to content

feat(subagent): honor subagent_def at runtime — load def.body + def.allowed_tools#1282

Open
dcarolan1 wants to merge 1 commit into
garrytan:masterfrom
dcarolan1:feature/subagent-def-runtime-contract
Open

feat(subagent): honor subagent_def at runtime — load def.body + def.allowed_tools#1282
dcarolan1 wants to merge 1 commit into
garrytan:masterfrom
dcarolan1:feature/subagent-def-runtime-contract

Conversation

@dcarolan1
Copy link
Copy Markdown

Summary

Wires data.subagent_def to be load-bearing at job-dispatch time: when set AND the named def exists in the plugin registry, def.body becomes the default system prompt and def.allowed_tools becomes the default allowed tool set. Both overridable by explicit data.system / data.allowed_tools. Backward compatible.

Motivation

The plugin-loader at src/core/minions/plugin-loader.ts does substantive work at worker startup:

  • Discovers gbrain.plugin.json manifests under each GBRAIN_PLUGIN_PATH entry
  • Validates each subagent def's allowed_tools is a subset of the brain-allowlist (fails loudly on typos)
  • Resolves left-wins collisions across multiple plugins
  • Logs [plugin-loader] loaded 'plugin-name' v1.0.0 (N subagents) per discovered plugin

But the loaded SubagentDefinition[] data was never consumed at runtime: the subagent handler (src/core/minions/handlers/subagent.ts:170) reads data.system ?? DEFAULT_SYSTEM and data.allowed_tools ?? registry directly from job data, with no fallback to whatever the named def specified. The data.subagent_def field was set + persisted but never read.

Practical impact: callers had to re-embed the entire system body + allowed_tools list in every queue.add('subagent', { ... }) submission to get the def's behavior. For a 4,583-byte subagent body, that's ~4.5 KB per job in minion_jobs.data plus the per-call complexity on the calling side.

Discovered while building a BAG-side subagent (aaron-nl-to-shell-ctx) under an internal stay-close-to-gbrain doctrine — the caller code in our TG dispatcher (Moses) was re-embedding the full subagent body on every submission, defeating the plugin system's stated purpose of letting operators declare reusable subagent contracts.

Behavior

When data.subagent_def is set AND a matching def exists in the plugin registry (populated at worker startup by loadPluginsFromEnv):

Field Resolution order
systemPrompt data.systemdef.body (when non-empty) → DEFAULT_SYSTEM
allowed_tools data.allowed_toolsdef.allowed_tools → full registry

Override semantics: data fields ALWAYS win when explicitly provided. Callers retain full control; the def is a convenience default.

Fail-open behavior (handler never throws on these):

  • data.subagent_def set but def not in registry (e.g., worker has a stale plugin path while caller has a newer one) → warn to stderr + fall through to current behavior
  • def.body empty or whitespace-only → treat as no-body, fall through to DEFAULT_SYSTEM
  • data.subagent_def absent → behavior unchanged (backward compatible)

Required infrastructure: register the registry

To enable lookup, loadPluginsFromEnv now ALSO populates a module-level _subagentRegistry: Map<string, SubagentDefinition> alongside its existing log output. Lookup-only public API:

export function getSubagentDef(name: string): SubagentDefinition | undefined

Registry lives for the worker process lifetime. Left-wins collision policy (already enforced by plugin-loader's existing subagentByName Map) is honored on registration.

Testing

New file test/subagent-def-runtime-contract.test.ts7 test cases, all PASS in 2.72s:

# Case
1 def.body becomes system prompt when subagent_def set + no data.system override
2 data.system overrides def.body (data wins)
3 def.allowed_tools used when data.allowed_tools absent
4 data.allowed_tools fully overrides def.allowed_tools (NOT intersection — data wins fully)
5 subagent_def names unknown def → warn + fall through to DEFAULT_SYSTEM + full registry (fail-open)
6 subagent_def absent → existing behavior unchanged (backward compat)
7 def.body empty/whitespace-only → treated as no-body, falls to DEFAULT_SYSTEM

Tests use the same pattern as existing test/subagent-handler.test.ts (PGLite in-memory + FakeMessagesClient capturing params.system and params.tools from the Messages API call). Registry seeding via a new __testing._seedSubagentDef(def) + __testing._clearSubagentRegistry() surface so tests don't need on-disk plugin fixtures.

bun run test: 7967 pass / 15 fail — 15 failures are the same pre-existing baseline (skillpack-check, check-resolvable, BrainRegistry, ConnectionManager, apply-migrations). Zero failures in plugin-loader / subagent / handler paths.

Backward Compatibility

Purely additive surface. Existing callers without data.subagent_def see no behavior change. The MinionJobContext type, the SubagentHandlerData shape, and the SubagentDefinition shape all unchanged — only the handler's resolution logic + the addition of the registry export.

Out of Scope

  • Plugin hot-reload. Registry lives for the worker process lifetime; removed plugins linger until restart. Acceptable for v1; hot-reload deserves its own design discussion.
  • Per-job def versioning. A future enhancement could pin data.subagent_def_version; this PR doesn't add it.
  • Intersection semantics on allowed_tools. This PR chose "data wins fully" over "intersect" — happy to flip if you prefer; let me know.

Notes

This is the second BAG → gbrain contribution under our internal stay-close-to-gbrain doctrine. First PR (queued separately) added --poll-interval flag to gbrain jobs work. Both came out of measurement work for a multi-host agent fleet that uses gbrain's MinionQueue + subagent + plugin systems heavily.

Happy to adjust per repo conventions — the Co-Authored-By: Claude trailer is on the commit but easily droppable, intersection-vs-override semantics easily swappable, CHANGELOG entry shape easily adjustable.

🤖 Generated with Claude Code — happy to adjust attribution as preferred.

…llowed_tools

The plugin-loader validates subagent def manifests at worker startup
(name, body, allowed_tools, fail-loud on unknown tools) and prints a
discovery banner, but the loaded plugins array was then thrown away.
The subagent handler had no registry to consume from, so it read
data.system (defaulting to DEFAULT_SYSTEM = "You are a helpful
assistant running as a gbrain subagent") and data.allowed_tools
(defaulting to full registry) directly from job data, completely
ignoring the def the caller named via data.subagent_def.

This forced callers to embed the full system body + tool list in
every job submission, defeating the plugin system's stated purpose
of letting operators declare reusable subagent contracts.

This PR closes the gap with two coordinated changes:

1. plugin-loader.ts establishes a module-level subagent registry
   (_subagentRegistry Map) populated by loadPluginsFromEnv alongside
   its existing log output. Lookup-only public API:
   `getSubagentDef(name): SubagentDefinition | undefined`. Lives for
   the worker process lifetime; left-wins collision policy already
   resolved by plugin-loader is honored.

2. subagent.ts handler resolves data.subagent_def → registered def
   at job-dispatch and uses it as the default source for system prompt
   + allowed_tools:
   - data.system → def.body → DEFAULT_SYSTEM
   - data.allowed_tools → def.allowed_tools → full registry
   Override semantics: data fields ALWAYS win when explicitly provided.

Failure modes (all fail-open, never throw):
- data.subagent_def names a def NOT in registry (worker has a stale
  plugin path): warn and fall through to current behavior
- def.body is empty or whitespace-only: treat as no-body, fall to
  DEFAULT_SYSTEM
- data.subagent_def absent: behavior unchanged (backward compatible)

Tests in test/subagent-def-runtime-contract.test.ts cover all four
resolution paths plus the three edge cases above. Inspection via
FakeMessagesClient.calls[0].system and .tools — exact pattern from
test/subagent-handler.test.ts. Registry seeding through plugin-loader's
__testing surface so tests don't need on-disk plugin fixtures.

Discovered while building a BAG-side subagent ("aaron-nl-to-shell-ctx")
under a stay-close-to-gbrain doctrine — caller had to re-embed the
4583-byte subagent body in every Moses-side queue.add submission for
the def to take effect.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
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