fix(tools): let subagents use asynchronously-registered extension tools, including under ext: - #157
Merged
Conversation
Subagents could not use tools provided by MCP extensions (e.g. pi-mcp-extension), even though those tools work in the main agent. pi-mcp registers its tools asynchronously from its session_start handler, after connecting to MCP servers — never during synchronous extension load. runAgent, however, snapshotted extension.tools.keys() into a static `tools:` allowlist right after loader.reload() and before bindExtensions() fires session_start. In pi-mono that allowlist (allowedToolNames) is frozen at construction and gates the tool *registry* itself, so MCP tools — registered later — were dropped permanently, not just deactivated. Scope extension tools via the session's live denylist instead of a frozen allowlist, matching how the main agent stays in sync. When extensions load and there is no `ext:` narrowing, pass `tools: undefined` (allowlist unset, so the live isAllowedTool gate admits tools whenever they register) and express scope as `excludeTools` = this extension's own orchestration tools + built-ins the agent didn't ask for + disallowedTools. Because an unset allowlist only activates pi's four default built-ins at turn 1, repair the active set post-bind via setActiveToolsByName(getAllTools()); tools registered later (lazy MCP servers) auto-activate through pi's refresh path. The static-allowlist path is retained unchanged for noExtensions/isolated and for `ext:` narrowing, where the exact set is known up front. May solve issue #125. I (the human) tested this manually by setting my local checkout of pi-subagents as the extension, then asking the LLM to spawn a subagent and use a search tool. Previously, it couldn't -- and now it can. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… tools Enforce ext: on the active set via a live predicate instead of a frozen allowlist, matching pi's own dynamic-tool-loading idiom. Guard turn 1 by wrapping beforeToolCall, since before_agent_start widens the tool set after that turn's tools are snapshotted. Add a pi@latest reachability guard for that hook. Fixes #125. Refs #141.
tintinweb
marked this pull request as ready for review
July 22, 2026 17:33
tintinweb
added a commit
to codesoda/pi-subagents
that referenced
this pull request
Jul 22, 2026
…ed result wait abortable
tintinweb
added a commit
that referenced
this pull request
Jul 31, 2026
* feat: add safe nested subagent delegation * fix(nested): reconcile scoping with #157 denylist; make nested result wait abortable * feat(nested): complete and harden opt-in nested subagent delegation Rounds out the delegation added in 7cdb119 so a nested run is as governed, as honest, and as auditable as a top-level one. Config surface: `allowed_subagents` and `max_subagent_depth` frontmatter, `maxSubagentDepth` in subagents.json plus an `/agents → Settings` row, and a per-agent cap that can only tighten the inherited one. `allowed_subagents` accepts booleans the way `extensions:`/`skills:` do — without that, YAML `true` stringified into an agent type literally named "true", so the tools appeared and every spawn was refused. Tools are no longer injected once an agent already sits at the cap, which is what makes depth 0/1 mean "nesting off" rather than "nesting always fails". Isolation: nested spawns resolve agents from a registry built for their own config root (buildAgentRegistry and the *In variants) instead of mutating the process-global one, and the scopeModels policy moves to model-scope.ts so both entry points enforce the same allowlist. Ownership metadata (parentAgentId, depth, configCwd, rootSessionId) is stripped from cross-extension spawns; rootSessionId names a transcript directory, so a forged value would be a path-traversal primitive. Lifecycle: children are stopped when their parent settles *or* ends a resumed turn — resume previously left them running, hidden from every surface and reachable by no one. Nested children stay out of the maxConcurrent pool (their parent already holds a slot, and queueing them behind it would deadlock a parent waiting on its own child), and spawnAndWait restores the shared onSpawned hook before awaiting, so a concurrent spawn can no longer inherit a foreground caller's callback. Honesty and accounting: a nested result ending stopped/aborted/steered now leads with the same partial-output warning top-level results carry, and a failed one keeps its partial output (partialOutputSuffix moves to status-note.ts for both callers). Token usage folds into every ancestor's totals, and children write their own .output transcript under the root session's directory behind the same output_transcript gate — so nested work stays inspectable and its spend attributable at any depth. Spawn failures (strict worktree isolation, cwd validation) return as tool errors instead of escaping the child's turn. * test(nested): pin both result wordings and cover delegation end-to-end nested-tools tests now assert each side of the #174 split: a foreground result says everything is above (the parent has no id to fetch with), a get_subagent_result poll keeps the "may be incomplete" wording (it does). The two paths can no longer quietly converge. nested-delegation-e2e is the first end-to-end coverage of nesting at all. Everything else stops short of a real session — the tool tests use a fake manager, the runner tests a mocked pi — leaving the load-bearing integration facts unproven: that pi admits the injected customTools into an opted-in child's ACTIVE tool set despite their names colliding with the ones stripped from every subagent, and that a grandchild's output travels back up two hops. It drives a real loader, a real extension, real runAgent, and two real child sessions on a faux model, and asserts the leaf that never opted in does not receive the tools. Faux by design: a live model declining to delegate would look like a pass. * test(nested): cover the background nested path end-to-end * feat(nested)!: defer the per-agent max_subagent_depth cap --------- Co-authored-by: tintinweb <tintinweb@oststrom.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Subagents silently lost every extension tool that registers asynchronously — after extension load. This is a category, not one broken extension:
pi-mcpregisters fromsession_start(once its MCP servers connect),context-modefrombefore_agent_start. Both do that deliberately, because eagerly spawning an MCP bridge during extension discovery orphans child processes on pi's non-agent code paths (--help, config, trust probing).Builds on #141 by @philipmw (cherry-picked). This adds the half that #141 explicitly left open:
ext:selectors.Fixes #125.
Root cause
Verified against pi-coding-agent 0.80.10,
agent-session.js:1943:allowedToolNamesgates tool registration, not merely the active set, and it's frozen at construction.runAgentsnapshottedextension.tools.keys()into that allowlist right afterloader.reload()— i.e. beforebindExtensions()firessession_start. Anything registering later was dropped permanently.Approach
Whenever extensions are in play, leave
allowedToolNamesunset so pi's live gate admits tools whenever they register, and express the name-stable permanent scope (our own orchestration tools, built-ins the agent didn't ask for,disallowed_tools:) asexcludeTools, which pi re-applies on every registry refresh.extensions: false/isolated: truekeep the static allowlist — nothing can register asynchronously there, and a hard registry gate is the right boundary for a sandbox.ext:narrowing then can't be a registry allowlist, because you can't list a tool that doesn't exist yet. Ite** set instead — what the model actually sees — via a predicate re-derived from the loader's live extensionmaps, the same mapsregisterToolwrites into.Two enforcement points, because neither covers the whole picture:
turn_endre-narrows the active set. pi emitsturn_endimmediately beforeprepareNextTurnre-snapshotsagent.state.tools, and session listeners run synchronously, so the narrow lands in time for turns 2..N.beforeToolCallvetoes out-of-scope calls. Turn 1 can't be narrowed at all:before_agent_startfiresay widen the tool set, butcreateContextSnapshot()freezes that turn's tools immediately after — there's nowindow. A call-time check is the only correct guard there.Both are installed on the session and deliberately not torn down, so resumed and steered turns stay scoped. pi's
dispose()clears_eventListeners, so they die with the session.resumeAgentandagent-manager.tsneeded no changes.This collapses #141's dual mode into a single path and removes the comment explaining why there were two. Net: 78 lines of code.
Alignment with upstream
pi 0.80.9 added Dynamic Tool Loading — register every tool, keep a subset active, call
setActiveToolsas things become relevant. That's exactly the shape used here, arrived at independently for adifferent use case, which is good evidence this is the pi-idiomatic approach rather than a workaround. The
{likewise pi's sanctioned shape (pi.on("tool_call")in the extensions Quick Start); pi turns it into an errortool result atagent-loop.js:419`.One thing genuinely reaches past the documented surface:
ExtensionBindingshas notool_callhook, so an SDK caller constructing a child session has no way to inject a veto. We wrap thebeforeToolCallproperty pi installs in theAgentSessionconstructor, chaining to the prior hook so extensiontool_callhandlers still fire. Verifiedsigned once (:136),_installAgentToolHooks()runs once in the constructor (:153) and is never re-run by_buildRuntime/reload().Because unit tests assert that against a hand-written mock and so can't catch upstream drift,
test/e2e/tool-veto-reachability.e2e.test.tsguards it against a real session, wired into the existingcompat-latest-pijob — following the precedentset by #152's
modelRuntimeguard.Evidence
New e2e fixture
ext-lazy.mjsregisters fromsession_start, reproducing the exact timing. Run against eachext:ext:(#125's config)ext:stays mutedMeasured, not asserted — the middle row is why #141 alone doesn't close #125.
Testing
effectiveAllowedTools()mock helper was replaced: it reimplemented pi'sisAllowedTool, so assertions routed through it tested the helper rather than pi.lastToolsPassed()now returns the session's real active set.ext:opt-in flip fails 7 tests (3 new, 4 pre-existing); deleting the veto fails the reachability guard.Behavior changes
ext:tracks late registration both ways — selecting a lazy extension surfaces its tools; leaving one out keeps it muted whenever it registers.ext:in use, an unselected extension registering atbefore_agent_start.Checked and unchanged: out-of-scope tools now sit in pi's registry, but
_rebuildSystemPromptwalks only active names, so they never reach the prompt; memory tools are folded intotoolNamesbefore scoping, somemory:agents keep read/write.