From 69a4b2f49c3ad79313fa416903ee476c10846358 Mon Sep 17 00:00:00 2001 From: mrosnerr Date: Tue, 28 Apr 2026 14:51:38 -0400 Subject: [PATCH] fix(messages-transform): isolate hook failures so tool-pair-validator always runs Previously each transform hook was awaited sequentially without per-hook error handling. If contextInjectorMessagesTransform or thinkingBlockValidator threw, toolPairValidator was silently skipped, leaving orphaned tool_use blocks in the post-compaction API payload and producing "messages.N: tool_use ids were found without tool_result blocks immediately after" 400s from Anthropic. Wraps each hook in runHookSafely so an upstream throw is logged but the chain continues. Adds regression tests covering the isolation contract and the consecutive-assistants compaction tail case (ses_22bd806). --- src/plugin/messages-transform.test.ts | 167 ++++++++++++++++++++++++++ src/plugin/messages-transform.ts | 55 +++++++-- 2 files changed, 213 insertions(+), 9 deletions(-) create mode 100644 src/plugin/messages-transform.test.ts diff --git a/src/plugin/messages-transform.test.ts b/src/plugin/messages-transform.test.ts new file mode 100644 index 000000000..d3c0d6315 --- /dev/null +++ b/src/plugin/messages-transform.test.ts @@ -0,0 +1,167 @@ +import { describe, it, expect } from "bun:test" + +import { createMessagesTransformHandler } from "./messages-transform" +import { createToolPairValidatorHook } from "../hooks/tool-pair-validator/hook" +import type { CreatedHooks } from "../create-hooks" + +type TestPart = { + type: string + id?: string + callID?: string + tool_use_id?: string + content?: string + text?: string +} + +type TestMessage = { + info: { role: "assistant" | "user" } + parts: TestPart[] +} + +type TransformHook = ( + input: Record, + output: { messages: TestMessage[] }, +) => Promise + +function makeHook(handler: TransformHook): NonNullable { + return { + "experimental.chat.messages.transform": handler as never, + } as never +} + +function makeHooks(overrides: { + contextInjector?: TransformHook + thinkingBlock?: TransformHook + toolPair?: TransformHook +}): CreatedHooks { + return { + contextInjectorMessagesTransform: overrides.contextInjector ? makeHook(overrides.contextInjector) : undefined, + thinkingBlockValidator: overrides.thinkingBlock ? makeHook(overrides.thinkingBlock) : undefined, + toolPairValidator: overrides.toolPair ? makeHook(overrides.toolPair) : undefined, + } as unknown as CreatedHooks +} + +async function runHandler( + hooks: CreatedHooks, + messages: TestMessage[], +): Promise { + const handler = createMessagesTransformHandler({ hooks }) + await handler({} as never, { messages: messages as never }) +} + +describe("createMessagesTransformHandler", () => { + it("runs all hooks in order when none throw", async () => { + //#given + const callOrder: string[] = [] + const hooks = makeHooks({ + contextInjector: async () => { + callOrder.push("context-injector") + }, + thinkingBlock: async () => { + callOrder.push("thinking-block-validator") + }, + toolPair: async () => { + callOrder.push("tool-pair-validator") + }, + }) + + //#when + await runHandler(hooks, []) + + //#then + expect(callOrder).toEqual([ + "context-injector", + "thinking-block-validator", + "tool-pair-validator", + ]) + }) + + it("runs tool-pair-validator even when context-injector throws", async () => { + //#given + let toolPairRan = false + const hooks = makeHooks({ + contextInjector: async () => { + throw new Error("context-injector boom") + }, + toolPair: async () => { + toolPairRan = true + }, + }) + + //#when + await runHandler(hooks, []) + + //#then + expect(toolPairRan).toBe(true) + }) + + it("runs tool-pair-validator even when thinking-block-validator throws", async () => { + //#given + let toolPairRan = false + const hooks = makeHooks({ + thinkingBlock: async () => { + throw new Error("thinking-block boom") + }, + toolPair: async () => { + toolPairRan = true + }, + }) + + //#when + await runHandler(hooks, []) + + //#then + expect(toolPairRan).toBe(true) + }) + + it("repairs orphaned tool_use after upstream hook throws (regression for ses_22bd806)", async () => { + //#given + const messages: TestMessage[] = [ + { info: { role: "user" }, parts: [{ type: "text", text: "summary stand-in" }] }, + { info: { role: "assistant" }, parts: [{ type: "tool_use", id: "toolu_01SRMQs3DUtVKWoSxC8bxxVA" }] }, + { info: { role: "assistant" }, parts: [{ type: "tool_use", id: "toolu_01Lu5cHvRtEvzoifP1UVBVRb" }] }, + { info: { role: "user" }, parts: [{ type: "text", text: "next" }] }, + ] + const hooks = makeHooks({ + contextInjector: async () => { + throw new Error("simulating upstream hook failure") + }, + toolPair: createRealToolPairValidator(), + }) + + //#when + await runHandler(hooks, messages) + + //#then + expect(messages).toHaveLength(5) + expect(messages[2]).toEqual({ + info: { role: "user" }, + parts: [{ type: "tool_result", tool_use_id: "toolu_01SRMQs3DUtVKWoSxC8bxxVA", content: "Tool output unavailable (context compacted)" }], + }) + expect(messages[4]?.parts[0]).toEqual({ + type: "tool_result", + tool_use_id: "toolu_01Lu5cHvRtEvzoifP1UVBVRb", + content: "Tool output unavailable (context compacted)", + }) + expect(messages[4]?.parts[1]).toEqual({ type: "text", text: "next" }) + }) + + it("does not throw when tool-pair-validator itself fails", async () => { + //#given + const hooks = makeHooks({ + toolPair: async () => { + throw new Error("validator boom") + }, + }) + + //#when / #then + await runHandler(hooks, []) + }) +}) + +function createRealToolPairValidator(): TransformHook { + const validator = createToolPairValidatorHook() + const handler = validator["experimental.chat.messages.transform"] + if (!handler) throw new Error("validator missing transform") + return handler as never +} diff --git a/src/plugin/messages-transform.ts b/src/plugin/messages-transform.ts index cd28b3832..99f926cfb 100644 --- a/src/plugin/messages-transform.ts +++ b/src/plugin/messages-transform.ts @@ -1,5 +1,6 @@ import type { Message, Part } from "@opencode-ai/sdk" +import { log } from "../shared/logger" import type { CreatedHooks } from "../create-hooks" type MessageWithParts = { @@ -9,20 +10,56 @@ type MessageWithParts = { type MessagesTransformOutput = { messages: MessageWithParts[] } +async function runMessagesTransformHookSafely( + hookName: string, + handler: ((input: I, output: O) => unknown | Promise) | null | undefined, + input: I, + output: O, +): Promise { + if (!handler) return + try { + await Promise.resolve(handler(input, output)) + } catch (error) { + // Isolate per-handler failures so later handlers (notably toolPairValidator) + // always run. A throw here used to leave orphaned tool_use blocks in the + // post-compaction payload, producing API 400s like + // "tool_use ids were found without tool_result blocks immediately after". + log("[messages-transform] hook execution failed", { + hook: hookName, + error, + }) + } +} + export function createMessagesTransformHandler(args: { hooks: CreatedHooks }): (input: Record, output: MessagesTransformOutput) => Promise { return async (input, output): Promise => { - await args.hooks.contextInjectorMessagesTransform?.[ - "experimental.chat.messages.transform" - ]?.(input, output) + await runMessagesTransformHookSafely( + "contextInjectorMessagesTransform", + args.hooks.contextInjectorMessagesTransform?.[ + "experimental.chat.messages.transform" + ], + input, + output, + ) - await args.hooks.thinkingBlockValidator?.[ - "experimental.chat.messages.transform" - ]?.(input, output) + await runMessagesTransformHookSafely( + "thinkingBlockValidator", + args.hooks.thinkingBlockValidator?.[ + "experimental.chat.messages.transform" + ], + input, + output, + ) - await args.hooks.toolPairValidator?.[ - "experimental.chat.messages.transform" - ]?.(input, output) + await runMessagesTransformHookSafely( + "toolPairValidator", + args.hooks.toolPairValidator?.[ + "experimental.chat.messages.transform" + ], + input, + output, + ) } }