diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 3cf1d5b8e..e32a536c5 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -19,6 +19,7 @@ - Fixed assistant Markdown headings and fenced code blocks appearing with literal source delimiters in the interactive transcript. - Fixed extension TUI method wrappers recursing indefinitely when delegating to the original method ([#7731](https://github.com/earendil-works/pi/issues/7731)). - Fixed right-click not pasting clipboard text in fullscreen mode on Windows. +- Fixed sub-agent spawning bypassing execution-mutating parent extension hooks by rejecting incompatible parent policies before child creation. ## [0.84.0] - 2026-08-06 diff --git a/packages/coding-agent/docs/sdk.md b/packages/coding-agent/docs/sdk.md index e02a2d64f..782c1c550 100644 --- a/packages/coding-agent/docs/sdk.md +++ b/packages/coding-agent/docs/sdk.md @@ -539,6 +539,8 @@ Root sessions run at most four children concurrently by default. Set `subagents: { maxConcurrent: number }` to configure that limit, or `subagents: false` to disable child agents and omit all three coordination tools. The names `spawn_agent`, `agent_action`, `agents`, and `escalate_to_parent` are reserved and cannot be supplied through `customTools`. +Child sessions intentionally do not load parent extensions. To prevent an extension policy bypass, `spawn_agent` fails closed when the root has a registered handler for any execution-mutating hook: `resources_discover`, `input`, `context`, `before_agent_start`, `before_provider_request`, `before_provider_headers`, `session_before_compact`, `tool_call`, `tool_result`, or `message_end`. The error lists every incompatible hook in that order. Remove those handlers before spawning, or set `subagents: false`; observation-only hooks and non-spawn child actions remain available. + Each live root-owned child also receives the child-only `escalate_to_parent` tool; roots, standalone sessions, and restored children that have not been explicitly woken do not receive it. It accepts one non-empty message of at most 1,000 characters for material information that could redirect or interrupt the parent, and the child must still return a complete final response. Multiple calls are allowed during one live run, subject to a fixed 8,000-character aggregate limit for accepted report text not yet admitted to parent context. diff --git a/packages/coding-agent/docs/usage.md b/packages/coding-agent/docs/usage.md index d1af7d900..cb9a69eb1 100644 --- a/packages/coding-agent/docs/usage.md +++ b/packages/coding-agent/docs/usage.md @@ -70,6 +70,8 @@ It displays the current in-memory roster without adding the command or roster to The enclosing parent prompt remains active after the parent attempts to finish. Diffex freezes the children started by that prompt, waits until all of them finish, fail, or are interrupted, and displays a deterministic notice as each child terminates. +Children do not load parent extensions. To prevent them from bypassing parent execution policy, spawning fails before child creation when the parent registers any of these hooks: `resources_discover`, `input`, `context`, `before_agent_start`, `before_provider_request`, `before_provider_headers`, `session_before_compact`, `tool_call`, `tool_result`, or `message_end`. The rejection lists the incompatible hooks in that order. Remove those handlers to spawn children; observation-only hooks and existing-child message, interrupt, and status actions remain available. + ### Child Escalations A running root-owned child can use its child-only `escalate_to_parent` tool to report a material blocker, risk, contradiction, or decision that could change the parent's plan. diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index ec4a13be9..790628a26 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -408,6 +408,18 @@ const CURRENT_PLAN_RESTORATION_CUSTOM_TYPE = "diffex.current-plan-restoration"; const CURRENT_PLAN_RESTORATION_REMINDER = "Reminder: restored context is not proof of completion. Verify current state before claiming work is done."; const MAX_PENDING_SUBAGENT_ESCALATION_CHARACTERS = 8_000; +const INCOMPATIBLE_SUBAGENT_PARENT_HOOKS = [ + "resources_discover", + "input", + "context", + "before_agent_start", + "before_provider_request", + "before_provider_headers", + "session_before_compact", + "tool_call", + "tool_result", + "message_end", +] as const; // ============================================================================ // AgentSession Class @@ -532,7 +544,11 @@ export class AgentSession { activeToolNames: this._initialActiveToolNames, includeAllExtensionTools: true, }); - this._subagentController?.bindParent(this, (name, message) => this._acceptSubagentEscalation(name, message)); + this._subagentController?.bindParent( + this, + () => INCOMPATIBLE_SUBAGENT_PARENT_HOOKS.filter((hook) => this._extensionRunner.hasHandlers(hook)), + (name, message) => this._acceptSubagentEscalation(name, message), + ); } get modelRuntime(): ModelRuntime { diff --git a/packages/coding-agent/src/core/subagents/controller.ts b/packages/coding-agent/src/core/subagents/controller.ts index 3002d697a..92dc6bb3e 100644 --- a/packages/coding-agent/src/core/subagents/controller.ts +++ b/packages/coding-agent/src/core/subagents/controller.ts @@ -180,6 +180,7 @@ export class SubagentController { #closed = false; #resetting = false; #parent: AgentSession | undefined; + #getIncompatibleParentHooks: (() => readonly string[]) | undefined; #deliverEscalation: ((name: string, message: string) => Promise) | undefined; #resetPromise: Promise | undefined; #resetCleanupPromise: Promise | undefined; @@ -204,16 +205,19 @@ export class SubagentController { createToolDefinitions(): ToolDefinition[] { const definitions = [ - createSpawnAgentTool((name, task, toolCallId) => { - const parent = this.#getBoundParent(); - this.#assertReservable(name); - const origin = captureSpawnOrigin(parent, toolCallId); - const captured = this.#captureSession?.(parent, name, origin); - if (!captured) { - throw new Error("Subagent session capture is unavailable"); - } - this.spawn(name, task, captured.createSession, origin, parent.agent.telemetryContext, captured); - }), + createSpawnAgentTool( + (name, task, toolCallId) => { + const parent = this.#getBoundParent(); + this.#assertReservable(name); + const origin = captureSpawnOrigin(parent, toolCallId); + const captured = this.#captureSession?.(parent, name, origin); + if (!captured) { + throw new Error("Subagent session capture is unavailable"); + } + this.spawn(name, task, captured.createSession, origin, parent.agent.telemetryContext, captured); + }, + () => this.#assertParentPolicyCompatible(), + ), createAgentActionTool((request) => { const parent = this.#getBoundParent(); return request.action === "message" @@ -229,12 +233,17 @@ export class SubagentController { return definitions.map((definition) => definition as unknown as ToolDefinition); } - bindParent(session: AgentSession, deliverEscalation?: (name: string, message: string) => Promise): void { + bindParent( + session: AgentSession, + getIncompatibleParentHooks: () => readonly string[], + deliverEscalation?: (name: string, message: string) => Promise, + ): void { this.#assertOpen(); if (this.#parent) { throw new Error("Subagent controller parent is already bound"); } this.#parent = session; + this.#getIncompatibleParentHooks = getIncompatibleParentHooks; this.#deliverEscalation = deliverEscalation; } @@ -318,6 +327,7 @@ export class SubagentController { telemetryContext: TelemetryContext = this.#telemetryContext, captured?: CapturedSubagentSession, ): void { + this.#assertParentPolicyCompatible(); this.#assertReservable(name); if (this.#persistRoster && !captured) { throw new Error("Persistent subagent admission requires captured session state"); @@ -715,6 +725,7 @@ export class SubagentController { const resetCleanup = this.#resetCleanupPromise; const records = this.#detachRecords(closedError); this.#parent = undefined; + this.#getIncompatibleParentHooks = undefined; this.#deliverEscalation = undefined; const shutdownCleanup = this.#cleanupRecords(records); this.#publish( @@ -1542,6 +1553,14 @@ export class SubagentController { if (this.#resetting) throw this.#resetError(); } + #assertParentPolicyCompatible(): void { + const incompatibleHooks = this.#getIncompatibleParentHooks?.() ?? []; + if (incompatibleHooks.length === 0) return; + throw new Error( + `Cannot spawn a subagent while parent extensions register incompatible hooks: ${incompatibleHooks.join(", ")}. Remove those hooks or disable subagents for this session.`, + ); + } + #assertReservable(name: string): void { this.#assertOpen(); if (!SUBAGENT_NAME_PATTERN.test(name)) { diff --git a/packages/coding-agent/src/core/tools/subagents.ts b/packages/coding-agent/src/core/tools/subagents.ts index d6d17163a..8cf328bb5 100644 --- a/packages/coding-agent/src/core/tools/subagents.ts +++ b/packages/coding-agent/src/core/tools/subagents.ts @@ -160,6 +160,7 @@ function assertAgentsRequest(request: unknown): asserts request is AgentsRequest export function createSpawnAgentTool( spawn: (name: string, task: string, toolCallId: string) => void, + assertCanSpawn: () => void = () => {}, ): ToolDefinition { return { name: "spawn_agent", @@ -173,6 +174,7 @@ export function createSpawnAgentTool( parameters: spawnAgentSchema, executionMode: "parallel", execute: async (toolCallId, { name, task }) => { + assertCanSpawn(); if (!SUBAGENT_NAME_PATTERN.test(name)) { throw new Error(`Invalid subagent name: ${name}`); } diff --git a/packages/coding-agent/test/suite/subagent-registration.test.ts b/packages/coding-agent/test/suite/subagent-registration.test.ts index 4c7ac9c2a..5199e9448 100644 --- a/packages/coding-agent/test/suite/subagent-registration.test.ts +++ b/packages/coding-agent/test/suite/subagent-registration.test.ts @@ -15,7 +15,7 @@ import { createAgentSessionFromServices, createAgentSessionServices, } from "../../src/core/agent-session-services.ts"; -import type { ToolDefinition } from "../../src/core/extensions/types.ts"; +import type { ExtensionAPI, InlineExtension, ToolDefinition } from "../../src/core/extensions/types.ts"; import { type CreateAgentSessionOptions, createAgentSession } from "../../src/core/sdk.ts"; import { SessionManager } from "../../src/core/session-manager.ts"; import { SubagentController } from "../../src/core/subagents/controller.ts"; @@ -23,10 +23,84 @@ import { RootLocalEphemeralSubagentSessionManagerFactory, type SubagentSessionManagerRequest, } from "../../src/core/subagents/session-manager-factory.ts"; +import { createTestExtensionsResult, createTestResourceLoader } from "../utilities.ts"; import { createHarness, getMessageText, type Harness } from "./harness.ts"; import { getLastUserText } from "./subagent-harness.ts"; const COORDINATION_TOOLS = ["spawn_agent", "agent_action", "agents"]; +const INCOMPATIBLE_PARENT_HOOKS = [ + "resources_discover", + "input", + "context", + "before_agent_start", + "before_provider_request", + "before_provider_headers", + "session_before_compact", + "tool_call", + "tool_result", + "message_end", +] as const; +type IncompatibleParentHook = (typeof INCOMPATIBLE_PARENT_HOOKS)[number]; +type ObservationParentHook = "agent_start" | "after_provider_response" | "tool_execution_start"; + +function registerParentHook(api: ExtensionAPI, hook: IncompatibleParentHook | ObservationParentHook): void { + switch (hook) { + case "resources_discover": + api.on("resources_discover", () => {}); + break; + case "input": + api.on("input", () => {}); + break; + case "context": + api.on("context", () => {}); + break; + case "before_agent_start": + api.on("before_agent_start", () => {}); + break; + case "before_provider_request": + api.on("before_provider_request", () => {}); + break; + case "before_provider_headers": + api.on("before_provider_headers", () => {}); + break; + case "session_before_compact": + api.on("session_before_compact", () => {}); + break; + case "tool_call": + api.on("tool_call", () => {}); + break; + case "tool_result": + api.on("tool_result", () => {}); + break; + case "message_end": + api.on("message_end", () => {}); + break; + case "agent_start": + api.on("agent_start", () => {}); + break; + case "after_provider_response": + api.on("after_provider_response", () => {}); + break; + case "tool_execution_start": + api.on("tool_execution_start", () => {}); + break; + } +} + +function createParentHookExtension( + hooks: readonly (IncompatibleParentHook | ObservationParentHook)[], +): InlineExtension { + return { + name: "parent-policy-test", + factory: (api) => { + for (const hook of hooks) registerParentHook(api, hook); + }, + }; +} + +function parentPolicyError(hooks: readonly IncompatibleParentHook[]): string { + return `Cannot spawn a subagent while parent extensions register incompatible hooks: ${hooks.join(", ")}. Remove those hooks or disable subagents for this session.`; +} interface Deferred { readonly promise: Promise; @@ -67,6 +141,7 @@ describe("root subagent tool registration", () => { let rootIndex = 0; afterEach(async () => { + vi.restoreAllMocks(); while (runtimeCleanups.length > 0) await runtimeCleanups.pop()?.(); while (sessions.length > 0) await sessions.pop()?.dispose(); while (harnesses.length > 0) await harnesses.pop()?.cleanup(); @@ -124,6 +199,27 @@ describe("root subagent tool registration", () => { return { session, services }; } + async function createPolicyRoot( + harness: Harness, + hooks: readonly (IncompatibleParentHook | ObservationParentHook)[], + options: { sessionManager?: SessionManager; resourceLoader?: ReturnType } = {}, + ): Promise<{ session: AgentSession; controller: SubagentController }> { + const extensionsResult = await createTestExtensionsResult([createParentHookExtension(hooks)], harness.tempDir); + const { session } = await createAgentSession({ + cwd: harness.tempDir, + agentDir: join(harness.tempDir, "policy-agent"), + modelRuntime: harness.session.modelRuntime, + settingsManager: harness.settingsManager, + resourceLoader: options.resourceLoader ?? createTestResourceLoader({ extensionsResult }), + sessionManager: options.sessionManager ?? SessionManager.inMemory(harness.tempDir), + model: harness.getModel("faux-1"), + thinkingLevel: "high", + }); + sessions.push(session); + const controller = (session as unknown as { _subagentController: SubagentController })._subagentController; + return { session, controller }; + } + it("registers and activates all coordination tools by default", async () => { const harness = await setupHarness(); const customTool = createTestTool("caller_tool"); @@ -204,8 +300,117 @@ describe("root subagent tool registration", () => { await expect( spawn.execute("spawn", { name: "worker", task: "task" }, undefined, undefined, {} as never), ).rejects.toThrow("Subagent controller parent is not bound"); - controller.bindParent(session); - expect(() => controller.bindParent(session)).toThrow("Subagent controller parent is already bound"); + controller.bindParent(session, () => []); + expect(() => controller.bindParent(session, () => [])).toThrow("Subagent controller parent is already bound"); + await controller.shutdown(); + }); + + it.each(INCOMPATIBLE_PARENT_HOOKS)("rejects spawn when the parent registers %s", async (hook) => { + const harness = await setupHarness(); + const { session, controller } = await createPolicyRoot(harness, [hook]); + + await expect( + getActiveTool(session, "spawn_agent").execute("spawn", { name: "worker", task: "task" }), + ).rejects.toThrow(parentPolicyError([hook])); + expect(controller.snapshot().agents).toEqual([]); + }); + + it("reports incompatible hook combinations once in authoritative order", async () => { + const harness = await setupHarness(); + const registered = [...INCOMPATIBLE_PARENT_HOOKS].reverse(); + const { session } = await createPolicyRoot(harness, registered); + + await expect( + getActiveTool(session, "spawn_agent").execute("spawn", { name: "worker", task: "task" }), + ).rejects.toThrow(parentPolicyError(INCOMPATIBLE_PARENT_HOOKS)); + }); + + it("rejects before validation, child capture, persistence, reservation, or provider work", async () => { + const harness = await setupHarness(); + harness.setResponses([fauxAssistantMessage("unexpected child response")]); + const sessionManager = SessionManager.create(harness.tempDir, join(harness.tempDir, "policy-sessions")); + const createChildManager = vi.spyOn(RootLocalEphemeralSubagentSessionManagerFactory.prototype, "create"); + const orderedHooks = ["resources_discover", "input", "tool_call", "message_end"] as const; + const { session, controller } = await createPolicyRoot( + harness, + ["message_end", "input", "tool_call", "input", "resources_discover"], + { sessionManager }, + ); + const entryCount = sessionManager.getEntryCount(); + const pendingResponses = harness.getPendingResponseCount(); + + await expect( + getActiveTool(session, "spawn_agent").execute("spawn", { name: "INVALID", task: "" }), + ).rejects.toThrow(parentPolicyError(orderedHooks)); + expect(controller.snapshot().agents).toEqual([]); + expect(sessionManager.getEntryCount()).toBe(entryCount); + expect(createChildManager).not.toHaveBeenCalled(); + expect(harness.getPendingResponseCount()).toBe(pendingResponses); + + const createSession = vi.fn(); + expect(() => controller.spawn("direct", "task", createSession)).toThrow(parentPolicyError(orderedHooks)); + expect(createSession).not.toHaveBeenCalled(); + expect(controller.snapshot().agents).toEqual([]); + expect(sessionManager.getEntryCount()).toBe(entryCount); + }); + + it("keeps observation hooks and non-spawn actions admissible", async () => { + const harness = await setupHarness(); + harness.setResponses([fauxAssistantMessage("child complete")]); + const observed = await createPolicyRoot(harness, [ + "agent_start", + "after_provider_response", + "tool_execution_start", + ]); + await getActiveTool(observed.session, "spawn_agent").execute("spawn", { name: "worker", task: "task" }); + await expect( + getActiveTool(observed.session, "agents").execute("wait", { timeout_ms: 1_000, until: "all" }), + ).resolves.toMatchObject({ details: { agents: [{ name: "worker", status: "idle" }] } }); + + const guarded = await createPolicyRoot(harness, ["context"]); + guarded.controller.reserve("existing", "existing task"); + guarded.controller.markFailed("existing", "existing failure"); + await expect( + getActiveTool(guarded.session, "agent_action").execute("interrupt", { + target: "existing", + action: "interrupt", + }), + ).resolves.toMatchObject({ details: { target: "existing", status: "failed" } }); + await expect(getActiveTool(guarded.session, "agents").execute("list", { timeout_ms: 0 })).resolves.toMatchObject({ + details: { agents: [{ name: "existing", status: "failed" }] }, + }); + }); + + it("checks the current extension runner after reload", async () => { + const harness = await setupHarness(); + const initialExtensions = await createTestExtensionsResult([], harness.tempDir); + const reloadedExtensions = await createTestExtensionsResult( + [createParentHookExtension(["before_provider_headers"])], + harness.tempDir, + ); + let currentExtensions = initialExtensions; + const resourceLoader = createTestResourceLoader(); + resourceLoader.getExtensions = () => currentExtensions; + resourceLoader.reload = async () => { + currentExtensions = reloadedExtensions; + }; + const { session } = await createPolicyRoot(harness, [], { resourceLoader }); + + await session.reload(); + await expect( + getActiveTool(session, "spawn_agent").execute("spawn", { name: "worker", task: "task" }), + ).rejects.toThrow(parentPolicyError(["before_provider_headers"])); + }); + + it("preserves unbound programmatic controller spawning", async () => { + const child = await setupHarness(); + child.setResponses([fauxAssistantMessage("programmatic child complete")]); + const controller = new SubagentController({ maxConcurrent: 1 }); + + controller.spawn("worker", "task", async () => child.session); + await expect(controller.queryAgents({ timeoutMs: 1_000, until: "all" })).resolves.toMatchObject({ + agents: [{ name: "worker", status: "idle", result: "programmatic child complete" }], + }); await controller.shutdown(); });