Repository navigation
🩹 fix: Drop Foreign Reasoning Blocks in Anthropic Message Converter - #243
Conversation
Prevents "Unsupported message content format" crashes on cross-provider
agent handoffs. When an agent on Bedrock Anthropic (extended thinking) hands
off to an agent on the official Anthropic provider, Bedrock leaves a
`reasoning_content` content block ({ reasoningText: { text, signature } }) in
history. The Anthropic converter (`_formatContent`) had no branch for it and
threw in the final else, crashing the run. Only hits the streaming path
(LibreChat agents always stream).
- Drop foreign reasoning blocks (Bedrock `reasoning_content`, Google
`reasoning`, LibreChat `think`); their provider-specific signatures cannot
be validated by Anthropic, so the receiving model produces its own thinking.
- Degrade gracefully on any unknown block: log and drop instead of throwing,
so a single unrecognized block can never crash an entire run.
- Add regression tests covering each reasoning family, the unknown-block
fallback, and the reasoning-only placeholder case.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1ee65c451
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| JSON.stringify(contentPart, null, 2) | ||
| ); | ||
| throw new Error('Unsupported message content format'); | ||
| return null; |
There was a problem hiding this comment.
This fallback runs for every message role, not just cross-provider AI histories. For a user prompt that contains an unsupported block that LibreChat already carries through as media, such as videos/audios from formatMessage (src/messages/format.ts:208-220), the converter now removes the block and sends only the remaining text or the _ placeholder instead of failing. That makes Anthropic answer as if the user-provided media never existed, so please limit the graceful drop to known foreign reasoning blocks or keep throwing for unsupported user content.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4b5b903. You're right — the graceful drop applied to every role. The fallback is now role-aware: assistant turns drop an unknown block (so a cross-provider handoff never crashes on a provider artifact), but user/tool turns still throw Unsupported message content format, so unsupported user media (e.g. a video_url block on a prompt) is surfaced rather than silently omitted. Foreign reasoning blocks (reasoning_content/reasoning/think) are still dropped regardless of role since they're assistant-internal. Regression test added: on a user turn, throws on an unsupported block rather than silently dropping it.
| input: functionCallPart.functionCall.args, | ||
| }; | ||
| } else if (foreignReasoningTypes.some((t) => t === contentPart.type)) { | ||
| return null; |
There was a problem hiding this comment.
When a Bedrock assistant turn contains reasoningContent plus a toolUse, convertConverseMessageToLangChainMessage records the tool only in message.tool_calls and does not add a tool_use content block (src/llm/bedrock/utils/message_outputs.ts:172-200). Dropping the only content block here turns that assistant message into the _ placeholder; the surrounding converter only materializes message.tool_calls when message.content is a string, so this Bedrock extended-thinking handoff now silently omits the transfer/tool call instead of preserving it. Please append unmatched tool_calls when reasoning filtering leaves no corresponding tool_use block.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4b5b903. Confirmed exactly as described: convertConverseMessageToLangChainMessage records the tool only on message.tool_calls (message_outputs.ts:181) with content holding just the reasoning block, and the array-content path of _convertMessagesToAnthropicPayload only console.warn'd instead of materializing tool_calls — so dropping the reasoning block left the _ placeholder and silently lost the transfer tool call. The array-content path now materializes any client tool_calls not already represented as a tool_use/server_tool_use/input_json_delta block, and strips the lone _ placeholder when real tool_use blocks are present. Regression test added: preserves a tool call carried only on tool_calls when its reasoning sibling is dropped.
Finding 1: the graceful drop ran for every message role, silently omitting unsupported user content (e.g. video/audio media on a user prompt). Now only assistant turns drop an unknown block; user/tool turns still throw so real input is never silently dropped. Finding 2: a Bedrock extended-thinking turn records the tool only on `message.tool_calls` and leaves `content` as just the reasoning block (no `tool_use`). Dropping that reasoning block left the `_` placeholder, and the array-content path never materialized `tool_calls` — silently losing the (handoff) tool call. Now unrepresented client tool_calls are materialized as tool_use blocks (and the lone placeholder is dropped when real tool_use is present). Adds regression tests for both.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b5b9037ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| (contentPart) => | ||
| (contentPart.type === 'tool_use' || | ||
| contentPart.type === 'input_json_delta' || | ||
| contentPart.type === 'server_tool_use') && | ||
| contentPart.id === toolCall.id | ||
| contentPart.type === 'tool_use' || | ||
| contentPart.type === 'input_json_delta' || | ||
| contentPart.type === 'server_tool_use' | ||
| ) |
There was a problem hiding this comment.
When a Gemini/Google assistant turn contains a functionCall content part plus the matching message.tool_calls entry (the existing path handled by _formatContent below), this set does not mark that content part as represented because it only considers type-based tool blocks. _formatContent still converts the functionCall into a tool_use, and then the new unrepresentedToolCalls logic appends the same tool call again from tool_calls, producing duplicate Anthropic tool_use blocks with the same id/name before the single tool result; Google → Anthropic handoffs with tool calls can therefore be rejected or execute the same call twice.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2cc7761. You're right — representedToolIds was built from the raw content by type, so a functionCall part (which _formatContent converts into a tool_use) wasn't counted as represented, and the materialization then appended a duplicate tool_use with the same id. It's now derived from the formatted output, so any tool_use _formatContent already produced — including from functionCall — is recognized as represented and not re-appended. Regression test added: does not duplicate a Google functionCall tool call already materialized by _formatContent.
| 'Dropping unsupported assistant content part:', | ||
| JSON.stringify(contentPart, null, 2) | ||
| ); | ||
| return null; |
There was a problem hiding this comment.
When a Google/Vertex response includes code execution, mapGenerateContentResultToChatResult stores visible executableCode and codeExecutionResult parts in assistant content (src/llm/google/utils/common.ts lines 873-881). Those blocks hit this new assistant fallback and are returned as null, so a Google → Anthropic handoff can continue with the executed code/result silently omitted from the prompt rather than converted to text or rejected. That loses evidence the next model needs to answer correctly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2cc7761. Agreed — silently dropping executableCode/codeExecutionResult loses real content the next model needs. I removed the blanket assistant-side drop entirely: only the known foreign reasoning types (reasoning_content/reasoning/think) are dropped, and every other unknown block throws Unsupported message content format again (the pre-existing behavior) rather than being silently omitted — for all roles. This also subsumes the round-1 user-media concern uniformly. Regression test added: throws (not silently drops) on an unknown assistant block such as Google code execution.
Finding 3 (P1): `representedToolIds` was computed from raw content by type, so a Google `functionCall` part — which `_formatContent` converts into a `tool_use` — was not counted as represented. The materialization then appended the same tool call again from `tool_calls`, producing duplicate `tool_use` blocks with the same id. It is now derived from the formatted output, so anything `_formatContent` already materialized (incl. functionCall) counts as present. Finding 4 (P2): the blanket assistant-side drop silently discarded Google code-execution blocks (`executableCode`/`codeExecutionResult`), losing real content on a Google → Anthropic handoff. Reverted the blanket drop: only known foreign reasoning (`reasoning_content`/`reasoning`/`think`) is dropped; every other unknown block throws again rather than being silently omitted (this also resolves the round-1 user-media concern uniformly, for all roles).
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d12ce7f27
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| name: correspondingToolCall.name, | ||
| input: functionCallPart.functionCall.args, | ||
| }; | ||
| } else if (foreignReasoningTypes.some((t) => t === contentPart.type)) { |
There was a problem hiding this comment.
When a HumanMessage or ToolMessage content array contains a block whose type happens to be reasoning, reasoning_content, or think, this branch returns null before the unsupported-content error, silently omitting that user/tool input from the Anthropic request. The provider-specific-signature rationale only applies to assistant reasoning artifacts; non-assistant content should still be surfaced or rejected instead of dropped.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 90e5bf1. Agreed — the drop rationale (provider-specific signatures Anthropic can't validate) only applies to assistant artifacts. The foreign-reasoning drop is now gated on isAIMessage(message); a reasoning_content/reasoning/think block on a user or tool turn falls through to the throw and is surfaced rather than silently dropped. Regression test added: does not drop a reasoning-typed block on a user turn (only assistant reasoning is dropped).
Finding 5 (P2): the foreign-reasoning drop ran for every message role, so a `reasoning_content`/`reasoning`/`think` block on a user or tool message was silently omitted. The provider-specific-signature rationale only applies to assistant artifacts, so the drop is now gated on `isAIMessage`; those types on a non-assistant turn fall through to the throw and are surfaced, not dropped.
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Symmetric to the Anthropic-side fix, for the reverse direction. An Anthropic extended-thinking turn leaves `thinking`/`redacted_thinking` blocks in history; the Bedrock Converse converter (convertAIMessageToConverseMessage) had no branch for them and threw "Unsupported content block type: thinking", crashing an Anthropic → Bedrock handoff. Foreign reasoning (`thinking`/`redacted_thinking`/`reasoning`/`think`) is now dropped on assistant turns; Bedrock's native `reasoning_content` is still converted, and any other unknown block still throws rather than being silently omitted. This covers the streaming path (`_streamResponseChunks` -> convertToConverseMessages) that LibreChat agents use; verified live (real Anthropic thinking output -> Bedrock stream, no throw). Regression tests added. Note: the non-streaming `.invoke()` path delegates to base @langchain/aws, which still throws on foreign reasoning, but agent runs always stream.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8683726ec2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| type: 'default', | ||
| }, | ||
| } as BedrockContentBlock); | ||
| } else if (FOREIGN_REASONING_TYPES.some((t) => t === block.type)) { |
There was a problem hiding this comment.
When an assistant turn contains only foreign reasoning blocks and has no tool_calls, this branch drops every block and convertAIMessageToConverseMessage returns { role: 'assistant', content: [] }. Bedrock Converse rejects messages without any content blocks, so an Anthropic/Google → Bedrock handoff in that reasoning-only case now moves from a local conversion error to a service-side validation failure; mirror the Anthropic-side placeholder behavior or omit the empty assistant turn after filtering.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 90bba28. Right — a reasoning-only assistant turn with no tool_calls dropped to empty content, which Bedrock Converse rejects. convertAIMessageToConverseMessage now falls back to a placeholder text block when filtering leaves the assistant content empty (mirrors the Anthropic-side _ placeholder). Regression test added: emits a placeholder (not empty content) when a reasoning-only turn is fully dropped.
| * cross-provider handoff (e.g. Bedrock → Anthropic) we drop them rather than | ||
| * forwarding an unusable block. The receiving model produces its own thinking. | ||
| */ | ||
| const foreignReasoningTypes = ['reasoning_content', 'reasoning', 'think']; |
There was a problem hiding this comment.
thinking blocks as foreign reasoning
Google thinking-enabled responses are represented in AIMessage.content as { type: 'thinking', thinking: ... } (see the Google spec asserting block.type === 'thinking'), but this new foreign-reasoning filter only includes Google's reasoning contentBlocks shape. In a Google → Anthropic handoff those thinking blocks still take the Anthropic-native thinking branch and are forwarded without an Anthropic signature, so the request can still fail instead of being sanitized; distinguish signed Anthropic thinking from foreign/unsigned Google thinking before forwarding.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 90bba28. Good catch — Google thinking-enabled output uses type: 'thinking' with no Anthropic signature (Google's thought-signatures live on functionCall parts in additional_kwargs, not on thinking content blocks — confirmed at src/llm/google/llm.spec.ts:1314). The native thinking branch now drops unsigned thinking on assistant turns (foreign/Google, which Anthropic rejects) and forwards only signed Anthropic-native thinking. redacted_thinking (carries data, not a signature) is unaffected. Regression tests added: drops an unsigned thinking block (Google thinking-enabled output) and forwards a signed thinking block (Anthropic-native).
…ned thinking Finding 6 (P2, Bedrock): a reasoning-only assistant turn with no tool_calls had every block dropped, yielding `content: []` which Bedrock Converse rejects (moving a local conversion error to a service-side validation failure). It now falls back to a placeholder text block, mirroring the Anthropic side. Finding 7 (P2, Anthropic): Google thinking-enabled output reuses `type: 'thinking'` but carries no Anthropic signature, so it took the native thinking branch and was forwarded unsigned — which Anthropic rejects. The thinking branch now drops unsigned thinking on assistant turns (foreign/Google) and forwards only signed (Anthropic-native) thinking. `redacted_thinking` (which carries `data`, not a signature) is unaffected.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90bba28ad5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| content: _formatContent(message), | ||
| content: [ | ||
| ...existingBlocks, | ||
| ...unrepresentedToolCalls.map(_convertLangChainToolCallToAnthropic), |
There was a problem hiding this comment.
When the receiving Anthropic agent has thinking enabled, Graph runs ensureThinkingBlockInMessages before this converter, and that helper treats reasoning_content as a thinking block (src/messages/format.ts:1836-1839), so a historical Bedrock assistant reasoning_content + tool_calls turn is not converted to the safe previous-context user message. This code then drops the only thinking block and appends the tool call, producing a latest assistant message whose first block is tool_use; the existing thinking-handoff invariant says tool-use assistant messages must start with valid thinking/redacted_thinking (src/specs/thinking-handoff.test.ts:19-27), so Bedrock→Anthropic handoffs into a thinking-enabled agent still fail or silently lose thinking.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 725d195. You're right that the pre-pass and the converter disagreed: ensureThinkingBlockInMessages counted reasoning_content (and unsigned thinking/reasoning) as a valid thinking block, so it left the cross-provider tool-use turn as an assistant message, and the converter then dropped the reasoning — leaving tool_use with no thinking. The pre-pass is now provider-aware via isNativeThinkingBlock: for an Anthropic target only signed thinking/redacted_thinking count, so a Bedrock reasoning_content+tool_use turn is routed to the safe [Previous agent context] conversion — the same handling already applied to no-thinking tool-use turns. The 38 existing thinking-handoff tests still pass; 5 cross-provider cases added. Verified end-to-end that the resulting payload is accepted by a thinking-enabled Anthropic agent. (Note: the live API also tolerates a dropped-reasoning tool_use turn with no thinking block, so this is primarily a consistency/defensive alignment with the documented invariant.)
There was a problem hiding this comment.
Update: after live-testing, we reverted the round-5 pre-pass change (3307920). The official Anthropic API accepts a dropped-reasoning tool_use turn with no thinking block, so the converter-level drop is sufficient and the pre-pass conversion — which also leaked reasoning into visible context (Finding 10) and discarded tool structure — is unnecessary here. The cross-provider crash this PR targets is fixed by the converter changes; the turn is now left as an assistant message with the foreign reasoning dropped.
| type: 'default', | ||
| }, | ||
| } as BedrockContentBlock); | ||
| } else if (FOREIGN_REASONING_TYPES.some((t) => t === block.type)) { |
There was a problem hiding this comment.
For Anthropic→Bedrock handoffs into a Bedrock thinking-enabled agent, ensureThinkingBlockInMessages also treats thinking/redacted_thinking as satisfying the thinking requirement (src/messages/format.ts:1836-1839), so it preserves the historical assistant tool-use turn instead of converting it to previous-context text. This branch drops those Anthropic thinking blocks, after which the existing tool_calls mapping still appends toolUse, leaving the latest assistant tool loop without a Bedrock reasoningContent block and violating the same thinking-handoff invariant documented for Anthropic/Bedrock thinking agents (src/specs/thinking-handoff.test.ts:19-27).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 725d195 — same provider-aware pre-pass fix, mirrored for Bedrock. isNativeThinkingBlock counts only reasoning_content (plus additional_kwargs.reasoning_content) for a Bedrock target, so an Anthropic thinking/redacted_thinking+tool_use turn handed to a thinking-enabled Bedrock agent is routed to the safe [Previous agent context] conversion instead of being left as a tool-use turn whose reasoning the Bedrock converter then drops. chainHasThinkingBlock is provider-aware to match. Covered by the new cross-provider tests + existing Bedrock thinking-chain tests.
There was a problem hiding this comment.
Update: reverted the round-5 pre-pass change for both providers (3307920) — see the Finding 8 and Finding 10 threads for rationale. The Bedrock-side converter drop already handles the Anthropic→Bedrock direction (live-verified); no lossy pre-pass conversion needed.
…re-pass Findings 8 & 9 (P1): ensureThinkingBlockInMessages counted ANY reasoning block (including foreign `reasoning_content`/`thinking` that the converter later drops) as satisfying the "a tool_use turn must begin with a thinking block" invariant, so it left a cross-provider foreign-reasoning tool-use turn as an assistant message instead of converting it to the safe [Previous agent context] form — leaving (after the converter drops the reasoning) a tool_use turn with no thinking block. isNativeThinkingBlock(block, provider) now counts a reasoning block only if the target provider's converter will keep it (Anthropic: signed `thinking` / `redacted_thinking`; Bedrock: `reasoning_content`). Foreign reasoning therefore routes the tool-use turn through the same safe conversion already applied to no-thinking tool-use turns. `additional_kwargs.reasoning_content` counts only for a Bedrock target; chainHasThinkingBlock is provider-aware to match. Same-provider thinking chains unchanged (38 existing tests pass); 5 new cross-provider cases added.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 725d19572b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| c.type === ContentTypes.REASONING || | ||
| c.type === 'redacted_thinking' | ||
| ) { | ||
| } else if (isNativeThinkingBlock(c, provider)) { |
There was a problem hiding this comment.
When the target is Anthropic or Bedrock and a previous assistant turn from the other provider has foreign reasoning plus tool_calls, this check no longer treats that reasoning as native, so the message falls into the [Previous agent context] conversion path. That path uses appendMessageContent's fallback serialization for unknown blocks, which turns blocks like Bedrock reasoning_content or Anthropic thinking—including hidden reasoning text and signatures—into user-visible JSON text instead of dropping them. The cross-provider conversion path should filter these foreign reasoning blocks before buffering the AI/Tool sequence.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Resolved by reverting round 5 in 3307920 — rather than fix the leak inside the conversion path, we removed the conversion for cross-provider foreign-reasoning turns entirely. Live testing confirmed the converter-level drop already yields an Anthropic-accepted payload for a dropped-reasoning tool_use turn (both handoff directions), so these turns now stay as assistant messages (reasoning dropped by the converter), which preserves the tool_use/tool_result structure and avoids serializing reasoning text/signatures into visible context. Thanks — this catch tipped the decision.
Reverts 725d195. Round 5 routed cross-provider foreign-reasoning tool-use turns through ensureThinkingBlockInMessages' `[Previous agent context]` conversion, which (Codex Finding 10) serialized the foreign `reasoning_content`/`thinking` blocks — including hidden reasoning text and signatures — into user-visible JSON, and also discarded the structured tool_use/tool_result. Live testing showed the converter-level drop (earlier commits) already produces an Anthropic-accepted payload for a dropped-reasoning tool_use turn in both handoff directions, so the pre-pass conversion is unnecessary here. Leaving these turns as assistant messages (reasoning dropped by the converter) preserves tool structure and avoids the leak.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33079207e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // input and fall through to the throw below rather than being silently | ||
| // dropped — as does any other unknown block (user media, Google | ||
| // code-execution), which must be surfaced, not discarded. | ||
| return null; |
There was a problem hiding this comment.
When the target Anthropic agent has thinking enabled, Graph.ts runs ensureThinkingBlockInMessages before this converter, and that helper treats reasoning_content as an existing thinking block (src/messages/format.ts lines 1835-1841), so a Bedrock handoff shaped like [reasoning_content, tool_use] is left as an assistant tool turn. Returning null here removes the only leading reasoning block; the payload can then contain text/tool_use without thinking/redacted_thinking at the start of the latest tool-use turn, which Anthropic thinking requests reject or degrade. Convert that foreign tool turn to safe prior-agent context instead of only dropping the block.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is the same interaction as Findings 8/9, which we resolved by reverting the round-5 pre-pass change (3307920) — a deliberate decision after live testing, not an oversight. With the official Anthropic API (claude-sonnet-4-6, thinking enabled), a [reasoning_content/thinking, tool_use] history whose foreign reasoning is dropped — i.e. a tool_use turn with no leading thinking block — is accepted and answered normally, not rejected. The alternative (routing the turn through ensureThinkingBlockInMessages' [Previous agent context] conversion) was implemented and reverted because it serialized the foreign reasoning text + signatures into visible context (your Finding 10) and discarded the tool_use/tool_result structure — strictly worse. Distinction that matters here: Anthropic rejects an invalid/mismatched thinking signature, not the absence of thinking on a historical tool turn. If a concrete repro of rejection surfaces (interleaved-thinking beta, multi-iteration loop), the right fix is a reasoning-stripped pre-pass conversion that avoids the Finding-10 leak — but absent that, dropping the foreign block is verified-correct and preserves structure.
There was a problem hiding this comment.
Settled with live tests (real creds, round-5 reverted, full pipeline ensureThinkingBlockInMessages → converter → provider API):
| scenario | receiver | result |
|---|---|---|
| single tool round, default betas | Anthropic-thinking (claude-sonnet-4-6) | ✅ accepted |
single round + interleaved-thinking-2025-05-14 beta |
Anthropic-thinking | ✅ accepted |
| multi-round (reasoning on 1st turn only, follow-up tool turn has none) | Anthropic-thinking | ✅ accepted |
In every case the converter drops the foreign reasoning_content, leaving an assistant tool_use turn with no leading thinking block, and Anthropic accepts and answers normally — it does not reject or degrade. So the converter-level drop is sufficient and the [Previous agent context] conversion (which leaked reasoning per Finding 10 and lost tool structure) is not needed here. Holding with the drop.
| @@ -0,0 +1,329 @@ | |||
| /* eslint-disable @typescript-eslint/no-explicit-any */ | |||
There was a problem hiding this comment.
/workspace/agents/AGENTS.md says “Never use any” and that all TypeScript/ESLint diagnostics must be addressed, but this new test file disables @typescript-eslint/no-explicit-any for the whole file and then relies on any casts for payload shapes. Keeping the lint rule active and using narrow local test types would preserve coverage while avoiding a repo-wide standards exception in newly added code.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The adjacent existing test files in this directory — streaming-tool-input.test.ts and server-tool-inputs.test.ts — already use this same file-level /* eslint-disable @typescript-eslint/no-explicit-any */ for the same reason: constructing deliberately cross-provider / edge-case message shapes that don't fit LangChain v1's strict content union, and asserting on loosely-typed provider payloads. AGENTS.md also discourages unknown and Record<string, unknown>, which leaves no clean typed alternative for these fixtures, so I followed the established sibling-test convention. Happy to convert these two files to narrow local types if you'd prefer the new tests hold a higher bar than the existing ones.
There was a problem hiding this comment.
Done in d534f64 — converted both files. Removed the file-level eslint-disable and every as any: content blocks are now plain inline literals (LangChain accepts { type, ... } shapes, so no cast is needed), and assertions read through small explicit view types (TestBlock / ConverseBlock) plus typed helpers (assistantBlocks / assistantContent). No any remains, no behavior/coverage change — 16 tests pass, build + lint clean.
| // cross-provider handoff (e.g. Anthropic → Bedrock) rather than crash. | ||
| // The Bedrock model produces its own reasoning. Anything else unknown | ||
| // still throws below — real content must be surfaced, not dropped. | ||
| return; |
There was a problem hiding this comment.
For a Bedrock target with thinking enabled, ensureThinkingBlockInMessages also treats prior Anthropic/Google reasoning blocks as valid thinking (src/messages/format.ts lines 1835-1841), so those assistant tool turns are not converted to safe prior-agent context before reaching this converter. Dropping the foreign block here and then appending toolUse below can leave the latest assistant/tool-result turn with no leading reasoningContent, which Bedrock Claude thinking requests require during tool use; handle these cross-provider thinking tool turns before discarding the block.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Same rationale as the Finding 11 thread (mirror direction). We reverted the round-5 provider-aware pre-pass (3307920) deliberately: the converter-level drop produces a payload the live Bedrock/Anthropic thinking APIs accept, while the [Previous agent context] conversion leaked foreign reasoning text/signatures into visible context (Finding 10) and lost tool structure. Bedrock's tool calls are materialized from msg.tool_calls regardless, so dropping the foreign reasoning block leaves a valid tool turn. If a concrete rejection repro surfaces, the fix would be a reasoning-stripped pre-pass conversion rather than re-introducing the leak.
There was a problem hiding this comment.
Settled with a live test (real creds): Anthropic thinking+tool turn → thinking-enabled Bedrock (claude-sonnet-4-5, calculator tool bound). The converter emits a single toolUse block (the Anthropic thinking is dropped, tool call preserved) and Bedrock accepts it and answers correctly ("17 × 23 is 391"). No leading reasoningContent is required for a historical tool turn after the foreign block is dropped. Same conclusion as the Finding 11 thread — converter-level drop is sufficient.
Addresses Codex Finding 12. Replaces the file-level
`/* eslint-disable @typescript-eslint/no-explicit-any */` and the per-block
`as any` casts in the two new cross-provider-reasoning test files with explicit
local view types (TestBlock / ConverseBlock) plus small typed helpers
(assistantBlocks / assistantContent). Content blocks are now plain inline
literals (no cast needed; LangChain accepts `{ type, ... }` shapes). No behavior
or coverage change — 16 tests still pass; build + lint clean.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d534f645c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| isAIMessage(message) && | ||
| foreignReasoningTypes.some((t) => t === contentPart.type) | ||
| ) { |
There was a problem hiding this comment.
When the receiving Anthropic agent has extended thinking enabled and a prior Bedrock/Google assistant turn contains tool_calls, ensureThinkingBlockInMessages still treats these foreign reasoning blocks as satisfying the tool-use thinking pre-pass, so the AI→Tool sequence is preserved. This branch then removes the only thinking block, and the payload can enter Anthropic with the last tool-use turn starting at tool_use/text instead of thinking/redacted_thinking, which Anthropic documents as invalid during tool use (https://platform.claude.com/docs/en/build-with-claude/extended-thinking); make the pre-pass provider-aware or buffer the foreign turn before stripping it.
Useful? React with 👍 / 👎.
| type: 'default', | ||
| }, | ||
| } as BedrockContentBlock); | ||
| } else if (FOREIGN_REASONING_TYPES.some((t) => t === block.type)) { |
There was a problem hiding this comment.
When the current Bedrock/Claude request has extended thinking enabled and history contains an Anthropic assistant turn with tool_calls, ensureThinkingBlockInMessages sees thinking/redacted_thinking and leaves the AI→Tool sequence intact, but this branch removes those blocks before toolUse blocks are appended below. Bedrock/Claude also requires preserving thinking blocks for the last assistant tool-use turn (https://docs.aws.amazon.com/bedrock/latest/userguide/claude-messages-extended-thinking.html), so Anthropic→Bedrock thinking handoffs can still be rejected; convert the foreign turn to buffered text or translate to a valid Bedrock reasoning block before dropping it.
Useful? React with 👍 / 👎.
1 participant
Problem
Cross-provider agent handoffs crash with
Unsupported message content formatwhen an agent on Bedrock Anthropic (extended thinking enabled) hands off to an agent on the official Anthropic provider (e.g. vialc_transfer_to_agent_*).Bedrock represents reasoning as a
reasoning_contentcontent block:{ "type": "reasoning_content", "reasoningText": { "text": "...", "signature": "..." } }On handoff, the accumulated history (containing that block) is re-sent through the official-Anthropic converter
_formatContent(src/llm/anthropic/utils/message_inputs.ts), which had no branch forreasoning_contentand hit the finalelse→throw new Error('Unsupported message content format').It only manifests on the streaming path (
_streamResponseChunks→_convertMessagesToAnthropicPayload), which is the path LibreChat agents always take. The non-streaming_generatepath uses the base@langchain/anthropicformatter, which tolerates the block — which is why this hid in plain sight.Fix
reasoning_content, Googlereasoning, and LibreChatthink. Their signatures are provider-specific and cannot be validated by Anthropic, so forwarding them is never valid; the receiving model produces its own thinking. (Converting to athinkingblock would fail Anthropic's signature validation; the existing empty-content placeholder already covers a turn that ends up with no blocks.)elsenow logs and drops instead of throwing, so a single unrecognized block can never crash an entire run again.Verification
Live (real Bedrock → official Anthropic, streaming): captured a real
reasoning_contentblock fromclaude-sonnet-4-5on Bedrock Converse with extended thinking, then sent it through the official Anthropic provider..stream()(the agent path)Unsupported message content format.invoke()streamingUnit: new
cross-provider-reasoning.test.ts(6 cases — each reasoning family, the unknown-block fallback, signature-not-forwarded, reasoning-only placeholder) + existingstreaming-tool-input.test.tsall pass.tscbuild and lint clean.Scope
Single-point fix at the provider boundary. Resolves every Bedrock/Google → Anthropic handoff, not just the originally reported configuration.