diff --git a/changelog.d/fixes/output-styles-top-level-system.md b/changelog.d/fixes/output-styles-top-level-system.md new file mode 100644 index 0000000000..c21d44c274 --- /dev/null +++ b/changelog.d/fixes/output-styles-top-level-system.md @@ -0,0 +1 @@ +- **fix(compression):** output styles and the caveman output mode now place their injected instruction in the top-level `system` field instead of a synthetic `messages[0]` entry for Anthropic-shaped requests, fixing the upstream 400 ("use the top-level 'system' parameter for the initial system prompt") ([#12584](https://github.com/diegosouzapw/OmniRoute/issues/12584)) diff --git a/open-sse/services/compression/outputMode.ts b/open-sse/services/compression/outputMode.ts index 2e188fbf3b..0de0095f5a 100644 --- a/open-sse/services/compression/outputMode.ts +++ b/open-sse/services/compression/outputMode.ts @@ -165,12 +165,14 @@ export function applyCavemanOutputMode( // Check idempotency before bypass so the marker in an already-injected system // message doesn't trigger a false-positive bypass (e.g. SHARED_BOUNDARIES keywords). - const alreadyApplied = messages.some( - (message) => - message.role === "system" && - typeof message.content === "string" && - message.content.includes(CAVEMAN_OUTPUT_MARKER) - ); + const alreadyApplied = + systemFieldIncludesMarker(body.system, CAVEMAN_OUTPUT_MARKER) || + messages.some( + (message) => + message.role === "system" && + typeof message.content === "string" && + message.content.includes(CAVEMAN_OUTPUT_MARKER) + ); if (alreadyApplied) return { body, applied: false, skippedReason: "already_applied" }; if (config.autoClarity !== false) { @@ -179,17 +181,92 @@ export function applyCavemanOutputMode( } const instruction = buildCavemanOutputInstruction(config, language); - const nextMessages = [...messages]; - const first = nextMessages[0]; + return { + body: { ...body, ...placeSystemInstruction(messages, body.system, instruction) }, + applied: true, + }; +} +/** + * Minimal message shape accepted by {@link placeSystemInstruction}. Both the + * legacy caveman injector and the unified output-styles injector funnel their + * instruction placement through it. + */ +interface InjectableMessage { + role?: unknown; + content?: unknown; + [key: string]: unknown; +} + +/** + * Decide where an injected system instruction lands on a chat body without + * ever creating a new `system` message at messages[0]. + * + * Anthropic-shaped bodies carry the initial system prompt in the top-level + * `system` field (string or content-block array) and the claude passthrough + * forwards `messages[]` upstream unchanged, so a synthetic system entry at + * index 0 is rejected with "messages.0: use the top-level 'system' parameter + * for the initial system prompt" (#12584). Placement preference: + * + * 1. Leading string-content system message: merge in place (legacy behavior). + * 2. Top-level `system` string/array: merge there (Anthropic contract). + * 3. Any later string-content system message: merge in place. + * 4. Append a trailing system message (valid at any position for + * OpenAI-shaped bodies; hoisted by the claude system-role extraction). + * + * Returns a partial body patch (`messages` and/or `system`) to spread over + * the original body. + */ +export function placeSystemInstruction( + messages: M[], + system: unknown, + instruction: string +): { messages?: Array; system?: unknown } { + const first = messages[0]; if (first?.role === "system" && typeof first.content === "string") { - nextMessages[0] = { - ...first, - content: `${first.content.trim()}\n\n${instruction}`, + const nextMessages: Array = [...messages]; + nextMessages[0] = { ...first, content: `${first.content.trim()}\n\n${instruction}` }; + return { messages: nextMessages }; + } + if (typeof system === "string") { + const trimmed = system.trim(); + return { system: trimmed ? `${trimmed}\n\n${instruction}` : instruction }; + } + if (Array.isArray(system)) { + return { system: [...system, { type: "text", text: instruction }] }; + } + const mergeIdx = messages.findIndex( + (message) => message?.role === "system" && typeof message.content === "string" + ); + if (mergeIdx >= 0) { + const nextMessages: Array = [...messages]; + const target = messages[mergeIdx]; + nextMessages[mergeIdx] = { + ...target, + content: `${(target.content as string).trim()}\n\n${instruction}`, }; - } else { - nextMessages.unshift({ role: "system", content: instruction }); + return { messages: nextMessages }; + } + if (messages.length === 0) { + // #12584: an empty array has no non-zero index to append at; route through + // `system` instead of creating messages[0]. + return { system: instruction }; } + return { messages: [...messages, { role: "system", content: instruction }] }; +} - return { body: { ...body, messages: nextMessages }, applied: true }; +/** + * True when an idempotency marker is already present in a top-level `system` + * field (string or Anthropic content-block array), so instruction injection + * that targets that field is not re-applied on retries or follow-up turns. + */ +export function systemFieldIncludesMarker(system: unknown, marker: string): boolean { + if (typeof system === "string") return system.includes(marker); + if (Array.isArray(system)) { + return system.some((block) => { + const text = (block as { text?: unknown } | null | undefined)?.text; + return typeof text === "string" && text.includes(marker); + }); + } + return false; } diff --git a/open-sse/services/compression/outputStyles/apply.ts b/open-sse/services/compression/outputStyles/apply.ts index fe3886ef2b..f932d4b9d0 100644 --- a/open-sse/services/compression/outputStyles/apply.ts +++ b/open-sse/services/compression/outputStyles/apply.ts @@ -1,4 +1,9 @@ -import { SHARED_BOUNDARIES, shouldBypassCavemanOutputMode } from "../outputMode.ts"; +import { + placeSystemInstruction, + SHARED_BOUNDARIES, + shouldBypassCavemanOutputMode, + systemFieldIncludesMarker, +} from "../outputMode.ts"; import { detectCompressionLanguage } from "../languageDetector.ts"; import { OUTPUT_STYLE_IDS, outputStyleMeta } from "./catalog.ts"; @@ -30,7 +35,6 @@ export interface OutputStylesResult { appliedStyles?: OutputStyleSelectionEntry[]; } - interface OutputStyleLanguageConfig { enabled?: boolean; autoDetect?: boolean; @@ -100,10 +104,7 @@ function resolveStyles( } /** Build the combined instruction body (no marker, no trailing boundary). Pure / deterministic. */ -function buildStyleInstructions( - resolved: OutputStyleSelectionEntry[], - language: string -): string { +function buildStyleInstructions(resolved: OutputStyleSelectionEntry[], language: string): string { const parts: string[] = []; for (const { id, level } of resolved) { const meta = outputStyleMeta(id); @@ -150,32 +151,34 @@ export function applyOutputStyles( }; } if (typeof body.input === "string" || Array.isArray(body.input)) { - return { body: { ...body, instructions: instruction }, applied: true, appliedStyles: resolved }; + return { + body: { ...body, instructions: instruction }, + applied: true, + appliedStyles: resolved, + }; } return { body, applied: false, skippedReason: "no_messages" }; } // Idempotency before bypass so an already-injected marker (which contains // SHARED_BOUNDARIES keywords) cannot trigger a false-positive bypass. - const alreadyApplied = messages.some( - (message) => - message.role === "system" && - typeof message.content === "string" && - message.content.includes(OUTPUT_STYLE_MARKER) - ); + const alreadyApplied = + systemFieldIncludesMarker(body.system, OUTPUT_STYLE_MARKER) || + messages.some( + (message) => + message.role === "system" && + typeof message.content === "string" && + message.content.includes(OUTPUT_STYLE_MARKER) + ); if (alreadyApplied) return { body, applied: false, skippedReason: "already_applied" }; // Content bypass (all-or-nothing for the turn): reuse the existing rules verbatim. const bypass = shouldBypassCavemanOutputMode(messages); if (bypass) return { body, applied: false, skippedReason: bypass }; - const nextMessages = [...messages]; - const first = nextMessages[0]; - if (first?.role === "system" && typeof first.content === "string") { - nextMessages[0] = { ...first, content: `${first.content.trim()}\n\n${instruction}` }; - } else { - nextMessages.unshift({ role: "system", content: instruction }); - } - - return { body: { ...body, messages: nextMessages }, applied: true, appliedStyles: resolved }; + return { + body: { ...body, ...placeSystemInstruction(messages, body.system, instruction) }, + applied: true, + appliedStyles: resolved, + }; } diff --git a/open-sse/services/systemPrompt.ts b/open-sse/services/systemPrompt.ts index 01e011eadd..d9ab979c10 100644 --- a/open-sse/services/systemPrompt.ts +++ b/open-sse/services/systemPrompt.ts @@ -119,8 +119,11 @@ export function injectSystemPrompt(body: T): T { } nextMessages[sysIdx] = msg; } - } else { - // No existing system message — combine both into one + } else if (result.system === undefined) { + // No existing system message — combine both into one. + // Anthropic-shaped bodies get the prompt via the top-level `system` + // branch below; a new system entry at messages[0] is rejected upstream + // ("messages.0: use the top-level 'system' parameter", #12584). const combined = [prefix, suffix].filter(Boolean).join("\n\n"); if (combined) { nextMessages.unshift({ role: "system", content: combined }); @@ -131,6 +134,9 @@ export function injectSystemPrompt(body: T): T { // Claude format (system field) if (result.system !== undefined) { + // #12584: a malformed non-string/non-array `system` (e.g. `null`) must not + // silently swallow the prompt — normalize before the format branch. + if (typeof result.system !== "string" && !Array.isArray(result.system)) result.system = ""; if (typeof result.system === "string") { let sys = result.system; if (prefix) sys = prefix + "\n\n" + sys; @@ -476,7 +482,10 @@ export function injectCustomSystemPrompt(body: Record, prompt: msg.content = (msg.content ? msg.content + "\n\n" : "") + prompt; } (result.messages as Array<{ role: string; content: unknown }>)[sysIdx] = msg; - } else { + } else if (result.system === undefined) { + // Anthropic-shaped bodies get the prompt via the top-level `system` + // branch below; a new system entry at messages[0] is rejected upstream + // ("messages.0: use the top-level 'system' parameter", #12584). result.messages = [ { role: "system", content: prompt }, ...(result.messages as Array<{ role: string; content: unknown }>), @@ -486,6 +495,9 @@ export function injectCustomSystemPrompt(body: Record, prompt: // Claude direct system field if (result.system !== undefined) { + // #12584: a malformed non-string/non-array `system` (e.g. `null`) must not + // silently swallow the prompt — normalize before the format branch. + if (typeof result.system !== "string" && !Array.isArray(result.system)) result.system = ""; if (typeof result.system === "string") { result.system = result.system ? result.system + "\n\n" + prompt : prompt; } else if (Array.isArray(result.system)) { diff --git a/tests/unit/compression/output-styles-apply.test.ts b/tests/unit/compression/output-styles-apply.test.ts index c392012843..a9a64d8213 100644 --- a/tests/unit/compression/output-styles-apply.test.ts +++ b/tests/unit/compression/output-styles-apply.test.ts @@ -7,9 +7,8 @@ import { type OutputStyleSelectionEntry, } from "../../../open-sse/services/compression/outputStyles/apply.ts"; -const sel = ( - ...entries: Array<[string, "lite" | "full" | "ultra"]> -): OutputStyleSelectionEntry[] => entries.map(([id, level]) => ({ id, level })); +const sel = (...entries: Array<[string, "lite" | "full" | "ultra"]>): OutputStyleSelectionEntry[] => + entries.map(([id, level]) => ({ id, level })); test("injects a system instruction with the unified marker", () => { const r = applyOutputStyles( @@ -17,9 +16,9 @@ test("injects a system instruction with the unified marker", () => { sel(["terse-prose", "full"]) ); assert.equal(r.applied, true); - assert.equal(r.body.messages?.[0]?.role, "system"); - assert.match(String(r.body.messages?.[0]?.content), new RegExp(escapeRe(OUTPUT_STYLE_MARKER))); - assert.match(String(r.body.messages?.[0]?.content), /Respond terse/); + assert.equal(r.body.messages?.at(-1)?.role, "system"); + assert.match(String(r.body.messages?.at(-1)?.content), new RegExp(escapeRe(OUTPUT_STYLE_MARKER))); + assert.match(String(r.body.messages?.at(-1)?.content), /Respond terse/); assert.deepEqual(r.appliedStyles, [{ id: "terse-prose", level: "full" }]); }); @@ -28,7 +27,7 @@ test("combines two styles in catalog order with a single shared boundary", () => { messages: [{ role: "user", content: "Refactor this module." }] }, sel(["less-code", "full"], ["terse-prose", "full"]) // requested out of order ); - const text = String(r.body.messages?.[0]?.content); + const text = String(r.body.messages?.at(-1)?.content); // catalog order is terse-prose before less-code const proseAt = text.indexOf("Respond terse"); const codeAt = text.indexOf("lazy senior dev"); @@ -62,9 +61,11 @@ test("idempotent: re-applying is a no-op", () => { const twice = applyOutputStyles(once, sel(["terse-prose", "full"])); assert.equal(twice.applied, false); assert.equal(twice.skippedReason, "already_applied"); - const markerCount = (String(twice.body.messages?.[0]?.content).match( - new RegExp(escapeRe(OUTPUT_STYLE_MARKER), "g") - ) ?? []).length; + const markerCount = ( + String(twice.body.messages?.at(-1)?.content).match( + new RegExp(escapeRe(OUTPUT_STYLE_MARKER), "g") + ) ?? [] + ).length; assert.equal(markerCount, 1); }); @@ -75,7 +76,7 @@ test("content bypass is all-or-nothing across every selected style", () => { ); assert.equal(r.applied, false); assert.equal(r.skippedReason, "security_warning"); - assert.equal(r.body.messages?.[0]?.role, "user"); // untouched + assert.equal(r.body.messages?.at(-1)?.role, "user"); // untouched }); test("no styles selected → body untouched", () => { @@ -83,7 +84,7 @@ test("no styles selected → body untouched", () => { const r = applyOutputStyles(body, []); assert.equal(r.applied, false); assert.equal(r.skippedReason, "no_styles"); - assert.equal(r.body.messages?.[0]?.content, "Tell me a joke."); + assert.equal(r.body.messages?.at(-1)?.content, "Tell me a joke."); }); test("unknown style id is skipped, never throws", () => { @@ -92,7 +93,10 @@ test("unknown style id is skipped, never throws", () => { sel(["__nope__", "full"], ["terse-prose", "full"]) ); assert.equal(r.applied, true); - assert.deepEqual(r.appliedStyles?.map((s) => s.id), ["terse-prose"]); + assert.deepEqual( + r.appliedStyles?.map((s) => s.id), + ["terse-prose"] + ); }); test("locale gate: terse-cjk only honored under zh", () => { @@ -110,7 +114,7 @@ test("locale gate: terse-cjk only honored under zh", () => { "zh" ); assert.equal(zh.applied, true); - assert.match(String(zh.body.messages?.[0]?.content), /文言/); + assert.match(String(zh.body.messages?.at(-1)?.content), /文言/); }); test("determinism: same (selection, language) yields byte-identical injected text", () => { @@ -118,7 +122,7 @@ test("determinism: same (selection, language) yields byte-identical injected tex applyOutputStyles( { messages: [{ role: "user", content: "do a thing" }] }, sel(["terse-prose", "full"], ["less-code", "lite"]) - ).body.messages?.[0]?.content; + ).body.messages?.at(-1)?.content; assert.equal(make(), make()); }); @@ -140,15 +144,15 @@ test("terse-prose localizes per language (back-compat with the legacy caveman pa sel(["terse-prose", "lite"]), "pt-BR" ); - assert.match(String(ptBR.body.messages?.[0]?.content), /Responda conciso/); - assert.doesNotMatch(String(ptBR.body.messages?.[0]?.content), /Respond concise/); + assert.match(String(ptBR.body.messages?.at(-1)?.content), /Responda conciso/); + assert.doesNotMatch(String(ptBR.body.messages?.at(-1)?.content), /Respond concise/); const en = applyOutputStyles( { messages: [{ role: "user", content: "Summarize logs." }] }, sel(["terse-prose", "lite"]), "en" ); - assert.match(String(en.body.messages?.[0]?.content), /Respond concise/); + assert.match(String(en.body.messages?.at(-1)?.content), /Respond concise/); }); function escapeRe(s: string): string { @@ -158,7 +162,10 @@ function escapeRe(s: string): string { test("resolveOutputStyleLanguage: en when language support is disabled", () => { const body = { messages: [{ role: "user", content: "preciso de ajuda com o arquivo" }] }; assert.equal(resolveOutputStyleLanguage(undefined, body), "en"); - assert.equal(resolveOutputStyleLanguage({ enabled: false, defaultLanguage: "pt-BR" }, body), "en"); + assert.equal( + resolveOutputStyleLanguage({ enabled: false, defaultLanguage: "pt-BR" }, body), + "en" + ); }); test("resolveOutputStyleLanguage: defaultLanguage when enabled without autoDetect", () => { @@ -170,15 +177,28 @@ test("resolveOutputStyleLanguage: defaultLanguage when enabled without autoDetec }); test("resolveOutputStyleLanguage: detects the last user message language when autoDetect is on", () => { - const de = { messages: [{ role: "user", content: "ich habe eine datei mit einem fehler, kannst du bitte helfen" }] }; + const de = { + messages: [ + { role: "user", content: "ich habe eine datei mit einem fehler, kannst du bitte helfen" }, + ], + }; const ru = { messages: [{ role: "user", content: "мне нужно исправить ошибку в этом файле" }] }; - assert.equal(resolveOutputStyleLanguage({ enabled: true, autoDetect: true, defaultLanguage: "en" }, de), "de"); - assert.equal(resolveOutputStyleLanguage({ enabled: true, autoDetect: true, defaultLanguage: "en" }, ru), "ru"); + assert.equal( + resolveOutputStyleLanguage({ enabled: true, autoDetect: true, defaultLanguage: "en" }, de), + "de" + ); + assert.equal( + resolveOutputStyleLanguage({ enabled: true, autoDetect: true, defaultLanguage: "en" }, ru), + "ru" + ); }); test("resolveOutputStyleLanguage: falls back to defaultLanguage when there is no user text", () => { assert.equal( - resolveOutputStyleLanguage({ enabled: true, autoDetect: true, defaultLanguage: "ja" }, { messages: [] }), + resolveOutputStyleLanguage( + { enabled: true, autoDetect: true, defaultLanguage: "ja" }, + { messages: [] } + ), "ja" ); }); @@ -186,8 +206,89 @@ test("resolveOutputStyleLanguage: falls back to defaultLanguage when there is no test("resolveOutputStyleLanguage: samples array content parts for detection", () => { const body = { messages: [ - { role: "user", content: [{ type: "text", text: "necesito ayuda con este archivo, gracias" }] }, + { + role: "user", + content: [{ type: "text", text: "necesito ayuda con este archivo, gracias" }], + }, ], }; - assert.equal(resolveOutputStyleLanguage({ enabled: true, autoDetect: true, defaultLanguage: "en" }, body), "es"); + assert.equal( + resolveOutputStyleLanguage({ enabled: true, autoDetect: true, defaultLanguage: "en" }, body), + "es" + ); +}); + +test("Anthropic shape: merges into a top-level string system instead of messages[0]", () => { + const r = applyOutputStyles( + { + system: "You are Claude Code.", + messages: [{ role: "user", content: "Summarize this API response." }], + }, + sel(["terse-prose", "full"]) + ); + assert.equal(r.applied, true); + assert.match(String(r.body.system), /You are Claude Code\./); + assert.match(String(r.body.system), new RegExp(escapeRe(OUTPUT_STYLE_MARKER))); + assert.equal(r.body.messages?.length, 1); + assert.equal(r.body.messages?.[0]?.role, "user"); +}); + +test("Anthropic shape: appends a text block to a block-array system", () => { + const baseBlock = { + type: "text", + text: "You are Claude Code.", + cache_control: { type: "ephemeral" }, + }; + const r = applyOutputStyles( + { system: [baseBlock], messages: [{ role: "user", content: "hi" }] }, + sel(["terse-prose", "full"]) + ); + assert.equal(r.applied, true); + const blocks = r.body.system as Array<{ type?: string; text?: string }>; + assert.equal(blocks.length, 2); + assert.deepEqual(blocks[0], baseBlock); + assert.equal(blocks[1]?.type, "text"); + assert.match(String(blocks[1]?.text), new RegExp(escapeRe(OUTPUT_STYLE_MARKER))); + assert.equal(r.body.messages?.[0]?.role, "user"); +}); + +test("Anthropic shape: idempotent when the marker lives in the top-level system", () => { + const once = applyOutputStyles( + { system: "You are Claude Code.", messages: [{ role: "user", content: "hi" }] }, + sel(["terse-prose", "full"]) + ).body; + const twice = applyOutputStyles(once, sel(["terse-prose", "full"])); + assert.equal(twice.applied, false); + assert.equal(twice.skippedReason, "already_applied"); + const markerCount = ( + String(twice.body.system).match(new RegExp(escapeRe(OUTPUT_STYLE_MARKER), "g")) ?? [] + ).length; + assert.equal(markerCount, 1); + + const onceArr = applyOutputStyles( + { system: [{ type: "text", text: "base" }], messages: [{ role: "user", content: "hi" }] }, + sel(["terse-prose", "full"]) + ).body; + const twiceArr = applyOutputStyles(onceArr, sel(["terse-prose", "full"])); + assert.equal(twiceArr.applied, false); + assert.equal((twiceArr.body.system as unknown[]).length, 2); +}); + +test("never introduces a system message at messages[0]", () => { + const shapes = [ + { system: "top", messages: [{ role: "user", content: "hi" }] }, + { messages: [{ role: "user", content: "hi" }] }, + { + messages: [ + { role: "user", content: "hi" }, + { role: "assistant", content: "hello" }, + { role: "user", content: "again" }, + ], + }, + ]; + for (const body of shapes) { + const r = applyOutputStyles(body, sel(["terse-prose", "full"])); + assert.equal(r.applied, true); + assert.notEqual(r.body.messages?.[0]?.role, "system"); + } }); diff --git a/tests/unit/compression/outputMode.test.ts b/tests/unit/compression/outputMode.test.ts index 1a3072b4f6..6697cb9c91 100644 --- a/tests/unit/compression/outputMode.test.ts +++ b/tests/unit/compression/outputMode.test.ts @@ -4,6 +4,7 @@ import { applyCavemanOutputMode, buildCavemanOutputInstruction, CAVEMAN_INSTRUCTION_BY_LANGUAGE, + placeSystemInstruction, shouldBypassCavemanOutputMode, } from "../../../open-sse/services/compression/outputMode.ts"; @@ -14,8 +15,10 @@ describe("Caveman output mode", () => { { enabled: true, intensity: "full", autoClarity: true } ); assert.equal(result.applied, true); - assert.equal(result.body.messages?.[0]?.role, "system"); - assert.match(String(result.body.messages?.[0]?.content), /Caveman Output Mode/); + // Trailing placement — never a synthetic system message at messages[0] (#12584). + assert.equal(result.body.messages?.[0]?.role, "user"); + assert.equal(result.body.messages?.at(-1)?.role, "system"); + assert.match(String(result.body.messages?.at(-1)?.content), /Caveman Output Mode/); }); it("appends to an existing system prompt", () => { @@ -59,6 +62,52 @@ describe("Caveman output mode", () => { assert.equal(markerCount, 1); }); + it("merges into an Anthropic top-level system field instead of messages[0]", () => { + const result = applyCavemanOutputMode( + { system: "You are Claude Code.", messages: [{ role: "user", content: "hi" }] }, + { enabled: true, intensity: "full", autoClarity: true } + ); + assert.equal(result.applied, true); + assert.match(String(result.body.system), /Caveman Output Mode/); + assert.equal(result.body.messages?.length, 1); + assert.equal(result.body.messages?.[0]?.role, "user"); + + const twice = applyCavemanOutputMode(result.body, { + enabled: true, + intensity: "full", + autoClarity: true, + }); + assert.equal(twice.applied, false); + assert.equal(twice.skippedReason, "already_applied"); + }); + + it("appends a block to an Anthropic block-array system and does not re-apply", () => { + const result = applyCavemanOutputMode( + { + system: [{ type: "text", text: "You are Claude Code." }], + messages: [{ role: "user", content: "hi" }], + }, + { enabled: true, intensity: "full", autoClarity: true } + ); + assert.equal(result.applied, true); + const blocks = result.body.system as Array<{ type: string; text: string }>; + assert.ok(Array.isArray(blocks)); + assert.equal(blocks.length, 2); + assert.equal(blocks[0]?.text, "You are Claude Code."); + assert.match(String(blocks[1]?.text), /Caveman Output Mode/); + assert.equal(result.body.messages?.length, 1); + assert.equal(result.body.messages?.[0]?.role, "user"); + + const twice = applyCavemanOutputMode(result.body, { + enabled: true, + intensity: "full", + autoClarity: true, + }); + assert.equal(twice.applied, false); + assert.equal(twice.skippedReason, "already_applied"); + assert.equal((twice.body.system as unknown[]).length, 2); + }); + it("does not modify user content", () => { const body = { messages: [{ role: "user", content: "Please explain this response." }] }; const result = applyCavemanOutputMode(body, { @@ -66,7 +115,7 @@ describe("Caveman output mode", () => { intensity: "full", autoClarity: true, }); - assert.equal(result.body.messages?.at(-1)?.content, body.messages[0].content); + assert.equal(result.body.messages?.[0]?.content, body.messages[0].content); }); it("uses Responses instructions when input has no messages", () => { @@ -123,3 +172,60 @@ describe("caveman instruction language map", () => { } }); }); + +describe("placeSystemInstruction", () => { + it("routes to `system` instead of messages[0] when messages is empty (#12584)", () => { + const result = placeSystemInstruction([], undefined, "be terse"); + assert.equal(result.system, "be terse"); + assert.equal(result.messages, undefined); + }); + + it("merges into a system message at index >= 1 and leaves messages[0] untouched", () => { + const messages = [ + { role: "user", content: "hi" }, + { role: "system", content: "You are terse." }, + { role: "assistant", content: "ok" }, + ]; + const result = placeSystemInstruction(messages, undefined, "be terse"); + assert.equal(result.system, undefined); + assert.equal(result.messages?.length, 3); + assert.equal(result.messages?.[0]?.content, "hi"); + assert.equal(result.messages?.[1]?.role, "system"); + assert.equal(result.messages?.[1]?.content, "You are terse.\n\nbe terse"); + assert.equal(result.messages?.[2]?.content, "ok"); + // input array and its members are not mutated + assert.notEqual(result.messages, messages); + assert.equal(messages[1]?.content, "You are terse."); + }); + + it("skips a block-content system message and merges into the next string system message", () => { + const result = placeSystemInstruction( + [ + { role: "system", content: [{ type: "text", text: "blocks" }] }, + { role: "system", content: "You are terse." }, + ], + undefined, + "be terse" + ); + assert.equal(result.system, undefined); + assert.equal(result.messages?.length, 2); + assert.deepEqual(result.messages?.[0]?.content, [{ type: "text", text: "blocks" }]); + assert.equal(result.messages?.[1]?.content, "You are terse.\n\nbe terse"); + }); + + it("appends a trailing system message when there is no system field or system message", () => { + const messages = [ + { role: "user", content: "hi" }, + { role: "assistant", content: "ok" }, + ]; + const result = placeSystemInstruction(messages, undefined, "be terse"); + assert.equal(result.system, undefined); + assert.equal(result.messages?.length, 3); + assert.equal(result.messages?.[0]?.role, "user"); + assert.equal(result.messages?.[1]?.role, "assistant"); + assert.equal(result.messages?.[2]?.role, "system"); + assert.equal(result.messages?.[2]?.content, "be terse"); + // instruction is appended, never prepended as a new messages[0] + assert.equal(messages.length, 2); + }); +}); diff --git a/tests/unit/custom-endpoint-system-prompt.test.ts b/tests/unit/custom-endpoint-system-prompt.test.ts index f208c8f7cc..8c30a71f45 100644 --- a/tests/unit/custom-endpoint-system-prompt.test.ts +++ b/tests/unit/custom-endpoint-system-prompt.test.ts @@ -17,9 +17,7 @@ import test from "node:test"; import assert from "node:assert/strict"; -const { injectCustomSystemPrompt } = await import( - "../../open-sse/services/systemPrompt.ts" -); +const { injectCustomSystemPrompt } = await import("../../open-sse/services/systemPrompt.ts"); // ─── injectCustomSystemPrompt ──────────────────────────────────────────────── @@ -144,13 +142,31 @@ test("settings defaults include customSystemPromptEnabled=false and customSystem false, "customSystemPromptEnabled default is false" ); - assert.equal( - settings.customSystemPrompt, - "", - "customSystemPrompt default is empty string" - ); + assert.equal(settings.customSystemPrompt, "", "customSystemPrompt default is empty string"); t.after(() => { resetDbInstance(); }); }); + +test("injectCustomSystemPrompt: claude-shaped body keeps prompt out of messages[0] (#12584)", () => { + const body = { + system: "You are Claude Code.", + messages: [{ role: "user", content: "Hello" }], + }; + const result = injectCustomSystemPrompt(body, "Always respond formally."); + assert.equal(result.messages.length, 1); + assert.equal(result.messages[0].role, "user"); + assert.ok(String(result.system).includes("Always respond formally.")); +}); + +test("injectCustomSystemPrompt: malformed null system still receives the prompt (#12584)", () => { + const body = { + system: null, + messages: [{ role: "user", content: "Hello" }], + }; + const result = injectCustomSystemPrompt(body, "Always respond formally."); + assert.equal(result.system, "Always respond formally.", "prompt is not silently dropped"); + assert.equal(result.messages.length, 1, "prompt does not leak into messages"); + assert.equal(result.messages[0].role, "user"); +}); diff --git a/tests/unit/system-prompt.test.ts b/tests/unit/system-prompt.test.ts index 7fb95fcb53..59a05948ce 100644 --- a/tests/unit/system-prompt.test.ts +++ b/tests/unit/system-prompt.test.ts @@ -293,3 +293,31 @@ test("postTranslation: codex regression — suffix lands on LAST developer after // Reset test.after(() => setSystemPromptConfig({ enabled: false, prefixPrompt: "", suffixPrompt: "" })); + +test("injectSystemPrompt: claude-shaped body keeps prompt out of messages[0] (#12584)", () => { + setSystemPromptConfig({ enabled: true, prefixPrompt: "PRE", suffixPrompt: "SUF" }); + const body = { + system: "You are Claude Code.", + messages: [{ role: "user", content: "hi" }], + }; + const result = injectSystemPrompt(body); + assert.equal(result.messages.length, 1); + assert.equal(result.messages[0].role, "user"); + assert.ok(String(result.system).includes("You are Claude Code.")); + assert.ok(String(result.system).includes("PRE")); + assert.ok(String(result.system).includes("SUF")); +}); + +test("injectSystemPrompt: malformed null system still receives the prompt (#12584)", () => { + setSystemPromptConfig({ enabled: true, prefixPrompt: "PRE", suffixPrompt: "SUF" }); + const body = { + system: null, + messages: [{ role: "user", content: "hi" }], + }; + const result = injectSystemPrompt(body); + assert.equal(typeof result.system, "string", "null system is normalized, not skipped"); + assert.ok(String(result.system).includes("PRE"), "prefix is not silently dropped"); + assert.ok(String(result.system).includes("SUF"), "suffix is not silently dropped"); + assert.equal(result.messages.length, 1, "prompt does not leak into messages"); + assert.equal(result.messages[0].role, "user"); +});