diff --git a/.env.example b/.env.example index 28a9f04b8f..32ad544497 100644 --- a/.env.example +++ b/.env.example @@ -2397,6 +2397,13 @@ APP_LOG_TO_FILE=true # Used by: open-sse/executors/cursor.ts. # CURSOR_TOOL_DIRECTIVE=1 +# Operator-defined system prompt text appended to the system message AFTER +# translation (post-translation injection), so it reaches codex/Responses and +# /v1/messages paths. Also used as the directive prefix stripped from echoed +# system preamble blocks. Leave unset to disable. +# Used by: open-sse/translator/request/claude-to-openai.ts, open-sse/translator/response/openai-to-claude.ts. +# OMNIROUTE_SYSTEM_INSTRUCTION_APPEND= + # Per-image fetch timeout (ms) for remote image_url vision input. Default: 15000. # Used by: open-sse/utils/cursorImages.ts. # CURSOR_IMAGE_FETCH_TIMEOUT_MS=15000 diff --git a/changelog.d/fixes/12904-inject-global-system-prompt-post-translation.md b/changelog.d/fixes/12904-inject-global-system-prompt-post-translation.md new file mode 100644 index 0000000000..d4bdb433bf --- /dev/null +++ b/changelog.d/fixes/12904-inject-global-system-prompt-post-translation.md @@ -0,0 +1 @@ +- fix(sse): inject the operator's global system prompt once, after request translation, for every target shape (Claude, Gemini, OpenAI Responses, OpenAI/Codex messages) instead of before translation — the pre-translation injection could be lost, repositioned, or duplicated 2-3× depending on the target format, and never reached the Responses API path at all. The new `injectSystemPromptPostTranslation()` is idempotent per request via a non-enumerable marker (#12904) diff --git a/docs/reference/ENVIRONMENT.md b/docs/reference/ENVIRONMENT.md index 0cd8db6e45..2ada70c885 100644 --- a/docs/reference/ENVIRONMENT.md +++ b/docs/reference/ENVIRONMENT.md @@ -1185,6 +1185,7 @@ changing them requires a code edit, not an env var: | `CURSOR_STREAM_TIMEOUT_MS` | `300000` | `open-sse/executors/cursor.ts` | Stream idle timeout (ms) for the Cursor executor. | | `CURSOR_KV_GRACE_MS` | `2000` | `open-sse/executors/cursor.ts` | Grace window (ms) after a composer kv_after_text soft terminator when bytes remain buffered — gives a trailing exec_mcp tool call time to complete its frame. | | `CURSOR_TOOL_DIRECTIVE` | enabled (`!== "0"`) | `open-sse/executors/cursor.ts` | Tool-commit directive that makes composer-2.5 reliably issue tool calls. Set `0` to disable. | +| `OMNIROUTE_SYSTEM_INSTRUCTION_APPEND` | _(unset)_ | `open-sse/translator/request/claude-to-openai.ts`, `open-sse/translator/response/openai-to-claude.ts` | Operator-defined system prompt text appended to the system message AFTER translation (post-translation injection), reaching codex/Responses and `/v1/messages` paths. Also used as the directive prefix stripped from echoed system preamble blocks. Leave unset to disable. | | `CURSOR_IMAGE_FETCH_TIMEOUT_MS` | `15000` | `open-sse/utils/cursorImages.ts` | Per-image fetch timeout (ms) for remote `image_url` vision input. | | `CURSOR_STATE_DB_PATH` | _(probed)_ | `open-sse/utils/cursorVersionDetector.ts` | Override the Cursor IDE state DB lookup used for IDE version detection. | | `CURSOR_AGENT_CLI_VERSION` | _(detect / pin)_ | `open-sse/utils/cursorAgentCliVersion.ts` | Agent CLI build id (`YYYY.MM.DD-`) for `x-cursor-client-version: cli-…` on Agent Run. | diff --git a/open-sse/handlers/chatCore.ts b/open-sse/handlers/chatCore.ts index d86df398c3..02504fa2ab 100644 --- a/open-sse/handlers/chatCore.ts +++ b/open-sse/handlers/chatCore.ts @@ -123,7 +123,11 @@ import { resolveChatCoreTargetFormat } from "./chatCore/targetFormat.ts"; import { resolveOmniGlyphTransport } from "../services/compression/imageTransportPolicy.ts"; import { stripStore, usesClaudeBridge } from "./chatCore/agentRouterProtocol.ts"; import { normalizeClaudeToolsForDispatch } from "./chatCore/claudeToolDefaults.ts"; -import { injectSystemPrompt, injectCustomSystemPrompt } from "../services/systemPrompt.ts"; +import { + injectCustomSystemPrompt, + injectSystemPromptPostTranslation, + injectSystemPromptPreTranslation, +} from "../services/systemPrompt.ts"; import { translateRequest, needsTranslation } from "../translator/index.ts"; import { applyReasoningRuleDirective } from "@/lib/reasoningRouting/policy"; import { withReasoningRuleContext } from "../utils/reasoningRuleContext.ts"; @@ -595,7 +599,6 @@ export async function handleChatCore({ }; }; let tokensCompressed: number | null = null; - body = injectSystemPrompt(body); // ── Per-endpoint custom system prompt (port of upstream #2063) ── // Reads from cachedSettings if available (passed in from combo/chat layer) // to avoid an extra DB read on the hot path. Falls through to getCachedSettings() @@ -2505,6 +2508,12 @@ export async function handleChatCore({ model || "", sourceFormat ); + // Carrier-less targets (kiro / antigravity) have no post-translation + // system carrier for the single pass at ~3068 to write into — inject + // into the client body BEFORE translation so their user-merge / + // relocation paths carry the global prompt (baseline coverage of the + // removed pre-translation pass). The gate writes ONE carrier only. + translatedBody = injectSystemPromptPreTranslation(translatedBody, { targetFormat }); translatedBody = translateRequest( sourceFormat, targetFormat, @@ -3097,6 +3106,19 @@ export async function handleChatCore({ isOpencodeClient, }); + // Global System Prompt — SINGLE injection point (post-translation) for + // carrier-ful targets. The old unconditional pre-translation pass + // (former chatCore injectSystemPrompt call) was removed: it chained + // with this pass to inject prefix/suffix 2-3x and dual-wrote + // body.system + messages[] on the claude path, which strict upstreams + // (HCP-Vision vLLM: "System message must be at the beginning") reject + // with 400. Format-aware via targetFormat: messages[] (openai/codex — + // prefix FIRST system, suffix LAST), claude `system` field, gemini + // `systemInstruction`, responses `instructions`. Carrier-less targets + // (kiro user-fold, antigravity Cloud Code envelope) are covered by the + // gated PRE-translation pass before translateRequest instead. + bodyToSend = injectSystemPromptPostTranslation(bodyToSend, { targetFormat }); + updatePendingScope(pendingScope, { providerRequest: bodyToSend, stage: "payload_prepared", diff --git a/open-sse/services/systemPrompt.ts b/open-sse/services/systemPrompt.ts index 8f4ef69a86..01e011eadd 100644 --- a/open-sse/services/systemPrompt.ts +++ b/open-sse/services/systemPrompt.ts @@ -93,6 +93,7 @@ export function injectSystemPrompt(body: T): T { if (!prefix && !suffix) return body; if (!isRecord(body)) return body; if (body._skipSystemPrompt) return body; + if (body._systemPromptInjected) return body; const result: Record = { ...body }; @@ -143,9 +144,305 @@ export function injectSystemPrompt(body: T): T { } } + markInjected(result); return Object.assign({}, body, result); } +/** + * Prepend `text` to a message content (string or array form). + */ +function prependToContent(msg: Record, text: string): void { + if (Array.isArray(msg.content)) { + msg.content = [{ type: "text", text }, ...msg.content]; + } else { + msg.content = text + "\n\n" + (msg.content || ""); + } +} + +/** + * Append `text` to a message content (string or array form). + */ +function appendToContent(msg: Record, text: string): void { + if (Array.isArray(msg.content)) { + msg.content = [...msg.content, { type: "text", text }]; + } else { + msg.content = (msg.content || "") + "\n\n" + text; + } +} + +// Non-enumerable marker: survives property access for the retry-loop guard, +// invisible to JSON.stringify so it never leaks into the upstream request body. +function markInjected(body: Record): void { + try { + Object.defineProperty(body, "_systemPromptInjected", { value: true, enumerable: false }); + } catch { + /* frozen/non-object edge — ignore */ + } +} + +/** + * Inject system prompts into a POST-TRANSLATION request body. + * + * Coverage model (the legacy unconditional pre-translation pass was removed — + * it chained into double injection): coverage = this format-aware + * post-translation pass for carrier-ful targets, plus the gated + * PRE-translation pass (injectSystemPromptPreTranslation) for carrier-less + * targets (kiro user-fold, antigravity Cloud Code envelope). + * + * Format-aware (opts.targetFormat) system carriers per target: + * - claude: `system` field (string or {type:"text"} block array) + * - gemini: `systemInstruction` ({ role, parts: [{ text }] }) + * - openai-responses: `instructions` string + * - openai/codex (default): messages[] system/developer roles — prefix on + * the FIRST and suffix on the LAST so the suffix retains the highest + * recency position, preserving the "After Prompt" semantics. + * + * @param {object} body - Translated request body (target shape resolved) + * @param {object} [opts] - `{ targetFormat }` from the resolved wire target + * @returns {object} Modified body + */ +export function injectSystemPromptPostTranslation(body, opts?: { targetFormat?: string }) { + const cfg = getConfig(); + if (!cfg.enabled) return body; + const prefix = cfg.prefixPrompt || ""; + const suffix = cfg.suffixPrompt || ""; + if (!prefix && !suffix) return body; + if (!body || typeof body !== "object") return body; + if (body._skipSystemPrompt) return body; + if (body._systemPromptInjected) return body; + const targetFormat = opts?.targetFormat || ""; + const combined = [prefix, suffix].filter(Boolean).join("\n\n"); + + const result = { ...body }; + + // Claude-format body (separate `system` field, or a claude target whose + // translated body has no system-role message to carry the prompt): inject + // into body.system — a system-role message inside claude messages[] is + // invalid there. When the translated body has no system field at all, CREATE + // it (combined) — previously this body shape fell through the messages[] + // early-return and silently got zero injection. + if (targetFormat === "claude" || result.system !== undefined) { + const hasSystemRole = + Array.isArray(result.messages) && + result.messages.some((m) => m && (m.role === "system" || m.role === "developer")); + if (!hasSystemRole) { + if (typeof result.system === "string") { + let sys = result.system; + if (prefix) sys = prefix + "\n\n" + sys; + if (suffix) sys = sys + "\n\n" + suffix; + result.system = sys; + } else if (Array.isArray(result.system)) { + let arr = [...result.system]; + if (prefix) arr = [{ type: "text", text: prefix }, ...arr]; + if (suffix) arr = [...arr, { type: "text", text: suffix }]; + result.system = arr; + } else { + result.system = combined; + } + markInjected(result); + return result; + } + } + + // Gemini-format body (contents[] + systemInstruction): inject into the + // systemInstruction parts (real translator shape: { role: "system", + // parts: [{ text }] }, see translator/request/claude-to-gemini.ts:95). If + // absent, create it — a messages-less gemini body previously fell through + // the messages[] early-return and silently got zero injection. + // Antigravity reaches 3068 as a Cloud Code envelope whose executor reads + // ONLY envelope.request (antigravity.ts:733) and which rejects unknown + // top-level fields with 400 (:813-815) — its coverage is restored by the + // gated pre-translation pass instead, so it must stay out of this branch. + if (targetFormat === "gemini" && !result.request) { + if (result.systemInstruction && typeof result.systemInstruction === "object") { + const si = result.systemInstruction as { role?: string; parts?: unknown[] }; + const parts = Array.isArray(si.parts) ? [...si.parts] : []; + if (prefix) parts.unshift({ text: prefix }); + if (suffix) parts.push({ text: suffix }); + result.systemInstruction = { ...si, role: si.role || "system", parts }; + } else { + const texts = [prefix, suffix].filter(Boolean); + result.systemInstruction = { role: "system", parts: texts.map((text) => ({ text })) }; + } + markInjected(result); + return result; + } + + // OpenAI Responses-format body (input + instructions): instructions is a + // plain string — wrap once. If absent, create it with the combined prompt. + // Do NOT touch `input` (message items, not a system carrier). + if (targetFormat === "openai-responses") { + const base = typeof result.instructions === "string" ? result.instructions : ""; + const parts = [prefix, base, suffix].filter(Boolean); + result.instructions = parts.join("\n\n"); + markInjected(result); + return result; + } + + // Kiro (conversationState/.../userInputMessage) has NO system carrier at all: + // openai-to-kiro.ts folds system messages into user turns wrapped in + // tags (#2306) and the executor keeps no system slot. + // Injection here would require inventing a carrier kiro upstreams reject — + // so kiro intentionally receives no global-prompt injection at this seam. + if (targetFormat === "kiro") { + return result; + } + + if (!result.messages || !Array.isArray(result.messages)) return result; + + result.messages = [...result.messages]; + const indices: number[] = []; + for (let i = 0; i < result.messages.length; i++) { + const m = result.messages[i] as { role?: string }; + if (m && (m.role === "system" || m.role === "developer")) indices.push(i); + } + + if (indices.length === 0) { + // No system message — combine both into one at the front (same as injectSystemPrompt). + if (combined) { + result.messages = [{ role: "system", content: combined }, ...result.messages]; + } + markInjected(result); + return result; + } + + if (prefix) { + const firstIdx = indices[0]; + result.messages[firstIdx] = { ...result.messages[firstIdx] }; + prependToContent(result.messages[firstIdx] as Record, prefix); + } + if (suffix) { + const lastIdx = indices[indices.length - 1]; + if (lastIdx !== indices[0]) { + result.messages[lastIdx] = { ...result.messages[lastIdx] }; + } + appendToContent(result.messages[lastIdx] as Record, suffix); + } + markInjected(result); + return result; +} + +/** + * Gated PRE-translation injection for targets with NO post-translation system + * carrier. Two such targets exist: + * - kiro: openai-to-kiro.ts folds system messages into user turns wrapped in + * tags (#2306) — reads body.messages system roles only + * (:283-284/:872), no body.system, no system slot in the Kiro payload. + * - antigravity: the translator wraps the payload in a Cloud Code envelope + * ({project, requestId, request:{contents, systemInstruction, ...}}) and + * the executor reads ONLY envelope.request (antigravity.ts:733/:417-425); + * the envelope rejects unknown top-level fields with 400 (:813-815), and + * envelope.request.systemInstruction is overwritten with + * ANTIGRAVITY_DEFAULT_SYSTEM after relocating client system content into + * the first user message (openai-to-gemini.ts:716-730). Post-translation + * injection at chatCore 3068 cannot reach the real carrier for either. + * + * Pre-translation the client body reaches this gate in one of four shapes, + * ALL covered here: messages[] (openai/codex source), claude `system` field + * (string), responses `input` + `instructions` (hub translation promotes + * instructions to a system message, openai-responses.ts:205-207), and gemini + * `contents` + `systemInstruction`. Not covered — and rejected by the guards + * above — are bodies with none of these carriers (empty/no-op return). + * + * SINGLE-CARRIER guarantee: writes into exactly ONE carrier — never both. The + * removed pass dual-wrote messages[] AND body.system; both would survive + * translation and fold ×2. + * + * @param {object} body - PRE-translation request body (client format) + * @param {object} [opts] - `{ targetFormat }` of the resolved wire target + * @returns {object} Modified body (or the original when gated out) + */ +export function injectSystemPromptPreTranslation(body, opts?: { targetFormat?: string }) { + const cfg = getConfig(); + if (!cfg.enabled) return body; + const prefix = cfg.prefixPrompt || ""; + const suffix = cfg.suffixPrompt || ""; + if (!prefix && !suffix) return body; + if (!body || typeof body !== "object") return body; + if (body._skipSystemPrompt) return body; + if (body._systemPromptInjected) return body; + + const targetFormat = opts?.targetFormat || ""; + // Carrier-ful targets (openai, codex, claude, gemini, openai-responses, + // cursor) receive their injection at the single post-translation pass + // (chatCore 3068) — pre-injecting here would chain into a double injection. + const CARRIERLESS_TARGETS = new Set(["kiro", "antigravity"]); + if (!CARRIERLESS_TARGETS.has(targetFormat)) return body; + + const combined = [prefix, suffix].filter(Boolean).join("\n\n"); + const result = { ...body }; + + // Claude-source client body: the `system` field is the authoritative carrier + // (#2468 ordering — prefix → client content → suffix). Checked FIRST so a + // body that also carries messages[] (user/assistant turns) never gets a + // second write into messages. + if (typeof result.system === "string") { + let sys = result.system; + if (prefix) sys = prefix + "\n\n" + sys; + if (suffix) sys = sys + "\n\n" + suffix; + result.system = sys; + markInjected(result); + return result; + } + if (Array.isArray(result.system)) { + let arr = [...result.system]; + if (prefix) arr = [{ type: "text", text: prefix }, ...arr]; + if (suffix) arr = [...arr, { type: "text", text: suffix }]; + result.system = arr; + markInjected(result); + return result; + } + + // Responses-source client body (input + instructions): wrap the instructions + // string once — the hub translation promotes it to a system message + // (openai-responses.ts:205-207) which the target then folds. Do NOT touch + // `input` (message items, not a system carrier). + if (Array.isArray(result.input)) { + const base = typeof result.instructions === "string" ? result.instructions : ""; + result.instructions = [prefix, base, suffix].filter(Boolean).join("\n\n"); + markInjected(result); + return result; + } + + // Gemini-source client body (contents + systemInstruction): inject into the + // parts once each; create the carrier when absent. + if (result.contents !== undefined) { + if (result.systemInstruction && typeof result.systemInstruction === "object") { + const si = result.systemInstruction as { role?: string; parts?: unknown[] }; + const parts = Array.isArray(si.parts) ? [...si.parts] : []; + if (prefix) parts.unshift({ text: prefix }); + if (suffix) parts.push({ text: suffix }); + result.systemInstruction = { ...si, role: si.role || "system", parts }; + } else { + const texts = [prefix, suffix].filter(Boolean); + result.systemInstruction = { role: "system", parts: texts.map((text) => ({ text })) }; + } + markInjected(result); + return result; + } + + // OpenAI-style client body: write into the system/developer message only. + if (Array.isArray(result.messages)) { + result.messages = [...result.messages]; + const sysIdx = result.messages.findIndex( + (m) => m && (m.role === "system" || m.role === "developer") + ); + if (sysIdx >= 0) { + result.messages[sysIdx] = { ...result.messages[sysIdx] }; + if (prefix) prependToContent(result.messages[sysIdx] as Record, prefix); + if (suffix) appendToContent(result.messages[sysIdx] as Record, suffix); + } else { + if (combined) { + result.messages = [{ role: "system", content: combined }, ...result.messages]; + } + } + markInjected(result); + return result; + } + + return result; +} + /** * Inject a per-request custom system prompt into the request body. * diff --git a/open-sse/translator/request/claude-to-openai.ts b/open-sse/translator/request/claude-to-openai.ts index 68a19558a0..f8883a9e62 100644 --- a/open-sse/translator/request/claude-to-openai.ts +++ b/open-sse/translator/request/claude-to-openai.ts @@ -165,6 +165,30 @@ export function claudeToOpenAIRequest(model, body, stream, credentials: unknown } } + // #reasoning-bilingual: DeepSeek-V4 and similar models emit user-facing text in the + // user's language (e.g. Korean) then continue with English planning/chain-of-thought + // in the same content field instead of using reasoning_content. Same mitigation as + // translator/response/openai-to-claude.ts's directivePreambleStripper.ts: when the + // operator configured OMNIROUTE_SYSTEM_INSTRUCTION_APPEND, append it here to the + // (system) message so the directive reaches the model on the /v1/messages (Claude + // Messages -> OpenAI Chat Completions) path too. + const systemAppend = process.env.OMNIROUTE_SYSTEM_INSTRUCTION_APPEND?.trim(); + if (systemAppend) { + const sysIndex = result.messages.findIndex((m) => m.role === "system"); + if (sysIndex >= 0) { + const sys = result.messages[sysIndex]; + if (typeof sys.content === "string") { + sys.content = sys.content + "\n\n" + systemAppend; + } else if (Array.isArray(sys.content)) { + (sys.content as JsonRecord[]).push({ type: "text", text: systemAppend }); + } else { + sys.content = systemAppend; + } + } else { + result.messages.unshift({ role: "system", content: systemAppend }); + } + } + // Convert messages if (body.messages && Array.isArray(body.messages)) { for (let i = 0; i < body.messages.length; i++) { diff --git a/tests/unit/global-prompt-single-injection.test.ts b/tests/unit/global-prompt-single-injection.test.ts new file mode 100644 index 0000000000..d06dd55f59 --- /dev/null +++ b/tests/unit/global-prompt-single-injection.test.ts @@ -0,0 +1,379 @@ +import test from "node:test"; +import { strict as assert } from "node:assert"; + +const { + setSystemPromptConfig, + injectSystemPromptPostTranslation, + injectSystemPromptPreTranslation, +} = await import("../../open-sse/services/systemPrompt.ts"); + +const PREFIX = "PREFIX-RULES"; +const SUFFIX = "SUFFIX-RULES"; + +function resetConfig() { + setSystemPromptConfig({ enabled: true, prefixPrompt: PREFIX, suffixPrompt: SUFFIX }); +} + +function countOccurrences(s, needle) { + return s.split(needle).length - 1; +} + +test("idempotence: second application is a no-op (ONE copy of prefix/suffix)", () => { + resetConfig(); + const body = { + messages: [ + { role: "system", content: "ORIG" }, + { role: "user", content: "hi" }, + ], + }; + const once = injectSystemPromptPostTranslation(body); + const twice = injectSystemPromptPostTranslation(once); + assert.equal(twice.messages.length, once.messages.length, "no message growth on second pass"); + assert.equal(countOccurrences(String(twice.messages[0].content), PREFIX), 1); + assert.equal(countOccurrences(String(twice.messages[0].content), SUFFIX), 1); +}); + +test("no client system -> exactly one combined system inserted", () => { + resetConfig(); + const body = { messages: [{ role: "user", content: "hi" }] }; + const out = injectSystemPromptPostTranslation(body); + const systems = out.messages.filter((m) => m.role === "system"); + assert.equal(systems.length, 1); + assert.ok(String(systems[0].content).startsWith(PREFIX)); + assert.ok(String(systems[0].content).endsWith(SUFFIX)); +}); + +test("claude-format body: system field gets injection, messages untouched", () => { + resetConfig(); + const body = { system: "CLIENT", messages: [{ role: "user", content: "hi" }] }; + const out = injectSystemPromptPostTranslation(body); + assert.equal(out.system, "PREFIX-RULES\n\nCLIENT\n\nSUFFIX-RULES"); + assert.ok( + !out.messages.some((m) => m.role === "system"), + "no system-role message inside claude messages" + ); +}); + +test("idempotence flag does not leak into upstream JSON", () => { + resetConfig(); + const body = { messages: [{ role: "user", content: "hi" }] }; + const out = injectSystemPromptPostTranslation(body); + assert.equal( + JSON.stringify(out).includes("_systemPromptInjected"), + false, + "flag must stay non-enumerable" + ); +}); + +test("multi-system codex semantics preserved: prefix on first, suffix on last", () => { + resetConfig(); + const body = { + messages: [ + { role: "system", content: "A" }, + { role: "developer", content: "B" }, + { role: "user", content: "hi" }, + ], + }; + const out = injectSystemPromptPostTranslation(body); + assert.equal(countOccurrences(String(out.messages[0].content), PREFIX), 1); + assert.equal(countOccurrences(String(out.messages[1].content), SUFFIX), 1); +}); + +test("skip flag respected", () => { + resetConfig(); + const body = { _skipSystemPrompt: true, messages: [{ role: "user", content: "hi" }] }; + const out = injectSystemPromptPostTranslation(body); + assert.equal(out.messages.length, 1, "no injection when _skipSystemPrompt"); +}); + +test("claude-format body WITHOUT system field: combined goes to body.system, not messages", () => { + resetConfig(); + const body = { messages: [{ role: "user", content: "hi" }] }; + const out = injectSystemPromptPostTranslation(body, { targetFormat: "claude" }); + assert.equal(out.system, "PREFIX-RULES\n\nSUFFIX-RULES"); + assert.ok( + !out.messages.some((m) => m.role === "system"), + "claude target must never get a system-role message in messages" + ); +}); + +test("claude-format array system: prefix/suffix as text blocks, once each", () => { + resetConfig(); + const body = { + system: [{ type: "text", text: "CLIENT" }], + messages: [{ role: "user", content: "hi" }], + }; + const out = injectSystemPromptPostTranslation(body, { targetFormat: "claude" }); + assert.deepEqual(out.system, [ + { type: "text", text: "PREFIX-RULES" }, + { type: "text", text: "CLIENT" }, + { type: "text", text: "SUFFIX-RULES" }, + ]); + assert.ok(!out.messages.some((m) => m.role === "system")); +}); + +test("gemini-format body: systemInstruction parts get prefix/suffix once each", () => { + resetConfig(); + // Real shape produced by open-sse/translator/request/claude-to-gemini.ts:95-97 + // and openai-to-gemini.ts:350-356: { role: "system", parts: [{ text }] } + const body = { + contents: [{ role: "user", parts: [{ text: "hi" }] }], + systemInstruction: { role: "system", parts: [{ text: "CLIENT" }] }, + }; + const out = injectSystemPromptPostTranslation(body, { targetFormat: "gemini" }); + const texts = out.systemInstruction.parts.map((p) => p.text); + assert.equal(texts.filter((t) => t === "PREFIX-RULES").length, 1); + assert.equal(texts.filter((t) => t === "SUFFIX-RULES").length, 1); + assert.ok(texts.includes("CLIENT")); +}); + +test("gemini-format body without systemInstruction: combined systemInstruction created", () => { + resetConfig(); + const body = { contents: [{ role: "user", parts: [{ text: "hi" }] }] }; + const out = injectSystemPromptPostTranslation(body, { targetFormat: "gemini" }); + const texts = out.systemInstruction.parts.map((p) => p.text).join("|"); + assert.equal(countOccurrences(texts, "PREFIX-RULES"), 1); + assert.equal(countOccurrences(texts, "SUFFIX-RULES"), 1); + assert.equal(out.contents.length, 1, "contents untouched"); +}); + +test("responses-format body: instructions wrapped once, input untouched", () => { + resetConfig(); + // Real targetFormat value is FORMATS.OPENAI_RESPONSES = "openai-responses" + // (open-sse/translator/formats.ts), not "responses". + const body = { + model: "m", + input: [{ type: "message", role: "user", content: [{ type: "input_text", text: "hi" }] }], + instructions: "INSTR", + }; + const out = injectSystemPromptPostTranslation(body, { targetFormat: "openai-responses" }); + assert.equal(out.instructions, "PREFIX-RULES\n\nINSTR\n\nSUFFIX-RULES"); + assert.equal(out.input.length, 1); +}); + +test("responses-format body without instructions: combined instructions created", () => { + resetConfig(); + const body = { + model: "m", + input: [{ type: "message", role: "user", content: [{ type: "input_text", text: "hi" }] }], + }; + const out = injectSystemPromptPostTranslation(body, { targetFormat: "openai-responses" }); + assert.equal(out.instructions, "PREFIX-RULES\n\nSUFFIX-RULES"); +}); + +// ---- Round 2: gated PRE-translation pass for carrier-less targets ---- + +// kiro: openai-to-kiro.ts:283-284/872 reads body.messages system roles ONLY +// (no body.system carrier exists). The gate must write into messages[] exactly +// once so the translator's fold carries prefix+suffix once. +test("pre-translation gate: kiro target gets single messages[] injection", () => { + resetConfig(); + const body = { + messages: [ + { role: "system", content: "SYS" }, + { role: "user", content: "hi" }, + ], + }; + const out = injectSystemPromptPreTranslation(body, { targetFormat: "kiro" }); + const sysMsg = out.messages.find((m) => m.role === "system"); + assert.ok(sysMsg, "system message preserved"); + assert.equal(countOccurrences(String(sysMsg.content), PREFIX), 1); + assert.equal(countOccurrences(String(sysMsg.content), SUFFIX), 1); +}); + +test("pre-translation gate: kiro target with array system content gets text blocks", () => { + resetConfig(); + const body = { + messages: [ + { role: "system", content: [{ type: "text", text: "SYS" }] }, + { role: "user", content: "hi" }, + ], + }; + const out = injectSystemPromptPreTranslation(body, { targetFormat: "kiro" }); + const texts = out.messages[0].content.map((c) => c.text).join("|"); + assert.equal(countOccurrences(texts, "PREFIX-RULES"), 1); + assert.equal(countOccurrences(texts, "SUFFIX-RULES"), 1); +}); + +test("pre-translation gate: kiro target without system message inserts combined system", () => { + resetConfig(); + const body = { messages: [{ role: "user", content: "hi" }] }; + const out = injectSystemPromptPreTranslation(body, { targetFormat: "kiro" }); + assert.equal(out.messages[0].role, "system"); + assert.equal(countOccurrences(String(out.messages[0].content), PREFIX), 1); + assert.equal(countOccurrences(String(out.messages[0].content), SUFFIX), 1); +}); + +test("pre-translation gate: idempotent on second application", () => { + resetConfig(); + const body = { + messages: [ + { role: "system", content: "SYS" }, + { role: "user", content: "hi" }, + ], + }; + const once = injectSystemPromptPreTranslation(body, { targetFormat: "kiro" }); + const twice = injectSystemPromptPreTranslation(once, { targetFormat: "kiro" }); + assert.equal(twice.messages.length, once.messages.length); + assert.equal(countOccurrences(String(twice.messages[0].content), PREFIX), 1); +}); + +test("pre-translation gate: does NOT dual-write — system field wins, messages untouched", () => { + resetConfig(); + // Round-3 M-2: for a body carrying BOTH carriers, the claude-source `system` + // field is the authoritative one (#2468 ordering: prefix → client → suffix); + // messages must stay untouched so the wrap is never duplicated. + const body = { + system: "STRAY", + messages: [ + { role: "system", content: "SYS" }, + { role: "user", content: "hi" }, + ], + }; + const out = injectSystemPromptPreTranslation(body, { targetFormat: "kiro" }); + assert.equal( + out.system, + "PREFIX-RULES\n\nSTRAY\n\nSUFFIX-RULES", + "system field is the single carrier" + ); + assert.equal( + countOccurrences(String(out.messages[0].content), PREFIX), + 0, + "messages untouched — dual-write would duplicate upstream" + ); +}); + +test("pre-translation gate: no-op for carrier-ful targets (openai handled at 3068)", () => { + resetConfig(); + const body = { + messages: [ + { role: "system", content: "SYS" }, + { role: "user", content: "hi" }, + ], + }; + const out = injectSystemPromptPreTranslation(body, { targetFormat: "openai" }); + assert.equal( + countOccurrences(String(out.messages[0].content), PREFIX), + 0, + "openai must get its injection post-translation, not pre" + ); + assert.equal(out.messages[0].content, "SYS"); +}); + +test("pre-translation gate: no-op when disabled or no prompts configured", () => { + setSystemPromptConfig({ enabled: false, prefixPrompt: PREFIX, suffixPrompt: SUFFIX }); + const body = { messages: [{ role: "user", content: "hi" }] }; + const out = injectSystemPromptPreTranslation(body, { targetFormat: "kiro" }); + assert.equal(out.messages.length, 1); + setSystemPromptConfig({ enabled: true, prefixPrompt: "", suffixPrompt: "" }); + const out2 = injectSystemPromptPreTranslation(body, { targetFormat: "kiro" }); + assert.equal(out2.messages.length, 1); + resetConfig(); +}); + +// antigravity: translator wraps the payload in a Cloud Code envelope and the +// executor reads ONLY envelope.request (antigravity.ts:733/:417-425) — the +// envelope rejects unknown top-level fields with 400 (:813-815). A top-level +// systemInstruction created post-translation would be an invalid field, and +// envelope.request.systemInstruction is already pinned to +// ANTIGRAVITY_DEFAULT_SYSTEM (openai-to-gemini.ts:719) — so antigravity must be +// gated PRE-translation into client messages, same as kiro. +test("pre-translation gate: antigravity target gets single messages[] injection", () => { + resetConfig(); + const body = { + messages: [ + { role: "system", content: "SYS" }, + { role: "user", content: "hi" }, + ], + }; + const out = injectSystemPromptPreTranslation(body, { targetFormat: "antigravity" }); + const sysMsg = out.messages.find((m) => m.role === "system"); + assert.ok(sysMsg, "system message preserved"); + assert.equal(countOccurrences(String(sysMsg.content), PREFIX), 1); + assert.equal(countOccurrences(String(sysMsg.content), SUFFIX), 1); +}); + +test("pre-translation gate: antigravity without system message inserts combined", () => { + resetConfig(); + const body = { messages: [{ role: "user", content: "hi" }] }; + const out = injectSystemPromptPreTranslation(body, { targetFormat: "antigravity" }); + assert.equal(out.messages[0].role, "system"); + assert.equal(countOccurrences(String(out.messages[0].content), PREFIX), 1); + assert.equal(countOccurrences(String(out.messages[0].content), SUFFIX), 1); +}); + +test("gemini branch: antigravity envelope must NOT get a top-level systemInstruction created", () => { + resetConfig(); + // 3068 receives the envelope itself for antigravity; the gemini branch must + // not fabricate an invalid top-level systemInstruction on it. + const body = { + project: "p", + requestId: "r", + request: { contents: [{ role: "user", parts: [{ text: "hi" }] }] }, + }; + const out = injectSystemPromptPostTranslation(body, { targetFormat: "antigravity" }); + assert.equal( + out.systemInstruction, + undefined, + "no invalid top-level systemInstruction on a Cloud Code envelope" + ); + assert.equal(out.messages, undefined, "envelope has no messages to mutate"); +}); + +// ---- Round 3: source-shape coverage in the gate ---- + +// I-NEW-1: a responses-source client (e.g. /v1/responses falling back to a kiro +// connection) reaches the gate with {input, instructions} — no messages[], no +// system field. The hub translation (openai-responses -> openai, +// openai-responses.ts:205-207) promotes instructions to a system message, so +// wrapping instructions here reaches the kiro fold. +test("pre-translation gate: responses-source body gets instructions wrapped once, input untouched", () => { + resetConfig(); + const body = { + model: "m", + input: [{ type: "message", role: "user", content: [{ type: "input_text", text: "hi" }] }], + instructions: "INSTR", + }; + const out = injectSystemPromptPreTranslation(body, { targetFormat: "kiro" }); + assert.equal(out.instructions, "PREFIX-RULES\n\nINSTR\n\nSUFFIX-RULES"); + assert.equal(out.input.length, 1, "input untouched"); +}); + +test("pre-translation gate: responses-source without instructions creates combined", () => { + resetConfig(); + const body = { + model: "m", + input: [{ type: "message", role: "user", content: [{ type: "input_text", text: "hi" }] }], + }; + const out = injectSystemPromptPreTranslation(body, { targetFormat: "kiro" }); + assert.equal(out.instructions, "PREFIX-RULES\n\nSUFFIX-RULES"); + assert.equal(out.input.length, 1); +}); + +// gemini-source client (contents + systemInstruction) reaching a carrier-less +// target: inject into systemInstruction parts exactly once. +test("pre-translation gate: gemini-source body gets systemInstruction parts once each", () => { + resetConfig(); + const body = { + contents: [{ role: "user", parts: [{ text: "hi" }] }], + systemInstruction: { role: "system", parts: [{ text: "CLIENT" }] }, + }; + const out = injectSystemPromptPreTranslation(body, { targetFormat: "antigravity" }); + const texts = out.systemInstruction.parts.map((p) => p.text); + assert.equal(texts.filter((t) => t === "PREFIX-RULES").length, 1); + assert.equal(texts.filter((t) => t === "SUFFIX-RULES").length, 1); + assert.ok(texts.includes("CLIENT")); +}); + +test("pre-translation gate: gemini-source without systemInstruction creates combined parts", () => { + resetConfig(); + const body = { contents: [{ role: "user", parts: [{ text: "hi" }] }] }; + const out = injectSystemPromptPreTranslation(body, { targetFormat: "antigravity" }); + const texts = out.systemInstruction.parts.map((p) => p.text).join("|"); + assert.equal(countOccurrences(texts, "PREFIX-RULES"), 1); + assert.equal(countOccurrences(texts, "SUFFIX-RULES"), 1); + assert.equal(out.contents.length, 1, "contents untouched"); +}); + +// M3: reset config so this file's settings never leak into other test files. +test.after(() => setSystemPromptConfig({ enabled: false, prefixPrompt: "", suffixPrompt: "" })); diff --git a/tests/unit/system-instruction-append-claude-to-openai.test.ts b/tests/unit/system-instruction-append-claude-to-openai.test.ts new file mode 100644 index 0000000000..b7d81bdab7 --- /dev/null +++ b/tests/unit/system-instruction-append-claude-to-openai.test.ts @@ -0,0 +1,82 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +const { claudeToOpenAIRequest } = + await import("../../open-sse/translator/request/claude-to-openai.ts"); + +const APPEND_ENV = "OMNIROUTE_SYSTEM_INSTRUCTION_APPEND"; +const DIRECTIVE = "TEST-DIRECTIVE-ABC"; + +function baseBody(withSystem: boolean) { + const body: Record = { + messages: [{ role: "user", content: [{ type: "text", text: "Hello" }] }], + }; + if (withSystem) body.system = [{ text: "Rule A" }]; + return body; +} + +function withEnv(directive: string | undefined, fn: () => void) { + const prev = process.env[APPEND_ENV]; + if (directive === undefined) delete process.env[APPEND_ENV]; + else process.env[APPEND_ENV] = directive; + try { + fn(); + } finally { + if (prev === undefined) delete process.env[APPEND_ENV]; + else process.env[APPEND_ENV] = prev; + } +} + +test("claude-to-openai appends the directive to an existing string system message (tail recency)", () => { + withEnv(DIRECTIVE, () => { + const result = claudeToOpenAIRequest("deepseek-v4", baseBody(true), false); + const sys = result.messages[0] as { role: string; content: unknown }; + assert.equal(sys.role, "system"); + assert.equal(typeof sys.content, "string"); + const content = sys.content as string; + assert.ok(content.includes("Rule A")); + assert.ok(content.includes(DIRECTIVE), "directive must reach the system message"); + assert.ok( + content.endsWith(DIRECTIVE), + "directive must be appended at the tail (highest recency)" + ); + }); +}); + +test("claude-to-openai creates a system message carrying the directive when body has none", () => { + withEnv(DIRECTIVE, () => { + const result = claudeToOpenAIRequest("deepseek-v4", baseBody(false), false); + const sys = result.messages[0] as { role: string; content: unknown }; + assert.equal(sys.role, "system"); + assert.equal(sys.content, DIRECTIVE); + }); +}); + +test("claude-to-openai appends the directive to array system content as a text block", () => { + withEnv(DIRECTIVE, () => { + const result = claudeToOpenAIRequest( + "deepseek-v4", + { + messages: [{ role: "user", content: [{ type: "text", text: "Hello" }] }], + system: [{ text: "Block A", cache_control: { type: "ephemeral" } }], + }, + false, + { _preserveCacheControl: true } + ); + const sys = result.messages[0] as { role: string; content: unknown }; + assert.equal(sys.role, "system"); + assert.ok(Array.isArray(sys.content), "array system content must stay an array"); + const blocks = sys.content as Array<{ type: string; text: string }>; + const lastBlock = blocks[blocks.length - 1]; + assert.equal(lastBlock.type, "text"); + assert.ok(lastBlock.text.includes(DIRECTIVE), "directive block must be appended last"); + }); +}); + +test("claude-to-openai does not inject when env is unset", () => { + withEnv(undefined, () => { + const result = claudeToOpenAIRequest("deepseek-v4", baseBody(true), false); + const sys = result.messages[0] as { role: string; content: unknown }; + assert.equal(sys.content, "Rule A"); + }); +}); diff --git a/tests/unit/system-prompt.test.ts b/tests/unit/system-prompt.test.ts index 796745767e..7fb95fcb53 100644 --- a/tests/unit/system-prompt.test.ts +++ b/tests/unit/system-prompt.test.ts @@ -1,8 +1,12 @@ import test from "node:test"; import assert from "node:assert/strict"; -const { injectSystemPrompt, setSystemPromptConfig, getSystemPromptConfig } = - await import("../../open-sse/services/systemPrompt.ts"); +const { + injectSystemPrompt, + injectSystemPromptPostTranslation, + setSystemPromptConfig, + getSystemPromptConfig, +} = await import("../../open-sse/services/systemPrompt.ts"); // ─── Config ───────────────────────────────────────────────────────────────── @@ -180,5 +184,112 @@ test("injectSystemPrompt: developer role treated as system", () => { assert.ok(result.messages[0].content.trimEnd().endsWith("SUF")); }); +// ─── Post-translation injection (codex/Responses path — #3) ──────────────── +// injectSystemPromptPostTranslation runs AFTER translation, on a body whose +// messages[] already contains the resolved system/developer messages. The key +// difference from injectSystemPrompt: with multiple system/developer messages +// (codex sends a developer role per input item), prefix goes on the FIRST and +// suffix on the LAST — so suffix retains the highest recency position. + +test("postTranslation: single system → prefix front, suffix back", () => { + setSystemPromptConfig({ enabled: true, prefixPrompt: "PRE", suffixPrompt: "SUF" }); + const body = { + messages: [ + { role: "system", content: "Original" }, + { role: "user", content: "hi" }, + ], + }; + const result = injectSystemPromptPostTranslation(body); + assert.ok(result.messages[0].content.startsWith("PRE")); + assert.ok(result.messages[0].content.includes("Original")); + assert.ok(result.messages[0].content.trimEnd().endsWith("SUF")); + assert.equal(result.messages.length, 2); +}); + +test("postTranslation: multiple system/developer → prefix on first, suffix on LAST", () => { + setSystemPromptConfig({ enabled: true, prefixPrompt: "PRE", suffixPrompt: "SUF" }); + // codex-like: several developer (system-equivalent) messages then a user + const body = { + messages: [ + { role: "developer", content: "dev-0" }, + { role: "developer", content: "dev-1" }, + { role: "developer", content: "dev-2" }, + { role: "user", content: "hi" }, + ], + }; + const result = injectSystemPromptPostTranslation(body); + assert.equal(result.messages.length, 4); + // prefix on FIRST system/developer (index 0) + assert.ok(result.messages[0].content.startsWith("PRE")); + assert.ok(result.messages[0].content.includes("dev-0")); + // suffix on LAST system/developer (index 2) — NOT index 0 + assert.ok(result.messages[2].content.includes("dev-2")); + assert.ok(result.messages[2].content.trimEnd().endsWith("SUF")); + // the first must NOT carry the suffix, the last must NOT carry the prefix + assert.ok(!result.messages[0].content.includes("SUF")); + assert.ok(!result.messages[2].content.includes("PRE")); + // middle untouched + assert.equal(result.messages[1].content, "dev-1"); +}); + +test("postTranslation: array content → unshift prefix on first, push suffix on last", () => { + setSystemPromptConfig({ enabled: true, prefixPrompt: "PRE", suffixPrompt: "SUF" }); + const body = { + messages: [ + { role: "developer", content: [{ type: "text", text: "dev-0" }] }, + { role: "developer", content: [{ type: "text", text: "dev-1" }] }, + { role: "user", content: "hi" }, + ], + }; + const result = injectSystemPromptPostTranslation(body); + // first developer: PRE at index 0 + assert.equal(result.messages[0].content[0].text, "PRE"); + assert.equal(result.messages[0].content[1].text, "dev-0"); + // last developer: SUF at the end + assert.equal(result.messages[1].content[0].text, "dev-1"); + assert.equal(result.messages[1].content[1].text, "SUF"); +}); + +test("postTranslation: no system → combined inserted at front", () => { + setSystemPromptConfig({ enabled: true, prefixPrompt: "PRE", suffixPrompt: "SUF" }); + const body = { messages: [{ role: "user", content: "hi" }] }; + const result = injectSystemPromptPostTranslation(body); + assert.equal(result.messages[0].role, "system"); + assert.ok(result.messages[0].content.includes("PRE")); + assert.ok(result.messages[0].content.includes("SUF")); + assert.equal(result.messages.length, 2); +}); + +test("postTranslation: disabled / empty → no change", () => { + setSystemPromptConfig({ enabled: false, prefixPrompt: "PRE", suffixPrompt: "SUF" }); + const body = { messages: [{ role: "user", content: "hi" }] }; + assert.deepEqual(injectSystemPromptPostTranslation(body), body); + setSystemPromptConfig({ enabled: true, prefixPrompt: "", suffixPrompt: "" }); + assert.deepEqual(injectSystemPromptPostTranslation(body), body); +}); + +test("postTranslation: codex regression — suffix lands on LAST developer after translation shape", () => { + // Simulate the post-translation body shape for a codex Responses request: + // openai-responses.ts converts `instructions`→system (index 0) and each + // input developer item→system. The After Prompt (suffix) MUST land on the + // LAST system/developer, not the first — otherwise it is buried by the + // later developer messages and loses recency priority. + setSystemPromptConfig({ enabled: true, prefixPrompt: "", suffixPrompt: "AFTER-PROMPT-MARKER" }); + const body = { + messages: [ + { role: "system", content: "instructions-from-catalog" }, + { role: "system", content: "dev-0-from-input" }, + { role: "system", content: "dev-1-from-input" }, + { role: "system", content: "dev-2-from-input" }, + { role: "user", content: "do the task" }, + ], + }; + const result = injectSystemPromptPostTranslation(body); + const lastSys = result.messages[3]; + assert.ok(lastSys.content.trimEnd().endsWith("AFTER-PROMPT-MARKER")); + // the first system must NOT carry the suffix + assert.ok(!result.messages[0].content.includes("AFTER-PROMPT-MARKER")); +}); + // Reset test.after(() => setSystemPromptConfig({ enabled: false, prefixPrompt: "", suffixPrompt: "" }));