mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-19 05:12:16 +03:00
fix(memory): merge memory into top-level system field instead of unshifting at messages[0] (#13425) (#13427)
Memory injection merges into an Anthropic-shaped top-level `system` field (string or text-block array) instead of prepending a `role: "system"` message at `messages[0]`, which Anthropic rejects with a 400 (#13425). Handled on both the system-first path (xiaomi-mimo) and the general path. Chosen over #13549, which covered only the string case. Maintainer fix: widened `ChatRequest.system` to `string | Array<{ type; text?; … }>`. Assigning the block array to the `string`-typed field raised 2× TS2322 under `check:open-sse-typecheck`. Validated in one consolidated batch of this series (37 PRs boarded together on `release/v3.8.51`): `typecheck:core`, `check:open-sse-typecheck` and `check:dashboard-typecheck` clean; ESLint clean on every changed file; file-size, complexity, cognitive-complexity, changelog-integrity, docs-counts, docs-sync and migration-numbering gates green (only the pre-existing `open-sse/utils/stream.ts` file-size red remains, inherited from the base); 3,743 focused `node:test` cases plus 34 vitest cases green. Thanks @KooshaPari!
This commit is contained in:
committed by
GitHub
parent
38ce44992e
commit
1738beb0fe
@@ -28,7 +28,8 @@ export interface ChatMessage {
|
||||
export interface ChatRequest {
|
||||
model: string;
|
||||
messages: ChatMessage[];
|
||||
system?: string;
|
||||
/** Anthropic-shaped bodies carry the system prompt here, as a string or text blocks (#13425). */
|
||||
system?: string | Array<{ type: string; text?: string; [key: string]: unknown }>;
|
||||
temperature?: number;
|
||||
max_tokens?: number;
|
||||
stream?: boolean;
|
||||
@@ -87,9 +88,7 @@ const BUILTIN_PROVIDERS_SYSTEM_MUST_BE_FIRST = new Set(["xiaomi-mimo", "mimo", "
|
||||
* Parses OMNIROUTE_STRICT_SYSTEM_PROVIDERS into a normalized id list.
|
||||
* Exported for tests; not expected to be called directly by other modules.
|
||||
*/
|
||||
export function parseStrictSystemProvidersEnv(
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
): string[] {
|
||||
export function parseStrictSystemProvidersEnv(env: NodeJS.ProcessEnv = process.env): string[] {
|
||||
const raw = env.OMNIROUTE_STRICT_SYSTEM_PROVIDERS ?? "";
|
||||
return raw
|
||||
.split(",")
|
||||
@@ -110,7 +109,7 @@ function resolveProvidersSystemMustBeFirst(env: NodeJS.ProcessEnv = process.env)
|
||||
*/
|
||||
export function systemMessageMustBeFirst(
|
||||
provider: string | null | undefined,
|
||||
env: NodeJS.ProcessEnv = process.env,
|
||||
env: NodeJS.ProcessEnv = process.env
|
||||
): boolean {
|
||||
if (!provider) return false;
|
||||
const normalized = provider.toLowerCase().trim();
|
||||
@@ -170,6 +169,16 @@ function injectSystemFirst(
|
||||
const merged: ChatMessage = { ...first, content: `${memoryText}\n${first.content}` };
|
||||
return { ...request, messages: [merged, ...messages.slice(1)] };
|
||||
}
|
||||
// #13425: Anthropic-shaped bodies carry the system prompt in the top-level
|
||||
// `system` field, not in messages[0]. Unshifting a `{role:"system"}` at
|
||||
// messages[0] triggers a 400 ("use the top-level 'system' parameter").
|
||||
// Merge the memory text into the top-level field instead.
|
||||
if (typeof request.system === "string") {
|
||||
return { ...request, system: `${memoryText}\n${request.system}` };
|
||||
}
|
||||
if (Array.isArray(request.system)) {
|
||||
return { ...request, system: [{ type: "text", text: memoryText }, ...request.system] };
|
||||
}
|
||||
const memorySystemMessage: ChatMessage = { role: "system", content: memoryText };
|
||||
return { ...request, messages: [memorySystemMessage, ...messages] };
|
||||
}
|
||||
@@ -281,6 +290,17 @@ export function injectMemory(
|
||||
return injectSystemFirst(request, messages, memoryText, memories.length);
|
||||
}
|
||||
|
||||
// #13425: Anthropic-shaped bodies carry the system prompt in the top-level
|
||||
// `system` field, not in messages[0]. If supportsSystem is true and the
|
||||
// request already has a top-level `system` field, merge memory there instead
|
||||
// of prepending a `{role:"system"}` at messages[0] — Anthropic rejects that.
|
||||
if (supportsSystem && (typeof request.system === "string" || Array.isArray(request.system))) {
|
||||
if (typeof request.system === "string") {
|
||||
return { ...request, system: `${memoryText}\n${request.system}` };
|
||||
}
|
||||
return { ...request, system: [{ type: "text", text: memoryText }, ...request.system] };
|
||||
}
|
||||
|
||||
// Strategy 1 (system): prepend before existing system messages, preserving the
|
||||
// caller's own instructions. Strategy 2 (user, e.g. o1-mini): inject as a user
|
||||
// message. Both honor the #3890 cache-safe anchor via placeMessage.
|
||||
|
||||
140
tests/unit/memory-injection-system-field-13425.test.ts
Normal file
140
tests/unit/memory-injection-system-field-13425.test.ts
Normal file
@@ -0,0 +1,140 @@
|
||||
/**
|
||||
* Tests for #13425: memory injection unshifts system message at messages[0]
|
||||
* despite top-level `system` field, causing Anthropic 400 errors.
|
||||
*
|
||||
* When a body carries a top-level `system` field (string or block array),
|
||||
* injectMemory must merge memory text into that field instead of prepending
|
||||
* a `{role:"system"}` at messages[0].
|
||||
*/
|
||||
|
||||
import { describe, it } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { injectMemory } from "../../src/lib/memory/injection.ts";
|
||||
import type { ChatRequest } from "../../src/lib/memory/injection.ts";
|
||||
import { MemoryType } from "../../src/lib/memory/types.ts";
|
||||
import type { Memory } from "../../src/lib/memory/types.ts";
|
||||
|
||||
function mem(content: string): Memory {
|
||||
return {
|
||||
id: `mem-${content}`,
|
||||
content,
|
||||
type: MemoryType.FACTUAL,
|
||||
apiKeyId: "k",
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:00:00.000Z",
|
||||
importance: 0.5,
|
||||
} as unknown as Memory;
|
||||
}
|
||||
|
||||
const MEMORY = [mem("User prefers dark mode")];
|
||||
|
||||
describe("memory injection into top-level system field (#13425)", () => {
|
||||
it("xiaomi-mimo: merges memory into string system field via injectSystemFirst", () => {
|
||||
const request: ChatRequest = {
|
||||
model: "test",
|
||||
system: "You are a helpful assistant.",
|
||||
messages: [{ role: "user", content: "hello" }],
|
||||
};
|
||||
const result = injectMemory(request, MEMORY, "xiaomi-mimo");
|
||||
|
||||
assert.ok(
|
||||
typeof result.system === "string" && result.system.includes("User prefers dark mode"),
|
||||
"memory text should be merged into top-level system field"
|
||||
);
|
||||
assert.ok(
|
||||
typeof result.system === "string" && result.system.includes("You are a helpful assistant."),
|
||||
"original system text should be preserved"
|
||||
);
|
||||
assert.notEqual(
|
||||
result.messages[0]?.role,
|
||||
"system",
|
||||
"should not unshift system message at messages[0]"
|
||||
);
|
||||
assert.equal(result.messages[0]?.content, "hello", "first user turn preserved");
|
||||
});
|
||||
|
||||
it("xiaomi-mimo: merges memory into block-array system field", () => {
|
||||
const request: ChatRequest = {
|
||||
model: "test",
|
||||
system: [{ type: "text", text: "You are a helpful assistant." }] as unknown as string,
|
||||
messages: [{ role: "user", content: "hello" }],
|
||||
};
|
||||
const result = injectMemory(request, MEMORY, "xiaomi-mimo");
|
||||
|
||||
assert.ok(Array.isArray(result.system), "system field should remain an array");
|
||||
const blocks = result.system as unknown as Array<{ type: string; text: string }>;
|
||||
assert.equal(blocks[0].type, "text");
|
||||
assert.ok(blocks[0].text.includes("User prefers dark mode"), "memory should be first block");
|
||||
assert.ok(
|
||||
blocks.some((b) => b.text?.includes("You are a helpful assistant.")),
|
||||
"original system text preserved in array"
|
||||
);
|
||||
assert.notEqual(result.messages[0]?.role, "system");
|
||||
});
|
||||
|
||||
it("claude: merges memory into string system field via general path", () => {
|
||||
const request: ChatRequest = {
|
||||
model: "test",
|
||||
system: "You are a helpful assistant.",
|
||||
messages: [{ role: "user", content: "hello" }],
|
||||
};
|
||||
const result = injectMemory(request, MEMORY, "claude");
|
||||
|
||||
assert.ok(
|
||||
typeof result.system === "string" && result.system.includes("User prefers dark mode"),
|
||||
"memory text should be merged into top-level system field"
|
||||
);
|
||||
assert.notEqual(result.messages[0]?.role, "system");
|
||||
assert.equal(result.messages[0]?.content, "hello");
|
||||
});
|
||||
|
||||
it("anthropic: merges memory into block-array system field via general path", () => {
|
||||
const request: ChatRequest = {
|
||||
model: "test",
|
||||
system: [{ type: "text", text: "You are a helpful assistant." }] as unknown as string,
|
||||
messages: [{ role: "user", content: "hello" }],
|
||||
};
|
||||
const result = injectMemory(request, MEMORY, "anthropic");
|
||||
|
||||
assert.ok(Array.isArray(result.system), "system field should remain an array");
|
||||
const blocks = result.system as unknown as Array<{ type: string; text: string }>;
|
||||
assert.ok(blocks[0].text.includes("User prefers dark mode"), "memory should be first block");
|
||||
assert.notEqual(result.messages[0]?.role, "system");
|
||||
});
|
||||
|
||||
it("merging into existing system message at messages[0] still works (xiaomi-mimo)", () => {
|
||||
const request: ChatRequest = {
|
||||
model: "test",
|
||||
messages: [
|
||||
{ role: "system", content: "Original system prompt" },
|
||||
{ role: "user", content: "hello" },
|
||||
],
|
||||
};
|
||||
const result = injectMemory(request, MEMORY, "xiaomi-mimo");
|
||||
|
||||
assert.equal(result.messages[0]?.role, "system");
|
||||
assert.ok(
|
||||
(result.messages[0]?.content as string).includes("User prefers dark mode"),
|
||||
"memory should be merged into existing leading system message"
|
||||
);
|
||||
assert.ok(
|
||||
(result.messages[0]?.content as string).includes("Original system prompt"),
|
||||
"original system text should be preserved"
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to messages prepend when no system field and no system message at [0]", () => {
|
||||
const request: ChatRequest = {
|
||||
model: "test",
|
||||
messages: [{ role: "user", content: "hello" }],
|
||||
};
|
||||
const result = injectMemory(request, MEMORY, "claude");
|
||||
|
||||
assert.equal(result.messages[0]?.role, "system");
|
||||
assert.ok(
|
||||
(result.messages[0]?.content as string).includes("User prefers dark mode"),
|
||||
"memory should be prepended as system message"
|
||||
);
|
||||
assert.equal(result.messages[1]?.content, "hello", "original first turn preserved");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user