diff --git a/CHANGELOG.md b/CHANGELOG.md index daaeece9dec..35d69d5d3b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ Docs: https://docs.openclaw.ai ### Breaking -- Plugin SDK/tool-result transforms: deprecate the Pi-only `api.registerEmbeddedExtensionFactory(...)` path for tool-result rewriting in favor of `api.registerAgentToolResultMiddleware(...)`, with `contracts.agentToolResultMiddleware` declaring the targeted harnesses. The legacy Pi hook remains wired as a bundled compatibility seam, but new plugins should use the harness-neutral middleware contract so transforms run consistently across Pi and Codex app-server dynamic tools. Thanks @vincentkoc. +- Plugin SDK/tool-result transforms: deprecate the Pi-only `api.registerEmbeddedExtensionFactory(...)` path for tool-result rewriting in favor of bundled `api.registerAgentToolResultMiddleware(...)`, with `contracts.agentToolResultMiddleware` declaring the targeted harnesses. The legacy Pi hook remains wired as a bundled compatibility seam, but new bundled transforms should use the harness-neutral middleware contract so transforms run consistently across Pi and Codex app-server dynamic tools. Thanks @vincentkoc. ### Changes @@ -54,6 +54,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Plugin SDK/tool-result transforms: restrict harness tool-result middleware to bundled plugins, fail closed on middleware errors, validate rewritten result shapes, preserve Pi per-call ids, and keep Codex media trust checks anchored to raw tool provenance. Thanks @vincentkoc. - Plugins/Google Chat: log webhook auth rejection reasons only after all candidates fail, and warn when add-on `appPrincipal` values do not match configuration. Fixes #71078. (#71145) Thanks @luyao618. - Models/configure: preserve the existing default model when provider auth is re-run from configure while keeping explicit default-setting commands authoritative. Fixes #70696. (#70793) Thanks @Sathvik-1007. - Config/plugins: accept `plugins.entries.*.hooks.allowConversationAccess` in validation, generated schema metadata, and plugin policy inspection so trusted external plugins can enable conversation-access hooks such as `agent_end` without local schema patches. Fixes #71215. (#71221) Thanks @BillChirico. diff --git a/docs/.generated/plugin-sdk-api-baseline.sha256 b/docs/.generated/plugin-sdk-api-baseline.sha256 index 66a2a097918..8487f07dbe5 100644 --- a/docs/.generated/plugin-sdk-api-baseline.sha256 +++ b/docs/.generated/plugin-sdk-api-baseline.sha256 @@ -1,2 +1,2 @@ -b86e6785acbd0f8f0f012691ce823c2e8d52bfd2507c2258408503162abb9adf plugin-sdk-api-baseline.json -e10acbed6c7c1b21700e358411c73877bdc0cb59ce102bafe680e210a2ac741b plugin-sdk-api-baseline.jsonl +a2005473b59e26995f563e8bd3f1c6782bd3ee193a65dd3255160d40e505fa4d plugin-sdk-api-baseline.json +5778c0bb6cfc85c1b0665ec431269bcb43b8891720d0b30467681420ab411721 plugin-sdk-api-baseline.jsonl diff --git a/docs/plugins/building-plugins.md b/docs/plugins/building-plugins.md index c69890d286e..5d5377cdfc2 100644 --- a/docs/plugins/building-plugins.md +++ b/docs/plugins/building-plugins.md @@ -170,11 +170,12 @@ A single plugin can register any number of capabilities via the `api` object: For the full registration API, see [SDK Overview](/plugins/sdk-overview#registration-api). -Use `api.registerAgentToolResultMiddleware(...)` when a plugin needs async -tool-result rewriting before the model sees the output. Declare the targeted -harnesses in `contracts.agentToolResultMiddleware`, for example -`["pi", "codex-app-server"]`. Prefer regular OpenClaw plugin hooks when the -work does not need pre-model tool-result timing. +Bundled plugins can use `api.registerAgentToolResultMiddleware(...)` when they +need async tool-result rewriting before the model sees the output. Declare the +targeted harnesses in `contracts.agentToolResultMiddleware`, for example +`["pi", "codex-app-server"]`. This is a trusted bundled-plugin seam; external +plugins should prefer regular OpenClaw plugin hooks unless OpenClaw grows an +explicit trust policy for this capability. If your plugin registers custom gateway RPC methods, keep them on a plugin-specific prefix. Core admin namespaces (`config.*`, diff --git a/docs/plugins/manifest.md b/docs/plugins/manifest.md index 4617b4393c3..cfca2d643fa 100644 --- a/docs/plugins/manifest.md +++ b/docs/plugins/manifest.md @@ -422,26 +422,28 @@ read without importing the plugin runtime. Each list is optional: -| Field | Type | What it means | -| -------------------------------- | ---------- | ---------------------------------------------------------------- | -| `embeddedExtensionFactories` | `string[]` | Deprecated embedded extension factory ids. | -| `agentToolResultMiddleware` | `string[]` | Harness ids this plugin may register tool-result middleware for. | -| `externalAuthProviders` | `string[]` | Provider ids whose external auth profile hook this plugin owns. | -| `speechProviders` | `string[]` | Speech provider ids this plugin owns. | -| `realtimeTranscriptionProviders` | `string[]` | Realtime-transcription provider ids this plugin owns. | -| `realtimeVoiceProviders` | `string[]` | Realtime-voice provider ids this plugin owns. | -| `memoryEmbeddingProviders` | `string[]` | Memory embedding provider ids this plugin owns. | -| `mediaUnderstandingProviders` | `string[]` | Media-understanding provider ids this plugin owns. | -| `imageGenerationProviders` | `string[]` | Image-generation provider ids this plugin owns. | -| `videoGenerationProviders` | `string[]` | Video-generation provider ids this plugin owns. | -| `webFetchProviders` | `string[]` | Web-fetch provider ids this plugin owns. | -| `webSearchProviders` | `string[]` | Web-search provider ids this plugin owns. | -| `tools` | `string[]` | Agent tool names this plugin owns for bundled contract checks. | +| Field | Type | What it means | +| -------------------------------- | ---------- | --------------------------------------------------------------------- | +| `embeddedExtensionFactories` | `string[]` | Deprecated embedded extension factory ids. | +| `agentToolResultMiddleware` | `string[]` | Harness ids a bundled plugin may register tool-result middleware for. | +| `externalAuthProviders` | `string[]` | Provider ids whose external auth profile hook this plugin owns. | +| `speechProviders` | `string[]` | Speech provider ids this plugin owns. | +| `realtimeTranscriptionProviders` | `string[]` | Realtime-transcription provider ids this plugin owns. | +| `realtimeVoiceProviders` | `string[]` | Realtime-voice provider ids this plugin owns. | +| `memoryEmbeddingProviders` | `string[]` | Memory embedding provider ids this plugin owns. | +| `mediaUnderstandingProviders` | `string[]` | Media-understanding provider ids this plugin owns. | +| `imageGenerationProviders` | `string[]` | Image-generation provider ids this plugin owns. | +| `videoGenerationProviders` | `string[]` | Video-generation provider ids this plugin owns. | +| `webFetchProviders` | `string[]` | Web-fetch provider ids this plugin owns. | +| `webSearchProviders` | `string[]` | Web-search provider ids this plugin owns. | +| `tools` | `string[]` | Agent tool names this plugin owns for bundled contract checks. | `contracts.embeddedExtensionFactories` is retained for bundled compatibility -code that still needs direct Pi embedded-runner events. New tool-result -transforms should declare `contracts.agentToolResultMiddleware` and register -with `api.registerAgentToolResultMiddleware(...)` instead. +code that still needs direct Pi embedded-runner events. New bundled +tool-result transforms should declare `contracts.agentToolResultMiddleware` +and register with `api.registerAgentToolResultMiddleware(...)` instead. +External plugins cannot register tool-result middleware because the seam can +rewrite high-trust tool output before the model sees it. Provider plugins that implement `resolveExternalAuthProfiles` should declare `contracts.externalAuthProviders`. Plugins without the declaration still run diff --git a/docs/plugins/sdk-agent-harness.md b/docs/plugins/sdk-agent-harness.md index 60eeff4e8de..e5bb2d49c44 100644 --- a/docs/plugins/sdk-agent-harness.md +++ b/docs/plugins/sdk-agent-harness.md @@ -146,11 +146,11 @@ OpenClaw only runs against the protocol surface it has been tested with. ### Tool-result middleware -Plugins can attach harness-neutral tool-result middleware through +Bundled plugins can attach harness-neutral tool-result middleware through `api.registerAgentToolResultMiddleware(...)` when their manifest declares the -targeted harness ids in `contracts.agentToolResultMiddleware`. This is the seam -for async tool-result transforms that must run before PI or Codex feeds tool -output back into the model. +targeted harness ids in `contracts.agentToolResultMiddleware`. This trusted +seam is for async tool-result transforms that must run before PI or Codex feeds +tool output back into the model. Legacy bundled plugins can still use `api.registerCodexAppServerExtensionFactory(...)` for Codex app-server-only diff --git a/docs/plugins/sdk-migration.md b/docs/plugins/sdk-migration.md index 97800589d11..671e94b771d 100644 --- a/docs/plugins/sdk-migration.md +++ b/docs/plugins/sdk-migration.md @@ -91,8 +91,9 @@ releases. - Replace Pi-only `api.registerEmbeddedExtensionFactory(...)` tool-result - handlers with harness-neutral middleware. + Bundled plugins should replace Pi-only + `api.registerEmbeddedExtensionFactory(...)` tool-result handlers with + harness-neutral middleware. ```typescript // Before: Pi-only compatibility hook @@ -121,7 +122,9 @@ releases. ``` Keep `contracts.embeddedExtensionFactories` only for bundled compatibility - code that still needs direct Pi embedded-runner events. + code that still needs direct Pi embedded-runner events. External plugins + cannot register tool-result middleware because it can rewrite high-trust + tool output before the model sees it. diff --git a/docs/plugins/sdk-overview.md b/docs/plugins/sdk-overview.md index a7421ef8f7e..9a31d935f5a 100644 --- a/docs/plugins/sdk-overview.md +++ b/docs/plugins/sdk-overview.md @@ -112,14 +112,15 @@ methods: - Use `api.registerAgentToolResultMiddleware(...)` when a plugin needs to - rewrite a tool result after execution and before the harness feeds that - result back into the model. This is the harness-neutral seam for async output - reducers such as tokenjuice. + Bundled plugins can use `api.registerAgentToolResultMiddleware(...)` when + they need to rewrite a tool result after execution and before the harness + feeds that result back into the model. This is the trusted harness-neutral + seam for async output reducers such as tokenjuice. -Plugins must declare `contracts.agentToolResultMiddleware` for each targeted -harness, for example `["pi", "codex-app-server"]`. Keep normal OpenClaw -plugin hooks for work that does not need pre-model tool-result timing. +Bundled plugins must declare `contracts.agentToolResultMiddleware` for each +targeted harness, for example `["pi", "codex-app-server"]`. External plugins +cannot register this middleware; keep normal OpenClaw plugin hooks for work +that does not need pre-model tool-result timing. diff --git a/extensions/codex/src/app-server/dynamic-tools.test.ts b/extensions/codex/src/app-server/dynamic-tools.test.ts index 6c6ff49200b..22ab952d43e 100644 --- a/extensions/codex/src/app-server/dynamic-tools.test.ts +++ b/extensions/codex/src/app-server/dynamic-tools.test.ts @@ -257,6 +257,83 @@ describe("createCodexDynamicToolBridge", () => { ); }); + it("passes raw tool failure state into agent tool result middleware", async () => { + const registry = createEmptyPluginRegistry(); + const handler = vi.fn(async (_event: { isError?: boolean }) => undefined); + registry.agentToolResultMiddlewares.push({ + pluginId: "tokenjuice", + pluginName: "Tokenjuice", + rawHandler: handler, + handler, + harnesses: ["codex-app-server"], + source: "test", + }); + setActivePluginRegistry(registry); + + const bridge = createBridgeWithToolResult("exec", { + content: [{ type: "text", text: "failed output" }], + details: { status: "failed", exitCode: 1 }, + }); + + await bridge.handleToolCall({ + threadId: "thread-1", + turnId: "turn-1", + callId: "call-1", + namespace: null, + tool: "exec", + arguments: { command: "false" }, + }); + + expect(handler).toHaveBeenCalledWith( + expect.objectContaining({ isError: true }), + expect.objectContaining({ harness: "codex-app-server" }), + ); + }); + + it("uses raw tool provenance for media trust after middleware rewrites details", async () => { + const registry = createEmptyPluginRegistry(); + const handler = vi.fn(async (event: { result: AgentToolResult }) => ({ + result: { + ...event.result, + content: [{ type: "text" as const, text: "Generated media reply." }], + details: { + media: { + mediaUrl: "/tmp/unsafe.png", + }, + }, + }, + })); + registry.agentToolResultMiddlewares.push({ + pluginId: "tokenjuice", + pluginName: "Tokenjuice", + rawHandler: handler, + handler, + harnesses: ["codex-app-server"], + source: "test", + }); + setActivePluginRegistry(registry); + + const bridge = createBridgeWithToolResult("browser", { + content: [{ type: "text", text: "raw output" }], + details: { + mcpServer: "external", + mcpTool: "browser", + }, + }); + + const result = await bridge.handleToolCall({ + threadId: "thread-1", + turnId: "turn-1", + callId: "call-1", + namespace: null, + tool: "browser", + arguments: {}, + }); + + expect(result).toEqual(expectInputText("Generated media reply.")); + expect(bridge.telemetry.toolMediaUrls).toEqual([]); + }); + it("still applies legacy codex app-server extension factories after middleware", async () => { const registry = createEmptyPluginRegistry(); const factory = async (codex: { diff --git a/extensions/codex/src/app-server/dynamic-tools.ts b/extensions/codex/src/app-server/dynamic-tools.ts index 69c5f56ff2b..097ab3791ce 100644 --- a/extensions/codex/src/app-server/dynamic-tools.ts +++ b/extensions/codex/src/app-server/dynamic-tools.ts @@ -87,12 +87,14 @@ export function createCodexDynamicToolBridge(params: { try { const preparedArgs = tool.prepareArguments ? tool.prepareArguments(args) : args; const rawResult = await tool.execute(call.callId, preparedArgs, params.signal); + const rawIsError = isToolResultError(rawResult); const middlewareResult = await middlewareRunner.applyToolResultMiddleware({ threadId: call.threadId, turnId: call.turnId, toolCallId: call.callId, toolName: tool.name, args, + isError: rawIsError, result: rawResult, }); const result = await legacyExtensionRunner.applyToolResultExtensions({ @@ -107,8 +109,9 @@ export function createCodexDynamicToolBridge(params: { toolName: tool.name, args, result, + mediaTrustResult: rawResult, telemetry, - isError: false, + isError: rawIsError || isToolResultError(result), }); void runAgentHarnessAfterToolCallHook({ toolName: tool.name, @@ -162,6 +165,7 @@ function collectToolTelemetry(params: { toolName: string; args: Record; result: AgentToolResult | undefined; + mediaTrustResult?: AgentToolResult; telemetry: CodexDynamicToolBridge["telemetry"]; isError: boolean; }): void { @@ -174,7 +178,11 @@ function collectToolTelemetry(params: { if (!params.isError && params.result) { const media = extractToolResultMediaArtifact(params.result); if (media) { - const mediaUrls = filterToolResultMediaUrls(params.toolName, media.mediaUrls, params.result); + const mediaUrls = filterToolResultMediaUrls( + params.toolName, + media.mediaUrls, + params.mediaTrustResult ?? params.result, + ); const seen = new Set(params.telemetry.toolMediaUrls); for (const mediaUrl of mediaUrls) { if (!seen.has(mediaUrl)) { @@ -208,6 +216,35 @@ function collectToolTelemetry(params: { }); } +function isRecord(value: unknown): value is Record { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +function isToolResultError(result: AgentToolResult): boolean { + const details = result.details; + if (!isRecord(details)) { + return false; + } + if (details.timedOut === true) { + return true; + } + if (typeof details.exitCode === "number" && details.exitCode !== 0) { + return true; + } + if (typeof details.status !== "string") { + return false; + } + const status = details.status.trim().toLowerCase(); + return ( + status !== "" && + status !== "0" && + status !== "ok" && + status !== "success" && + status !== "completed" && + status !== "running" + ); +} + function convertToolContent( content: TextContent | ImageContent, ): CodexDynamicToolCallOutputContentItem[] { diff --git a/src/agents/codex-app-server.extensions.test.ts b/src/agents/codex-app-server.extensions.test.ts index 2e13cf7bc4d..63c6faf511f 100644 --- a/src/agents/codex-app-server.extensions.test.ts +++ b/src/agents/codex-app-server.extensions.test.ts @@ -118,6 +118,43 @@ describe("agent tool result middleware", () => { expect(listAgentToolResultMiddlewares("codex-app-server")).toHaveLength(0); }); + it("rejects middleware from non-bundled plugins even when they declare the contract", () => { + const tmp = createTempDir(); + process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = "/nonexistent/bundled/plugins"; + + const pluginFile = writeTempPlugin({ + dir: tmp, + id: "tool-result-middleware", + manifest: { + contracts: { + agentToolResultMiddleware: ["codex-app-server"], + }, + }, + body: `export default { id: "tool-result-middleware", register(api) { + api.registerAgentToolResultMiddleware(() => undefined, { harnesses: ["codex-app-server"] }); +} };`, + }); + + const registry = loadOpenClawPlugins({ + workspaceDir: tmp, + config: { + plugins: { + load: { paths: [pluginFile] }, + allow: ["tool-result-middleware"], + }, + }, + }); + + expect(registry.diagnostics).toContainEqual( + expect.objectContaining({ + level: "error", + pluginId: "tool-result-middleware", + message: "only bundled plugins can register agent tool result middleware", + }), + ); + expect(listAgentToolResultMiddlewares("codex-app-server")).toHaveLength(0); + }); + it("merges harnesses when a plugin registers the same middleware function twice", () => { const tmp = createTempDir(); process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = tmp; diff --git a/src/agents/harness/tool-result-middleware.test.ts b/src/agents/harness/tool-result-middleware.test.ts new file mode 100644 index 00000000000..330b7ca375c --- /dev/null +++ b/src/agents/harness/tool-result-middleware.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it } from "vitest"; +import { createAgentToolResultMiddlewareRunner } from "./tool-result-middleware.js"; + +describe("createAgentToolResultMiddlewareRunner", () => { + it("fails closed when middleware throws", async () => { + const runner = createAgentToolResultMiddlewareRunner({ harness: "pi" }, [ + () => { + throw new Error("raw secret should not be logged or returned"); + }, + ]); + + const result = await runner.applyToolResultMiddleware({ + toolCallId: "call-1", + toolName: "exec", + args: {}, + result: { content: [{ type: "text", text: "raw secret" }], details: {} }, + }); + + expect(result).toEqual({ + content: [ + { + type: "text", + text: "Tool output unavailable due to post-processing error.", + }, + ], + details: { + status: "failed", + middlewareError: true, + }, + }); + }); + + it("discard invalid middleware results and keeps the previous result", async () => { + const original = { content: [{ type: "text" as const, text: "raw" }], details: {} }; + const runner = createAgentToolResultMiddlewareRunner({ harness: "codex-app-server" }, [ + () => ({ result: { content: "not an array" } as never }), + ]); + + const result = await runner.applyToolResultMiddleware({ + toolCallId: "call-1", + toolName: "exec", + args: {}, + result: original, + }); + + expect(result).toBe(original); + }); + + it("accepts well-formed middleware results", async () => { + const runner = createAgentToolResultMiddlewareRunner({ harness: "codex-app-server" }, [ + () => ({ + result: { + content: [{ type: "text", text: "compacted" }], + details: { compacted: true }, + }, + }), + ]); + + const result = await runner.applyToolResultMiddleware({ + toolCallId: "call-1", + toolName: "exec", + args: {}, + result: { content: [{ type: "text", text: "raw" }], details: {} }, + }); + + expect(result.content).toEqual([{ type: "text", text: "compacted" }]); + expect(result.details).toEqual({ compacted: true }); + }); +}); diff --git a/src/agents/harness/tool-result-middleware.ts b/src/agents/harness/tool-result-middleware.ts index 761b317993b..e8e80e14ffe 100644 --- a/src/agents/harness/tool-result-middleware.ts +++ b/src/agents/harness/tool-result-middleware.ts @@ -6,8 +6,59 @@ import type { OpenClawAgentToolResult, } from "../../plugins/agent-tool-result-middleware-types.js"; import { listAgentToolResultMiddlewares } from "../../plugins/agent-tool-result-middleware.js"; +import { truncateUtf16Safe } from "../../utils.js"; const log = createSubsystemLogger("agents/harness"); +const MAX_MIDDLEWARE_CONTENT_BLOCKS = 200; +const MAX_MIDDLEWARE_TEXT_CHARS = 100_000; +const MAX_MIDDLEWARE_IMAGE_DATA_CHARS = 5_000_000; + +function isRecord(value: unknown): value is Record { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +function isValidMiddlewareContentBlock(value: unknown): boolean { + if (!isRecord(value) || typeof value.type !== "string") { + return false; + } + if (value.type === "text") { + return typeof value.text === "string" && value.text.length <= MAX_MIDDLEWARE_TEXT_CHARS; + } + if (value.type === "image") { + return ( + typeof value.mimeType === "string" && + value.mimeType.trim().length > 0 && + typeof value.data === "string" && + value.data.length <= MAX_MIDDLEWARE_IMAGE_DATA_CHARS + ); + } + return false; +} + +function isValidMiddlewareToolResult(value: unknown): value is OpenClawAgentToolResult { + if (!isRecord(value) || !Array.isArray(value.content)) { + return false; + } + if (value.content.length > MAX_MIDDLEWARE_CONTENT_BLOCKS) { + return false; + } + return value.content.every(isValidMiddlewareContentBlock); +} + +function buildMiddlewareFailureResult(): OpenClawAgentToolResult { + return { + content: [ + { + type: "text", + text: "Tool output unavailable due to post-processing error.", + }, + ], + details: { + status: "failed", + middlewareError: true, + }, + }; +} export function createAgentToolResultMiddlewareRunner( ctx: AgentToolResultMiddlewareContext, @@ -22,13 +73,25 @@ export function createAgentToolResultMiddlewareRunner( try { const next = await handler({ ...event, result: current }, ctx); if (next?.result) { - current = next.result; + if (isValidMiddlewareToolResult(next.result)) { + current = next.result; + } else { + log.warn( + `[${ctx.harness}] discarded invalid tool result middleware output for ${truncateUtf16Safe( + event.toolName, + 120, + )}`, + ); + } } - } catch (error) { - const detail = error instanceof Error ? error.message : String(error); + } catch { log.warn( - `[${ctx.harness}] tool result middleware failed for ${event.toolName}: ${detail}`, + `[${ctx.harness}] tool result middleware failed for ${truncateUtf16Safe( + event.toolName, + 120, + )}`, ); + return buildMiddlewareFailureResult(); } } return current; diff --git a/src/agents/pi-embedded-runner.extensions.test.ts b/src/agents/pi-embedded-runner.extensions.test.ts index 78397379027..6bdb1e45bce 100644 --- a/src/agents/pi-embedded-runner.extensions.test.ts +++ b/src/agents/pi-embedded-runner.extensions.test.ts @@ -2,6 +2,8 @@ import { SessionManager } from "@mariozechner/pi-coding-agent"; import { afterEach, describe, expect, it } from "vitest"; import { listEmbeddedExtensionFactories } from "../plugins/embedded-extension-factory.js"; import { loadOpenClawPlugins } from "../plugins/loader.js"; +import { createEmptyPluginRegistry } from "../plugins/registry.js"; +import { setActivePluginRegistry } from "../plugins/runtime.js"; import { buildEmbeddedExtensionFactories } from "./pi-embedded-runner/extensions.js"; import { cleanupTempPluginTestEnvironment, @@ -22,6 +24,63 @@ afterEach(() => { }); describe("buildEmbeddedExtensionFactories", () => { + it("bridges middleware mutations with unique fallback tool call ids", async () => { + const seenToolCallIds: string[] = []; + const registry = createEmptyPluginRegistry(); + registry.agentToolResultMiddlewares.push({ + pluginId: "tokenjuice", + pluginName: "tokenjuice", + rawHandler: () => undefined, + handler: (event) => { + seenToolCallIds.push(event.toolCallId); + event.result.content = [{ type: "text", text: `compacted ${seenToolCallIds.length}` }]; + return undefined; + }, + harnesses: ["pi"], + source: "test", + }); + setActivePluginRegistry(registry); + + const factories = buildEmbeddedExtensionFactories({ + cfg: undefined, + sessionManager: SessionManager.inMemory(), + provider: "openai", + modelId: "gpt-5.4", + model: undefined, + }); + expect(factories).toHaveLength(1); + + const handlers = new Map(); + await factories[0]?.({ + on(event: string, handler: Function) { + handlers.set(event, handler); + }, + } as never); + const handler = handlers.get("tool_result"); + + const first = await handler?.( + { toolName: "exec", content: [{ type: "text", text: "raw 1" }], details: {} }, + { cwd: "/tmp" }, + ); + const second = await handler?.( + { toolName: "exec", content: [{ type: "text", text: "raw 2" }], details: {} }, + { cwd: "/tmp" }, + ); + + expect(first).toEqual({ + content: [{ type: "text", text: "compacted 1" }], + details: {}, + }); + expect(second).toEqual({ + content: [{ type: "text", text: "compacted 2" }], + details: {}, + }); + expect(seenToolCallIds).toHaveLength(2); + expect(seenToolCallIds[0]).toMatch(/^pi-/); + expect(seenToolCallIds[1]).toMatch(/^pi-/); + expect(seenToolCallIds[0]).not.toBe(seenToolCallIds[1]); + }); + it("includes plugin-registered embedded extension factories and restores them from cache", async () => { const tmp = createTempDir(); process.env.OPENCLAW_BUNDLED_PLUGINS_DIR = tmp; diff --git a/src/agents/pi-embedded-runner/extensions.ts b/src/agents/pi-embedded-runner/extensions.ts index a9b43b758bd..8cbfecb7219 100644 --- a/src/agents/pi-embedded-runner/extensions.ts +++ b/src/agents/pi-embedded-runner/extensions.ts @@ -1,6 +1,8 @@ +import { randomUUID } from "node:crypto"; import type { AgentToolResult } from "@mariozechner/pi-agent-core"; import type { ExtensionFactory, SessionManager } from "@mariozechner/pi-coding-agent"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; +import { listAgentToolResultMiddlewares } from "../../plugins/agent-tool-result-middleware.js"; import { listEmbeddedExtensionFactories } from "../../plugins/embedded-extension-factory.js"; import type { ProviderRuntimeModel } from "../../plugins/provider-runtime-model.types.js"; import { resolveContextWindowInfo } from "../context-window-guard.js"; @@ -34,13 +36,21 @@ function recordFromUnknown(value: unknown): Record { } function buildAgentToolResultMiddlewareFactory(): ExtensionFactory { - const runner = createAgentToolResultMiddlewareRunner({ harness: "pi" }); + const handlers = listAgentToolResultMiddlewares("pi"); + const runner = createAgentToolResultMiddlewareRunner({ harness: "pi" }, handlers); return (pi) => { pi.on("tool_result", async (rawEvent: unknown, ctx: { cwd?: string }) => { + if (handlers.length === 0) { + return undefined; + } const event = recordFromUnknown(rawEvent) as PiToolResultEvent; if (!event.toolName) { return undefined; } + const toolCallId = + typeof event.toolCallId === "string" && event.toolCallId.trim() + ? event.toolCallId + : `pi-${randomUUID()}`; const content = Array.isArray(event.content) ? event.content : []; const current = { content, @@ -49,16 +59,13 @@ function buildAgentToolResultMiddlewareFactory(): ExtensionFactory { const result = await runner.applyToolResultMiddleware({ threadId: event.threadId, turnId: event.turnId, - toolCallId: event.toolCallId ?? event.toolName, + toolCallId, toolName: event.toolName, args: recordFromUnknown(event.input), cwd: ctx.cwd, isError: event.isError, result: current, }); - if (result === current) { - return undefined; - } return { content: result.content, details: result.details, diff --git a/src/plugins/captured-registration.test.ts b/src/plugins/captured-registration.test.ts index 0352f47148a..924a8e19ea0 100644 --- a/src/plugins/captured-registration.test.ts +++ b/src/plugins/captured-registration.test.ts @@ -46,6 +46,9 @@ describe("captured plugin registration", () => { description: "Captured command", handler: async () => ({ text: "ok" }), }); + api.registerAgentToolResultMiddleware(() => undefined, { + harnesses: ["codex-app-server"], + }); }, }); @@ -53,6 +56,8 @@ describe("captured plugin registration", () => { expect(captured.providers.map((provider) => provider.id)).toEqual(["captured-provider"]); expect(captured.textTransforms).toHaveLength(1); expect(captured.textTransforms[0]?.input).toHaveLength(1); + expect(captured.agentToolResultMiddlewares).toHaveLength(1); + expect(captured.agentToolResultMiddlewares[0]?.harnesses).toEqual(["codex-app-server"]); expect(captured.api.registerMemoryEmbeddingProvider).toBeTypeOf("function"); }); }); diff --git a/src/plugins/captured-registration.ts b/src/plugins/captured-registration.ts index 54fe1881b73..e5558121d43 100644 --- a/src/plugins/captured-registration.ts +++ b/src/plugins/captured-registration.ts @@ -1,9 +1,14 @@ import type { ExtensionFactory } from "@mariozechner/pi-coding-agent"; import type { OpenClawConfig } from "../config/types.openclaw.js"; -import type { AgentToolResultMiddleware } from "./agent-tool-result-middleware-types.js"; +import type { + AgentToolResultMiddleware, + AgentToolResultMiddlewareOptions, +} from "./agent-tool-result-middleware-types.js"; +import { normalizeAgentToolResultMiddlewareHarnesses } from "./agent-tool-result-middleware.js"; import { buildPluginApi } from "./api-builder.js"; import type { CodexAppServerExtensionFactory } from "./codex-app-server-extension-types.js"; import type { MemoryEmbeddingProviderAdapter } from "./memory-embedding-providers.js"; +import type { PluginAgentToolResultMiddlewareRegistration } from "./registry-types.js"; import type { PluginRuntime } from "./runtime/types.js"; import type { AnyAgentTool, @@ -40,7 +45,7 @@ export type CapturedPluginRegistration = { textTransforms: PluginTextTransformRegistration[]; embeddedExtensionFactories: ExtensionFactory[]; codexAppServerExtensionFactories: CodexAppServerExtensionFactory[]; - agentToolResultMiddlewares: AgentToolResultMiddleware[]; + agentToolResultMiddlewares: PluginAgentToolResultMiddlewareRegistration[]; speechProviders: SpeechProviderPlugin[]; realtimeTranscriptionProviders: RealtimeTranscriptionProviderPlugin[]; realtimeVoiceProviders: RealtimeVoiceProviderPlugin[]; @@ -65,7 +70,7 @@ export function createCapturedPluginRegistration(params?: { const textTransforms: PluginTextTransformRegistration[] = []; const embeddedExtensionFactories: ExtensionFactory[] = []; const codexAppServerExtensionFactories: CodexAppServerExtensionFactory[] = []; - const agentToolResultMiddlewares: AgentToolResultMiddleware[] = []; + const agentToolResultMiddlewares: PluginAgentToolResultMiddlewareRegistration[] = []; const speechProviders: SpeechProviderPlugin[] = []; const realtimeTranscriptionProviders: RealtimeTranscriptionProviderPlugin[] = []; const realtimeVoiceProviders: RealtimeVoiceProviderPlugin[] = []; @@ -149,8 +154,19 @@ export function createCapturedPluginRegistration(params?: { registerCodexAppServerExtensionFactory(factory: CodexAppServerExtensionFactory) { codexAppServerExtensionFactories.push(factory); }, - registerAgentToolResultMiddleware(handler: AgentToolResultMiddleware) { - agentToolResultMiddlewares.push(handler); + registerAgentToolResultMiddleware( + handler: AgentToolResultMiddleware, + options?: AgentToolResultMiddlewareOptions, + ) { + const harnesses = normalizeAgentToolResultMiddlewareHarnesses(options); + agentToolResultMiddlewares.push({ + pluginId: "captured-plugin-registration", + pluginName: "Captured Plugin Registration", + rawHandler: handler, + handler, + harnesses, + source: "captured-plugin-registration", + }); }, registerCliBackend(backend: CliBackendPlugin) { cliBackends.push(backend); diff --git a/src/plugins/registry.ts b/src/plugins/registry.ts index ac53d0ccdb5..b0f391e3ae6 100644 --- a/src/plugins/registry.ts +++ b/src/plugins/registry.ts @@ -336,6 +336,15 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) { handler: Parameters[0], options: Parameters[1], ) => { + if (record.origin !== "bundled") { + pushDiagnostic({ + level: "error", + pluginId: record.id, + source: record.source, + message: "only bundled plugins can register agent tool result middleware", + }); + return; + } if (typeof (handler as unknown) !== "function") { pushDiagnostic({ level: "error", @@ -377,11 +386,10 @@ export function createPluginRegistry(registryParams: PluginRegistryParams) { try { return await handler(event, ctx); } catch (error) { - const detail = error instanceof Error ? error.message : String(error); registryParams.logger.warn( - `[plugins] agent tool result middleware failed for ${record.id}: ${detail}`, + `[plugins] agent tool result middleware failed for ${record.id}`, ); - return undefined; + throw error; } }; registry.agentToolResultMiddlewares.push({