mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
🎫 fix: Strip Reserved Fields From Bedrock additionalModelRequestFields (#14246)
* 🐛 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
This commit is contained in:
parent
5b0330fdfb
commit
91658339ec
2 changed files with 117 additions and 1 deletions
|
|
@ -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<string, unknown>;
|
||||
expect((parsed.additionalModelRequestFields as Record<string, unknown>).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<string, unknown>;
|
||||
|
||||
const output = bedrockOutputParser(parsed);
|
||||
const amrf = output.additionalModelRequestFields as Record<string, unknown> | 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<string, unknown>;
|
||||
expect(
|
||||
(parsed.additionalModelRequestFields as Record<string, unknown>)[reserved],
|
||||
).toBeDefined();
|
||||
|
||||
const output = bedrockOutputParser(parsed);
|
||||
const amrf = output.additionalModelRequestFields as Record<string, unknown> | 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<string, unknown>;
|
||||
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 = {
|
||||
|
|
|
|||
|
|
@ -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<string, unknown>) => {
|
||||
const knownKeys = [...Object.keys(s.tConversationSchema.shape), 'topK', 'top_k'];
|
||||
let result: Record<string, unknown> = {};
|
||||
|
|
@ -776,7 +795,21 @@ export const bedrockOutputParser = (data: Record<string, unknown>) => {
|
|||
}
|
||||
|
||||
result = configureThinking(result as AnthropicInput);
|
||||
const amrf = result.additionalModelRequestFields as Record<string, unknown> | undefined;
|
||||
let amrf = result.additionalModelRequestFields as Record<string, unknown> | undefined;
|
||||
// Reserved top-level Converse request fields; a copy inside
|
||||
// additionalModelRequestFields makes Bedrock reject the request
|
||||
// ("The additional field <name> 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;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue