From 91658339ec39b5bc5d1394167fb3e7b25aef8d6a Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 14 Jul 2026 15:29:39 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=AB=20fix:=20Strip=20Reserved=20Fields?= =?UTF-8?q?=20From=20Bedrock=20`additionalModelRequestFields`=20(#14246)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🐛 fix: Strip duplicate `system` from Bedrock additionalModelRequestFields Bedrock Anthropic presets bind the system prompt to the `system` model param. bedrockInputParser routes `system` into additionalModelRequestFields, then bedrockOutputParser promotes it back to the root as a known key without removing the copy. Bedrock Converse then sees `system` in both places and rejects the request ("The additional field system conflicts with an existing field"), which surfaces once context compression/summarization runs. Delete `system` from additionalModelRequestFields after promoting it to the root. `system` is the only leaked field that collides with a reserved top-level Converse field, so the fix is scoped to it and leaves other passthrough fields untouched. Clones before mutating to avoid touching the caller's input. Closes #14029 * 🛡️ fix: Guard scalar additionalModelRequestFields before `in` check DocumentType permits scalar values (boolean/number/string), so a saved Bedrock preset/agent can carry a non-object additionalModelRequestFields. The new `system` cleanup used `'system' in amrf`, which throws TypeError on a truthy scalar. Guard with a typeof-object check to keep the prior tolerant behavior; the empty-check is left unchanged. * 🛡️ fix: Strip all reserved Converse fields from additionalModelRequestFields --- packages/data-provider/specs/bedrock.spec.ts | 83 ++++++++++++++++++++ packages/data-provider/src/bedrock.ts | 35 ++++++++- 2 files changed, 117 insertions(+), 1 deletion(-) diff --git a/packages/data-provider/specs/bedrock.spec.ts b/packages/data-provider/specs/bedrock.spec.ts index 7952925131..9f3cf81e31 100644 --- a/packages/data-provider/specs/bedrock.spec.ts +++ b/packages/data-provider/specs/bedrock.spec.ts @@ -1372,6 +1372,89 @@ describe('bedrockInputParser', () => { }); }); + // Regression for #14029: `system` is a reserved top-level Converse field, so a + // copy left inside additionalModelRequestFields makes Bedrock reject the request + // ("The additional field system conflicts with an existing field"). + describe('system field (issue #14029)', () => { + test('promotes system to root without duplicating it in additionalModelRequestFields', () => { + const parsed = bedrockInputParser.parse({ + model: 'some-other-model', + system: 'You are a helpful assistant.', + }) as Record; + expect((parsed.additionalModelRequestFields as Record).system).toBe( + 'You are a helpful assistant.', + ); + + const output = bedrockOutputParser(parsed); + expect(output.system).toBe('You are a helpful assistant.'); + expect(output.additionalModelRequestFields).toBeUndefined(); + }); + + test('strips system from additionalModelRequestFields while preserving other fields', () => { + const parsed = bedrockInputParser.parse({ + model: 'anthropic.claude-3-7-sonnet', + system: 'You are a helpful assistant.', + }) as Record; + + const output = bedrockOutputParser(parsed); + const amrf = output.additionalModelRequestFields as Record | undefined; + expect(output.system).toBe('You are a helpful assistant.'); + expect(amrf?.system).toBeUndefined(); + expect(amrf?.thinking).toBeDefined(); + }); + + // DocumentType permits scalars, so a saved preset can carry a non-object + // additionalModelRequestFields; the `system` cleanup must not throw on it. + test.each([['a-scalar-string'], [42], [true]])( + 'tolerates a scalar additionalModelRequestFields (%p) without throwing', + (scalar) => { + expect(() => + bedrockOutputParser({ + model: 'some-other-model', + additionalModelRequestFields: scalar, + }), + ).not.toThrow(); + }, + ); + + // `system` is not the only reserved name: the input parser's catch-all routes + // ANY unknown preset key into additionalModelRequestFields, and each reserved + // Converse field collides the same way when the request sends it top-level. + test.each([['messages'], ['modelId'], ['toolConfig'], ['inferenceConfig']])( + 'strips reserved Converse field %p from additionalModelRequestFields', + (reserved) => { + const parsed = bedrockInputParser.parse({ + model: 'some-other-model', + [reserved]: { some: 'value' }, + }) as Record; + expect( + (parsed.additionalModelRequestFields as Record)[reserved], + ).toBeDefined(); + + const output = bedrockOutputParser(parsed); + const amrf = output.additionalModelRequestFields as Record | undefined; + expect(amrf?.[reserved]).toBeUndefined(); + }, + ); + + test('keeps non-reserved passthrough fields intact while stripping reserved ones', () => { + const output = bedrockOutputParser({ + model: 'some-other-model', + additionalModelRequestFields: { + system: 'dup', + messages: [], + anthropic_beta: ['context-1m-2025-08-07'], + top_k: 40, + }, + }); + const amrf = output.additionalModelRequestFields as Record; + expect(amrf.system).toBeUndefined(); + expect(amrf.messages).toBeUndefined(); + expect(amrf.anthropic_beta).toEqual(['context-1m-2025-08-07']); + expect(amrf.top_k).toBe(40); + }); + }); + describe('Model switching cleanup', () => { test('should strip anthropic_beta when switching from Anthropic to non-Anthropic model', () => { const staleConversationData = { diff --git a/packages/data-provider/src/bedrock.ts b/packages/data-provider/src/bedrock.ts index 3aad248075..69b9b9fa7b 100644 --- a/packages/data-provider/src/bedrock.ts +++ b/packages/data-provider/src/bedrock.ts @@ -737,6 +737,25 @@ function configureThinking(data: AnthropicInput): AnthropicInput { return updatedData; } +/** Top-level Converse request fields (issue #14029: `system` from a preset). + * The input parser's catch-all routes unknown keys into + * additionalModelRequestFields, and Bedrock rejects any that collide with a + * field the request already sends (`messages`/`modelId` always, + * `inferenceConfig` whenever maxTokens is set, `toolConfig` for agents). */ +const RESERVED_CONVERSE_FIELDS = [ + 'system', + 'messages', + 'modelId', + 'toolConfig', + 'inferenceConfig', + 'guardrailConfig', + 'promptVariables', + 'requestMetadata', + 'performanceConfig', + 'additionalModelRequestFields', + 'additionalModelResponseFieldPaths', +]; + export const bedrockOutputParser = (data: Record) => { const knownKeys = [...Object.keys(s.tConversationSchema.shape), 'topK', 'top_k']; let result: Record = {}; @@ -776,7 +795,21 @@ export const bedrockOutputParser = (data: Record) => { } result = configureThinking(result as AnthropicInput); - const amrf = result.additionalModelRequestFields as Record | undefined; + let amrf = result.additionalModelRequestFields as Record | undefined; + // Reserved top-level Converse request fields; a copy inside + // additionalModelRequestFields makes Bedrock reject the request + // ("The additional field conflicts with an existing field"). + // Guard against non-object values, which the schema's DocumentType permits. + if (amrf && typeof amrf === 'object') { + const reserved = RESERVED_CONVERSE_FIELDS.filter((key) => key in (amrf ?? {})); + if (reserved.length > 0) { + amrf = { ...amrf }; + for (const key of reserved) { + delete amrf[key]; + } + result.additionalModelRequestFields = amrf; + } + } if (!amrf || Object.keys(amrf).length === 0) { delete result.additionalModelRequestFields; }