diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 5474e943..d5d7e5dd 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -6,6 +6,7 @@ - Fixed extra spacing between thinking-only assistant content and subsequent tool execution blocks when assistant messages contain no text - Fixed queued steering/follow-up/custom messages remaining stuck after threshold auto-compaction by resuming the agent loop when Agent-level queues still contain pending messages ([#1312](https://github.com/badlogic/pi-mono/pull/1312) by [@ferologics](https://github.com/ferologics)) +- Fixed `tool_result` extension handlers to chain result patches across handlers instead of last-handler-wins behavior ([#1280](https://github.com/badlogic/pi-mono/issues/1280)) ## [0.52.6] - 2026-02-05 diff --git a/packages/coding-agent/docs/extensions.md b/packages/coding-agent/docs/extensions.md index 6ee39fb4..332ce755 100644 --- a/packages/coding-agent/docs/extensions.md +++ b/packages/coding-agent/docs/extensions.md @@ -524,6 +524,11 @@ pi.on("tool_call", (event) => { Fired after tool executes. **Can modify result.** +`tool_result` handlers chain like middleware: +- Handlers run in extension load order +- Each handler sees the latest result after previous handler changes +- Handlers can return partial patches (`content`, `details`, or `isError`); omitted fields keep their current values + ```typescript import { isBashToolResult } from "@mariozechner/pi-coding-agent"; diff --git a/packages/coding-agent/src/core/extensions/runner.ts b/packages/coding-agent/src/core/extensions/runner.ts index 09df246f..63862731 100644 --- a/packages/coding-agent/src/core/extensions/runner.ts +++ b/packages/coding-agent/src/core/extensions/runner.ts @@ -41,6 +41,7 @@ import type { SessionBeforeTreeResult, ToolCallEvent, ToolCallEventResult, + ToolResultEvent, ToolResultEventResult, UserBashEvent, UserBashEventResult, @@ -98,7 +99,13 @@ interface BeforeAgentStartCombinedResult { */ type RunnerEmitEvent = Exclude< ExtensionEvent, - ToolCallEvent | UserBashEvent | ContextEvent | BeforeAgentStartEvent | ResourcesDiscoverEvent | InputEvent + | ToolCallEvent + | ToolResultEvent + | UserBashEvent + | ContextEvent + | BeforeAgentStartEvent + | ResourcesDiscoverEvent + | InputEvent >; export type ExtensionErrorListener = (error: ExtensionError) => void; @@ -484,11 +491,9 @@ export class ExtensionRunner { ); } - async emit( - event: RunnerEmitEvent, - ): Promise { + async emit(event: RunnerEmitEvent): Promise { const ctx = this.createContext(); - let result: SessionBeforeCompactResult | SessionBeforeTreeResult | ToolResultEventResult | undefined; + let result: SessionBeforeCompactResult | SessionBeforeTreeResult | undefined; for (const ext of this.extensions) { const handlers = ext.handlers.get(event.type); @@ -504,10 +509,6 @@ export class ExtensionRunner { return result; } } - - if (event.type === "tool_result" && handlerResult) { - result = handlerResult as ToolResultEventResult; - } } catch (err) { const message = err instanceof Error ? err.message : String(err); const stack = err instanceof Error ? err.stack : undefined; @@ -524,6 +525,56 @@ export class ExtensionRunner { return result; } + async emitToolResult(event: ToolResultEvent): Promise { + const ctx = this.createContext(); + const currentEvent: ToolResultEvent = { ...event }; + let modified = false; + + for (const ext of this.extensions) { + const handlers = ext.handlers.get("tool_result"); + if (!handlers || handlers.length === 0) continue; + + for (const handler of handlers) { + try { + const handlerResult = (await handler(currentEvent, ctx)) as ToolResultEventResult | undefined; + if (!handlerResult) continue; + + if (handlerResult.content !== undefined) { + currentEvent.content = handlerResult.content; + modified = true; + } + if (handlerResult.details !== undefined) { + currentEvent.details = handlerResult.details; + modified = true; + } + if (handlerResult.isError !== undefined) { + currentEvent.isError = handlerResult.isError; + modified = true; + } + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + const stack = err instanceof Error ? err.stack : undefined; + this.emitError({ + extensionPath: ext.path, + event: "tool_result", + error: message, + stack, + }); + } + } + } + + if (!modified) { + return undefined; + } + + return { + content: currentEvent.content, + details: currentEvent.details, + isError: currentEvent.isError, + }; + } + async emitToolCall(event: ToolCallEvent): Promise { const ctx = this.createContext(); let result: ToolCallEventResult | undefined; diff --git a/packages/coding-agent/src/core/extensions/wrapper.ts b/packages/coding-agent/src/core/extensions/wrapper.ts index e88f96e8..736c87f5 100644 --- a/packages/coding-agent/src/core/extensions/wrapper.ts +++ b/packages/coding-agent/src/core/extensions/wrapper.ts @@ -4,7 +4,7 @@ import type { AgentTool, AgentToolUpdateCallback } from "@mariozechner/pi-agent-core"; import type { ExtensionRunner } from "./runner.js"; -import type { RegisteredTool, ToolCallEventResult, ToolResultEventResult } from "./types.js"; +import type { RegisteredTool, ToolCallEventResult } from "./types.js"; /** * Wrap a RegisteredTool into an AgentTool. @@ -72,7 +72,7 @@ export function wrapToolWithExtensions(tool: AgentTool, runner: Exten // Emit tool_result event - extensions can modify the result if (runner.hasHandlers("tool_result")) { - const resultResult = (await runner.emit({ + const resultResult = await runner.emitToolResult({ type: "tool_result", toolName: tool.name, toolCallId, @@ -80,7 +80,7 @@ export function wrapToolWithExtensions(tool: AgentTool, runner: Exten content: result.content, details: result.details, isError: false, - })) as ToolResultEventResult | undefined; + }); if (resultResult) { return { @@ -94,7 +94,7 @@ export function wrapToolWithExtensions(tool: AgentTool, runner: Exten } catch (err) { // Emit tool_result event for errors if (runner.hasHandlers("tool_result")) { - await runner.emit({ + await runner.emitToolResult({ type: "tool_result", toolName: tool.name, toolCallId, diff --git a/packages/coding-agent/test/extensions-runner.test.ts b/packages/coding-agent/test/extensions-runner.test.ts index e9747c1d..df2f2036 100644 --- a/packages/coding-agent/test/extensions-runner.test.ts +++ b/packages/coding-agent/test/extensions-runner.test.ts @@ -399,6 +399,98 @@ describe("ExtensionRunner", () => { }); }); + describe("tool_result chaining", () => { + it("chains content modifications across handlers", async () => { + const extCode1 = ` + export default function(pi) { + pi.on("tool_result", async (event) => { + return { + content: [...event.content, { type: "text", text: "ext1" }], + }; + }); + } + `; + const extCode2 = ` + export default function(pi) { + pi.on("tool_result", async (event) => { + return { + content: [...event.content, { type: "text", text: "ext2" }], + }; + }); + } + `; + fs.writeFileSync(path.join(extensionsDir, "tool-result-1.ts"), extCode1); + fs.writeFileSync(path.join(extensionsDir, "tool-result-2.ts"), extCode2); + + const result = await discoverAndLoadExtensions([], tempDir, tempDir); + const runner = new ExtensionRunner(result.extensions, result.runtime, tempDir, sessionManager, modelRegistry); + + const chained = await runner.emitToolResult({ + type: "tool_result", + toolName: "my_tool", + toolCallId: "call-1", + input: {}, + content: [{ type: "text", text: "base" }], + details: { initial: true }, + isError: false, + }); + + expect(chained).toBeDefined(); + const chainedContent = chained?.content; + expect(chainedContent).toBeDefined(); + expect(chainedContent![0]).toEqual({ type: "text", text: "base" }); + expect(chainedContent).toHaveLength(3); + const appendedText = chainedContent! + .slice(1) + .filter((item): item is { type: "text"; text: string } => item.type === "text") + .map((item) => item.text); + expect(appendedText.sort()).toEqual(["ext1", "ext2"]); + }); + + it("preserves previous modifications when later handlers return partial patches", async () => { + const extCode1 = ` + export default function(pi) { + pi.on("tool_result", async () => { + return { + content: [{ type: "text", text: "first" }], + details: { source: "ext1" }, + }; + }); + } + `; + const extCode2 = ` + export default function(pi) { + pi.on("tool_result", async () => { + return { + isError: true, + }; + }); + } + `; + fs.writeFileSync(path.join(extensionsDir, "tool-result-partial-1.ts"), extCode1); + fs.writeFileSync(path.join(extensionsDir, "tool-result-partial-2.ts"), extCode2); + + const result = await discoverAndLoadExtensions([], tempDir, tempDir); + const runner = new ExtensionRunner(result.extensions, result.runtime, tempDir, sessionManager, modelRegistry); + + const chained = await runner.emitToolResult({ + type: "tool_result", + toolName: "my_tool", + toolCallId: "call-2", + input: {}, + content: [{ type: "text", text: "base" }], + details: { initial: true }, + isError: false, + }); + + expect(chained).toEqual({ + content: [{ type: "text", text: "first" }], + details: { source: "ext1" }, + isError: true, + }); + }); + }); + describe("hasHandlers", () => { it("returns true when handlers exist for event type", async () => { const extCode = `