Conversation
A subagent spawn fails before its first prompt whenever the parent session
holds a tool that a package activates on demand:
Child tool preflight failed: requested tool(s) "web_search", ... are
unavailable after child extensions initialized.
inheritedChildToolAllowlist() projects the parent's live active-tool
surface into the child's requested allowlist, but bindChildSessionExtensions()
only activates the hardcoded names in CHILD_SAFE_PACKAGE_TOOL_NAMES
(fd, rg, git_show, git_diff, git_log). Every other inherited name must
already be active in the fresh child session, which is false for tools
activated at runtime:
- pi-web-access with toolActivation "auto" starts a session with only
web_enable and activates web_search and friends after the model
calls it
- pi-agent-browser-native adds agent_browser_action/_qa/... only after
agent_browser_tools enables them
- MCP deferred exposure brings in tool_search; codemode exposure
brings in codemode
Activate every requested name the child has registered instead. requested
is already the narrowing allowlist (parent surface minus
CHILD_EXCLUDED_TOOL_NAMES), so this cannot widen the child's boundary,
and it removes the dependency on a list that can never stay in sync with
what users install.
A name the child cannot expose at all no longer aborts the spawn either.
requested is inherited from the parent surface, so a narrowed child is
not a configuration error worth failing for; report it once with counts
and continue.
Fixes openpi-dev#637
…lists Two independent defects made a subagent spawn fail, and the first fix for one of them relaxed a contract the preflight exists for. 1. The inherited-tool report relaxed the check for every caller, so an explicit allowlist naming a tool the child cannot expose no longer failed. Three tests asserted that. Make the relaxation opt-in: bindChildSessionExtensions() keeps the hard failure by default and only reports when the caller passes tolerateInheritedMisses, which the two callers that project the parent's live surface now do (direct spawn and the workflow runner, through a new inheritedTools flag on SpawnTask / RunAgentOptions). 2. tool_search and codemode were never registered in a child, not merely inactive. Pi's built-in extensions (codemode, tool-search, mcp, llama.cpp) are builtin:true inline extensions whose code reaches a loader only through extensionFactories: the CLI's main() supplies them, an SDK caller has to. Inject tool-search and codemode, the two the parent activates; mcp stays out because a user's own MCP extension replaces it, and llama.cpp only serves local models. 3. The replay fingerprint realpath'd every extension, so injecting a built-in extension (a synthetic `builtin:name` path with no file behind it) threw and silently disabled journaling for every workflow agent call. Skip synthetic paths there; their code comes from the host pi version, not from project resources, which is what the fingerprint binds. Tests: extension realpath helpers and the empty-package-filter assertion now ignore synthetic extension paths. Verified on pi 0.99.1 with bun 1.3.14: the three preflight tests pass, the extension/shared, subagents, and workflows suites are at baseline (2 unrelated worktree-cancellation timeouts), 1120 vitest cases pass, and typecheck, biome format, lint, and the config/docs/discipline contract checks are clean.
56c2b8e to
b086e13
Compare
The web workbench builds its own session services and passed only its two custom factories (command discovery, turn-change recording). Like the subagent child loader, that left the session with no built-in registry: `builtin:tool-search`, `builtin:codemode`, and `builtin:mcp` resolved to "Unknown built-in extension", so `tool_search`, `codemode`, and MCP tools were never registered for a web session either. Move the factory list into extensions/shared/pi-builtin-extensions.ts and use it from both call sites, so a fix to the built-in set lands in both. Verified on pi 0.99.1: a loader built with the shared helper registers `builtin:tool-search`, `builtin:codemode`, and `builtin:mcp` with no errors, and the web/child suites are unaffected.
tt-a1i
left a comment
There was a problem hiding this comment.
Reviewed the complete base-to-head change at e8c43cb, including an independent subagent correctness review. No remaining findings after fixing explicit-role tolerance, synthetic replay identities, and the independent review's rejected-activation finding.
The preflight now checks Pi's actual active tool surface, while child tool registration and nested execution remain within the parent-derived policy. Reviewed native factories bind replay identity to their enabled names and the executing Pi version; unknown inline identities disable replay.
Local validation passed: bun run check; bun run test (2,130 Node tests passed, 8 skipped; 1,120 Vitest tests passed). Independent child-session and replay-safety suites: 41 passed, 0 failed. The hidden-tool regression failed before the activation readback repair and passed afterward. Current-head remote CI and branch protection still need to clear before merge. External MCP servers and remote-provider/manual installed-runtime behavior were not validated in this review.
Problem
Fixes #637. A parent's live tool surface can contain runtime-activated tools that a fresh SDK child has not activated or cannot register. The previous child preflight rejected these inherited misses, making ordinary subagent spawns fail. SDK children and the Web runtime also lacked Pi's native extension factories.
The initial patch introduced two additional gaps: the public Direct and Workflow entries tolerated misses even for explicitly declared role tools, and replay fingerprints ignored all synthetic extension identities. Independent review also found that a tool Pi refuses to activate could incorrectly satisfy preflight.
Value
Restore ordinary child execution with Pi's current tool surface while keeping explicit role requirements enforceable before the child's first prompt. Preserve replay safety when native and inline extensions are loaded.
Approach
toolslist.tool-search,codemode, andmcpfactories between child resource loaders and the Web runtime. Keep native replacement behavior and the existing child tool policy.hiddenactivation coverage, and real SDK loader identity regressions. Preserve the investigation in the source-scoped record.Validation
Candidate:
e8c43cb1eb264a790e57954e4d4cf36db17faf72; locked Pi SDK 0.99.1, Bun 1.3.14, Node 26.8.1 on macOS.bun run check: passed, including production Web build, configuration/documentation checks, formatting, lint, and TypeScript.bun run test: passed; 2,130 Node tests passed, 8 skipped; 1,120 Vitest tests passed.Impact