From f04cc0fa9cb6b587711159aed1c2dfca14831775 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Wed, 25 Mar 2026 23:23:46 +0900 Subject: [PATCH] fix(thinking-block-validator): replace model-name gating with content-based history detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace isExtendedThinkingModel() model-name check with hasSignedThinkingBlocksInHistory() which scans message history for real Anthropic-signed thinking blocks. Content-based gating is more robust than model-name checks — works correctly with custom model IDs, proxied models, and new model releases without code changes. - Add isSignedThinkingPart() that matches type thinking/redacted_thinking with valid signature - Skip synthetic parts (injected by previous hook runs) - GPT reasoning blocks (type=reasoning, no signature) correctly excluded - Add comprehensive tests: signed injection, redacted_thinking, reasoning negative case, synthetic skip Inspired by PR #2653 content-based approach, combined with redacted_thinking support from 0732cb85. Ultraworked with Sisyphus Co-authored-by: Sisyphus --- .../thinking-block-validator/hook.test.ts | 222 ++++++++++++------ src/hooks/thinking-block-validator/hook.ts | 120 ++++++---- 2 files changed, 219 insertions(+), 123 deletions(-) diff --git a/src/hooks/thinking-block-validator/hook.test.ts b/src/hooks/thinking-block-validator/hook.test.ts index 9eab6dff6..0601cbcfc 100644 --- a/src/hooks/thinking-block-validator/hook.test.ts +++ b/src/hooks/thinking-block-validator/hook.test.ts @@ -1,108 +1,184 @@ -const { describe, expect, test } = require("bun:test") +declare const describe: (name: string, fn: () => void) => void +declare const it: (name: string, fn: () => void | Promise) => void +declare const expect: (value: T) => { + toBe(expected: T): void + toEqual(expected: unknown): void + toHaveLength(expected: number): void +} -const { createThinkingBlockValidatorHook } = require("./hook") +import { createThinkingBlockValidatorHook } from "./hook" type TestPart = { type: string - id: string text?: string thinking?: string - data?: string signature?: string + synthetic?: boolean } type TestMessage = { - info: { - role: string - id?: string - modelID?: string - } + info: { role: "assistant" | "user" } parts: TestPart[] } -function createMessage(info: TestMessage["info"], parts: TestPart[]): TestMessage { - return { info, parts } -} +async function runTransform(messages: TestMessage[]): Promise { + const hook = createThinkingBlockValidatorHook() + const transform = hook["experimental.chat.messages.transform"] -function createTextPart(id: string, text: string): TestPart { - return { type: "text", id, text } -} + if (!transform) { + throw new Error("missing thinking block validator transform") + } -function createSignedThinkingPart(id: string, thinking: string, signature: string): TestPart { - return { type: "thinking", id, thinking, signature } -} - -function createRedactedThinkingPart(id: string, signature: string): TestPart { - return { type: "redacted_thinking", id, data: "encrypted", signature } + await transform({}, { messages: messages as never }) } describe("createThinkingBlockValidatorHook", () => { - test("reuses the previous signed thinking part verbatim when assistant content lacks a leading thinking block", async () => { - const transform = Reflect.get(createThinkingBlockValidatorHook(), "experimental.chat.messages.transform") - expect(typeof transform).toBe("function") + it("injects signed thinking history verbatim", async () => { + //#given + const signedThinkingPart: TestPart = { + type: "thinking", + thinking: "plan", + signature: "signed-thinking", + } + const messages = [ + { + info: { role: "assistant" }, + parts: [signedThinkingPart], + }, + { + info: { role: "assistant" }, + parts: [{ type: "text", text: "continue" }], + }, + ] satisfies TestMessage[] - const previousThinkingPart = createSignedThinkingPart("prt_prev_signed", "prior reasoning", "sig_prev") - const targetTextPart = createTextPart("prt_target_text", "tool result") - const messages: TestMessage[] = [ - createMessage({ role: "user", modelID: "claude-opus-4-6-thinking" }, [createTextPart("prt_user_text", "continue")]), - createMessage({ role: "assistant", id: "msg_prev" }, [previousThinkingPart, createTextPart("prt_prev_text", "done")]), - createMessage({ role: "assistant", id: "msg_target" }, [targetTextPart]), - ] + //#when + await runTransform(messages) - await Reflect.apply(transform, undefined, [{}, { messages }]) - - expect(messages[2]?.parts[0]).toBe(previousThinkingPart) - expect(messages[2]?.parts).toEqual([previousThinkingPart, targetTextPart]) + //#then + expect(messages[1]?.parts[0]).toBe(signedThinkingPart) }) - test("skips injection when no signed Anthropic thinking part exists in history", async () => { - const transform = Reflect.get(createThinkingBlockValidatorHook(), "experimental.chat.messages.transform") - expect(typeof transform).toBe("function") + it("injects signed redacted_thinking history verbatim", async () => { + //#given + const signedRedactedThinkingPart: TestPart = { + type: "redacted_thinking", + signature: "signed-redacted-thinking", + } + const messages = [ + { + info: { role: "assistant" }, + parts: [signedRedactedThinkingPart], + }, + { + info: { role: "assistant" }, + parts: [{ type: "tool_use" }], + }, + ] satisfies TestMessage[] - const targetTextPart = createTextPart("prt_target_text", "tool result") - const messages: TestMessage[] = [ - createMessage({ role: "user", modelID: "claude-opus-4-6-thinking" }, [createTextPart("prt_user_text", "continue")]), - createMessage({ role: "assistant", id: "msg_prev" }, [{ type: "reasoning", id: "prt_reason", text: "gpt reasoning" }]), - createMessage({ role: "assistant", id: "msg_target" }, [targetTextPart]), - ] + //#when + await runTransform(messages) - await Reflect.apply(transform, undefined, [{}, { messages }]) - - expect(messages[2]?.parts).toEqual([targetTextPart]) + //#then + expect(messages[1]?.parts[0]).toBe(signedRedactedThinkingPart) }) - test("does not inject when the assistant message already starts with redacted thinking", async () => { - const transform = Reflect.get(createThinkingBlockValidatorHook(), "experimental.chat.messages.transform") - expect(typeof transform).toBe("function") + it("skips hook when history contains reasoning only", async () => { + //#given + const reasoningPart: TestPart = { + type: "reasoning", + text: "internal reasoning", + } + const messages = [ + { + info: { role: "assistant" }, + parts: [reasoningPart], + }, + { + info: { role: "assistant" }, + parts: [{ type: "text", text: "continue" }], + }, + ] satisfies TestMessage[] - const existingThinkingPart = createRedactedThinkingPart("prt_redacted", "sig_redacted") - const targetTextPart = createTextPart("prt_target_text", "tool result") - const messages: TestMessage[] = [ - createMessage({ role: "user", modelID: "claude-opus-4-6-thinking" }, [createTextPart("prt_user_text", "continue")]), - createMessage({ role: "assistant", id: "msg_target" }, [existingThinkingPart, targetTextPart]), - ] + //#when + await runTransform(messages) - await Reflect.apply(transform, undefined, [{}, { messages }]) - - expect(messages[1]?.parts).toEqual([existingThinkingPart, targetTextPart]) + //#then + expect(messages[1]?.parts).toEqual([{ type: "text", text: "continue" }]) }) - test("skips processing for models without extended thinking", async () => { - const transform = Reflect.get(createThinkingBlockValidatorHook(), "experimental.chat.messages.transform") - expect(typeof transform).toBe("function") + it("skips hook when no signed history exists", async () => { + //#given + const messages = [ + { + info: { role: "assistant" }, + parts: [{ type: "thinking", thinking: "draft" }], + }, + { + info: { role: "assistant" }, + parts: [{ type: "text", text: "continue" }], + }, + ] satisfies TestMessage[] - const previousThinkingPart = createSignedThinkingPart("prt_prev_signed", "prior reasoning", "sig_prev") - const targetTextPart = createTextPart("prt_target_text", "tool result") - const messages: TestMessage[] = [ - createMessage({ role: "user", modelID: "gpt-5.4" }, [createTextPart("prt_user_text", "continue")]), - createMessage({ role: "assistant", id: "msg_prev" }, [previousThinkingPart]), - createMessage({ role: "assistant", id: "msg_target" }, [targetTextPart]), - ] + //#when + await runTransform(messages) - await Reflect.apply(transform, undefined, [{}, { messages }]) + //#then + expect(messages[1]?.parts).toEqual([{ type: "text", text: "continue" }]) + }) - expect(messages[2]?.parts).toEqual([targetTextPart]) + it("skips hook when history contains synthetic signed blocks only", async () => { + //#given + const syntheticSignedPart: TestPart = { + type: "thinking", + thinking: "synthetic", + signature: "synthetic-signature", + synthetic: true, + } + const messages = [ + { + info: { role: "assistant" }, + parts: [syntheticSignedPart], + }, + { + info: { role: "assistant" }, + parts: [{ type: "text", text: "continue" }], + }, + ] satisfies TestMessage[] + + //#when + await runTransform(messages) + + //#then + expect(messages[1]?.parts).toEqual([{ type: "text", text: "continue" }]) + }) + + it("does not reinject when the message already starts with redacted_thinking", async () => { + //#given + const signedThinkingPart: TestPart = { + type: "thinking", + thinking: "plan", + signature: "signed-thinking", + } + const leadingRedactedThinkingPart: TestPart = { + type: "redacted_thinking", + signature: "existing-redacted-thinking", + } + const messages = [ + { + info: { role: "assistant" }, + parts: [signedThinkingPart], + }, + { + info: { role: "assistant" }, + parts: [leadingRedactedThinkingPart, { type: "text", text: "continue" }], + }, + ] satisfies TestMessage[] + + //#when + await runTransform(messages) + + //#then + expect(messages[1]?.parts[0]).toBe(leadingRedactedThinkingPart) + expect(messages[1]?.parts).toHaveLength(2) }) }) - -export {} diff --git a/src/hooks/thinking-block-validator/hook.ts b/src/hooks/thinking-block-validator/hook.ts index 67b6c12c0..544d8e672 100644 --- a/src/hooks/thinking-block-validator/hook.ts +++ b/src/hooks/thinking-block-validator/hook.ts @@ -21,11 +21,6 @@ interface MessageWithParts { parts: Part[] } -type SignedThinkingPart = Part & { - type: "thinking" | "redacted_thinking" - signature: string -} - type MessagesTransformHook = { "experimental.chat.messages.transform"?: ( input: Record, @@ -33,25 +28,39 @@ type MessagesTransformHook = { ) => Promise } -/** - * Check if a model has extended thinking enabled - * Uses patterns from think-mode/switcher.ts for consistency - */ -function isExtendedThinkingModel(modelID: string): boolean { - if (!modelID) return false - const lower = modelID.toLowerCase() +type SignedThinkingPart = Part & { + type: "thinking" | "redacted_thinking" + thinking?: string + signature: string + synthetic?: boolean +} - // Check for explicit thinking/high variants (always enabled) - if (lower.includes("thinking") || lower.endsWith("-high")) { - return true +function isSignedThinkingPart(part: Part): part is SignedThinkingPart { + const type = part.type as string + if (type !== "thinking" && type !== "redacted_thinking") { + return false } - // Check for thinking-capable models (claude-4 family, claude-3) - // Aligns with THINKING_CAPABLE_MODELS in think-mode/switcher.ts - return ( - lower.includes("claude-sonnet-4") || - lower.includes("claude-opus-4") || - lower.includes("claude-3") + const signature = (part as { signature?: unknown }).signature + const synthetic = (part as { synthetic?: unknown }).synthetic + return typeof signature === "string" && signature.length > 0 && synthetic !== true +} + +/** + * Check if there are any Anthropic-signed thinking blocks in the message history. + * + * Only returns true for real `type: "thinking"` blocks with a valid `signature`. + * GPT reasoning blocks (`type: "reasoning"`) are intentionally excluded — they + * have no Anthropic signature and must never be forwarded to the Anthropic API. + * + * Model-name checks are unreliable (miss GPT+thinking, custom model IDs, etc.) + * so we inspect the messages themselves. + */ +function hasSignedThinkingBlocksInHistory(messages: MessageWithParts[]): boolean { + return messages.some( + m => + m.info.role === "assistant" && + m.parts?.some((p: Part) => isSignedThinkingPart(p)), ) } @@ -79,36 +88,42 @@ function startsWithThinkingBlock(parts: Part[]): boolean { return type === "thinking" || type === "redacted_thinking" || type === "reasoning" } -function isSignedThinkingPart(part: Part): part is SignedThinkingPart { - const type = part.type as string - if (type !== "thinking" && type !== "redacted_thinking") { - return false - } - - const signature = (part as { signature?: unknown }).signature - return typeof signature === "string" && signature.length > 0 -} - -function findPreviousThinkingPart( - messages: MessageWithParts[], - currentIndex: number -): SignedThinkingPart | null { +/** + * Find the most recent Anthropic-signed thinking part from previous assistant messages. + * + * Returns the original Part object (including its `signature` field) so it can + * be reused verbatim in another message. Only `type: "thinking"` blocks with + * both a `signature` and `thinking` field are returned — GPT `type: "reasoning"` + * blocks are excluded because they lack an Anthropic signature and would be + * rejected by the API with "Invalid `signature` in `thinking` block". + * Synthetic parts injected by a previous run of this hook are also skipped. + */ +function findPreviousThinkingPart(messages: MessageWithParts[], currentIndex: number): SignedThinkingPart | null { // Search backwards from current message for (let i = currentIndex - 1; i >= 0; i--) { const msg = messages[i] if (msg.info.role !== "assistant") continue - if (!msg.parts) continue + for (const part of msg.parts) { - if (isSignedThinkingPart(part)) { - return part - } + // Only Anthropic thinking blocks — type must be "thinking", not "reasoning" + if (!isSignedThinkingPart(part)) continue + + return part } } return null } +/** + * Prepend an existing thinking block (with its original signature) to a + * message's parts array. + * + * We reuse the original Part verbatim instead of creating a new one, because + * the Anthropic API validates the `signature` field against the thinking + * content. Any synthetic block we create ourselves would fail that check. + */ function prependThinkingBlock(message: MessageWithParts, thinkingPart: SignedThinkingPart): void { if (!message.parts) { message.parts = [] @@ -129,13 +144,12 @@ export function createThinkingBlockValidatorHook(): MessagesTransformHook { return } - // Get the model info from the last user message - const lastUserMessage = messages.findLast(m => m.info.role === "user") - const modelIDValue = (lastUserMessage?.info as { modelID?: unknown } | undefined)?.modelID - const modelID = typeof modelIDValue === "string" ? modelIDValue : "" - - // Only process if extended thinking might be enabled - if (!isExtendedThinkingModel(modelID)) { + // Skip if there are no Anthropic-signed thinking blocks in history. + // This is more reliable than checking model names — works for Claude, + // GPT with thinking variants, or any future model. Crucially, GPT + // reasoning blocks (type="reasoning", no signature) do NOT trigger this + // hook — only real Anthropic thinking blocks do. + if (!hasSignedThinkingBlocksInHistory(messages)) { return } @@ -148,12 +162,18 @@ export function createThinkingBlockValidatorHook(): MessagesTransformHook { // Check if message has content parts but doesn't start with thinking if (hasContentParts(msg.parts) && !startsWithThinkingBlock(msg.parts)) { + // Find the most recent real thinking part (with valid signature) from + // previous turns. If none exists we cannot safely inject a thinking + // block — a synthetic block without a signature would cause the API + // to reject the request with "Invalid `signature` in `thinking` block". const previousThinkingPart = findPreviousThinkingPart(messages, i) - if (!previousThinkingPart) { - continue - } - prependThinkingBlock(msg, previousThinkingPart) + if (previousThinkingPart) { + prependThinkingBlock(msg, previousThinkingPart) + } + // If no real thinking part is available, skip injection entirely. + // The downstream error (if any) is preferable to a guaranteed API + // rejection caused by a signature-less synthetic thinking block. } } },