From 31359d8d5450f9ed0cda8450964ba1478849679c Mon Sep 17 00:00:00 2001 From: naivezip <253203888+naivezip@users.noreply.github.com> Date: Tue, 8 Sep 2026 20:24:49 +0800 Subject: [PATCH] fix(child-session): dynamically exclude blocked package tools from child inheritance (#483) pi-intercom is excluded from child sessions via blockedPackageSources to prevent concurrent process.env session cross-wiring (#128). However, when pi-intercom was active in the parent session, inheritedChildToolAllowlist projected its tools into child sessions, causing subagent_spawn and workflow child sessions to fail preflight checks. Rather than hardcoding foreign tool names into OpenPI's CHILD_EXCLUDED_TOOL_NAMES (which is strictly reserved for OpenPI-owned parent-only tools and enforced by the fail-closed drift guard), dynamically drop tools from blocked packages during child inheritance: - Inspect tool.sourceInfo in inheritedChildToolAllowlist against blocked child package matchers - Pass pi.getAllTools() and cwd context from subagents/index.ts and workflows/index.ts - Preserve CHILD_EXCLUDED_TOOL_NAMES and bidirectional drift guards strictly for OpenPI tools - Add regression coverage verifying dynamic exclusion of tools from blocked packages while preserving ordinary tools and passing child preflight --- extensions/shared/child-session.ts | 98 ++++++++++++++++++- extensions/subagents/index.ts | 4 + extensions/workflows/index.ts | 4 + tests/extensions/shared/child-session.test.ts | 78 +++++++++++++++ 4 files changed, 183 insertions(+), 1 deletion(-) diff --git a/extensions/shared/child-session.ts b/extensions/shared/child-session.ts index 61a9d3ce..80e060fb 100644 --- a/extensions/shared/child-session.ts +++ b/extensions/shared/child-session.ts @@ -550,17 +550,113 @@ export function effectiveChildToolAllowlist(tools?: readonly string[]) { ); } +export interface ChildToolInheritanceOptions { + availableTools?: readonly { + name: string; + sourceInfo?: { + path?: string; + source?: string; + baseDir?: string; + scope?: string; + origin?: string; + }; + }[]; + cwd?: string; + agentDir?: string; +} + +/** + * Checks whether a tool originates from a package that is blocked from child sessions. + * Currently, pi-intercom packages are blocked (via blockedPackageSources) to avoid + * process.env session cross-wiring in concurrent child sessions (#128). + */ +export function isBlockedChildTool( + tool: { + name: string; + sourceInfo?: { + path?: string; + source?: string; + baseDir?: string; + scope?: string; + origin?: string; + }; + }, + options: { cwd?: string; agentDir?: string } = {}, +) { + if (!tool.sourceInfo) return false; + const { source, baseDir, path: toolFilePath } = tool.sourceInfo; + if ( + source === "builtin" || + source === "sdk" || + (toolFilePath && toolFilePath.startsWith("<")) + ) { + return false; + } + const isPiIntercomPackage = createPiIntercomPackageMatcher({ + cwd: options.cwd ?? process.cwd(), + agentDir: options.agentDir ?? getAgentDir(), + }); + const candidatePath = baseDir ?? toolFilePath; + try { + return isPiIntercomPackage(source ?? "", candidatePath); + } catch { + if ( + source && + (source === "npm:pi-intercom" || source.includes("pi-intercom")) + ) { + return true; + } + if (candidatePath && candidatePath.includes("pi-intercom")) { + return true; + } + return false; + } +} + /** Project the parent's active surface into a child; a role can only narrow it. * Active tools are a visibility choice, not a filesystem/network sandbox. * Inactive tools are not implicitly activated by delegation. + * Tools registered by packages that are blocked from child sessions (e.g. pi-intercom) + * are dynamically dropped during inheritance when availableTools metadata is provided. */ export function inheritedChildToolAllowlist( parentTools: readonly string[], roleTools?: readonly string[], + options?: + | ChildToolInheritanceOptions + | readonly { + name: string; + sourceInfo?: { + path?: string; + source?: string; + baseDir?: string; + scope?: string; + origin?: string; + }; + }[], ) { const allowed = roleTools === undefined ? undefined : new Set(roleTools); + const optionsObj: ChildToolInheritanceOptions = Array.isArray(options) + ? { availableTools: options } + : (options ?? {}); + + const blockedTools = new Set(); + if (optionsObj.availableTools) { + for (const tool of optionsObj.availableTools) { + if ( + isBlockedChildTool(tool, { + cwd: optionsObj.cwd, + agentDir: optionsObj.agentDir, + }) + ) { + blockedTools.add(tool.name); + } + } + } + return effectiveChildToolAllowlist([...new Set(parentTools)])!.filter( - (name) => allowed === undefined || allowed.has(name), + (name) => + !blockedTools.has(name) && (allowed === undefined || allowed.has(name)), ); } diff --git a/extensions/subagents/index.ts b/extensions/subagents/index.ts index f753c77b..e12d1779 100644 --- a/extensions/subagents/index.ts +++ b/extensions/subagents/index.ts @@ -940,6 +940,10 @@ export default function ( const childTools = inheritedChildToolAllowlist( pi.getActiveTools(), requestedChildTools, + { + availableTools: pi.getAllTools?.(), + cwd: ctx.cwd, + }, ); // Read at spawn time so `/openpi-setup` changes affect the next child // without reloading this extension. Undefined preserves parent-model diff --git a/extensions/workflows/index.ts b/extensions/workflows/index.ts index ff0f5a9c..c510cdf0 100644 --- a/extensions/workflows/index.ts +++ b/extensions/workflows/index.ts @@ -1647,6 +1647,10 @@ export default function workflows( const childTools = inheritedChildToolAllowlist( pi.getActiveTools(), agentType?.tools, + { + availableTools: pi.getAllTools?.(), + cwd: ctx.cwd, + }, ); if ( opts.working_dir !== undefined && diff --git a/tests/extensions/shared/child-session.test.ts b/tests/extensions/shared/child-session.test.ts index ff49e6c6..a2cff38f 100644 --- a/tests/extensions/shared/child-session.test.ts +++ b/tests/extensions/shared/child-session.test.ts @@ -1575,3 +1575,81 @@ test("child delegation inherits active tools and custom restrictions only narrow assert.deepEqual(inheritedChildToolAllowlist(parent, []), []); assert.deepEqual(inheritedChildToolAllowlist([], ["bash"]), []); }); + +test("tools from blocked packages like pi-intercom are dynamically excluded from child allowlist and pass preflight", async () => { + // Simulate a parent session where pi-intercom is active alongside native tools and third-party tools + const parentActiveTools = [ + "read", + "bash", + "edit", + "intercom", + "intercom_git", + "weather", + ]; + const availableTools = [ + { name: "read", sourceInfo: { source: "builtin" } }, + { name: "bash", sourceInfo: { source: "builtin" } }, + { name: "edit", sourceInfo: { source: "builtin" } }, + { name: "weather", sourceInfo: { source: "npm:pi-weather" } }, + { name: "intercom", sourceInfo: { source: "npm:pi-intercom" } }, + { + name: "intercom_git", + sourceInfo: { + source: "git:https://github.com/nicobailon/pi-intercom", + }, + }, + ]; + + // 1. CHILD_EXCLUDED_TOOL_NAMES must NOT include community tool names + assert.equal( + (CHILD_EXCLUDED_TOOL_NAMES as readonly string[]).includes("intercom"), + false, + "CHILD_EXCLUDED_TOOL_NAMES must remain strictly for OpenPI package tools", + ); + + // 2. Inherited allowlist dynamically drops tools from blocked packages + const inherited = inheritedChildToolAllowlist(parentActiveTools, undefined, { + availableTools, + }); + assert.deepEqual(inherited, ["read", "bash", "edit", "weather"]); + assert.equal(inherited.includes("intercom"), false); + assert.equal(inherited.includes("intercom_git"), false); + assert.equal(inherited.includes("weather"), true); + + // 3. An explicit role allowlist naming a blocked package tool must also drop it + const explicitNarrowed = inheritedChildToolAllowlist( + parentActiveTools, + ["read", "intercom", "weather"], + { availableTools }, + ); + assert.deepEqual(explicitNarrowed, ["read", "weather"]); + + // 4. Array shorthand for options works identically + const arrayShorthand = inheritedChildToolAllowlist( + parentActiveTools, + undefined, + availableTools, + ); + assert.deepEqual(arrayShorthand, ["read", "bash", "edit", "weather"]); + + // 5. Child tool policy constructed from inherited tools has no blocked tools + const policy = childToolPolicy(inherited); + assert.equal(policy.tools?.includes("intercom"), false); + + // 6. bindChildSessionExtensions preflight must pass with the sanitized inherited allowlist + const mockChildSession = { + async bindExtensions() {}, + getActiveToolNames: () => ["read", "bash", "edit", "weather"], + getAllTools: () => [ + { name: "read" }, + { name: "bash" }, + { name: "edit" }, + { name: "weather" }, + ], + setActiveToolsByName(_names: string[]) {}, + }; + + await assert.doesNotReject( + bindChildSessionExtensions(mockChildSession, inherited), + ); +});