From c044d5c1f1f244548525a9ea825c7e4221453153 Mon Sep 17 00:00:00 2001 From: orchief <33409040+orchief@users.noreply.github.com> Date: Sun, 19 Jul 2026 19:48:25 +0800 Subject: [PATCH 1/2] fix(openai): avoid duplicate OAuth system prompts Root cause: Chat bridge system messages were mirrored to instructions while remaining in input as developer messages. Fix: omit only losslessly promoted system messages for non-JSON-object OAuth Chat requests while preserving compatibility paths. Closes #4603 --- .../service/openai_codex_transform.go | 81 +++++++++++++++---- .../service/openai_codex_transform_test.go | 68 ++++++++++++++-- .../openai_gateway_chat_completions.go | 4 +- .../openai_gateway_chat_completions_test.go | 73 +++++++++++++++++ 4 files changed, 202 insertions(+), 24 deletions(-) diff --git a/backend/internal/service/openai_codex_transform.go b/backend/internal/service/openai_codex_transform.go index 653545440..9d47c1c56 100644 --- a/backend/internal/service/openai_codex_transform.go +++ b/backend/internal/service/openai_codex_transform.go @@ -77,10 +77,11 @@ type codexTransformResult struct { } type codexOAuthTransformOptions struct { - IsCodexCLI bool - IsCompact bool - SkipDefaultInstructions bool - PreserveToolCallIDs bool + IsCodexCLI bool + IsCompact bool + SkipDefaultInstructions bool + PreserveToolCallIDs bool + OmitPromotedSystemMessagesFromInput bool } const codexImageGenerationFunctionToolName = "image_gen.imagegen" @@ -219,9 +220,11 @@ func applyCodexOAuthTransformWithOptions(reqBody map[string]any, opts codexOAuth } // ChatGPT internal Codex endpoint does not accept role:"system". - // Keep the guidance in input as developer for Responses JSON mode, and - // also mirror it into instructions because Codex OAuth requires it. - if extractSystemMessagesFromInput(reqBody) { + // Mirror its text into instructions because Codex OAuth requires it. Some + // callers must also keep the guidance in input as developer (notably + // Responses JSON object mode), while Chat Completions compatibility can + // omit text-only messages after promoting them losslessly. + if extractSystemMessagesFromInput(reqBody, opts.OmitPromotedSystemMessagesFromInput) { result.Modified = true } @@ -1097,31 +1100,46 @@ func extractTextFromContent(content any) string { } } -// extractSystemMessagesFromInput scans input for role=="system", maps those -// items to developer, and mirrors their text into reqBody["instructions"]. -// It preserves the input items so Responses JSON mode can still see JSON -// instructions in input messages. -func extractSystemMessagesFromInput(reqBody map[string]any) bool { +// extractSystemMessagesFromInput scans input for role=="system" and mirrors +// their text into reqBody["instructions"]. By default it maps those items to +// developer so Responses JSON mode can still see JSON instructions in input. +// When omitPromoted is true, text-only items are removed after their content is +// losslessly promoted; mixed or malformed content is retained as developer. +func extractSystemMessagesFromInput(reqBody map[string]any, omitPromoted bool) bool { input, ok := reqBody["input"].([]any) if !ok || len(input) == 0 { return false } var systemTexts []string + filteredInput := make([]any, 0, len(input)) modified := false for _, item := range input { m, ok := item.(map[string]any) - if !ok { + if !ok || m["role"] != "system" { + filteredInput = append(filteredInput, item) continue } - if role, _ := m["role"].(string); role != "system" { - continue + + if omitPromoted { + if losslessText, lossless := extractLosslessTextFromContent(m["content"]); lossless { + if losslessText != "" { + systemTexts = append(systemTexts, losslessText) + } + modified = true + continue + } } - m["role"] = "developer" - modified = true + if text := extractTextFromContent(m["content"]); text != "" { systemTexts = append(systemTexts, text) } + m["role"] = "developer" + filteredInput = append(filteredInput, item) + modified = true + } + if omitPromoted && len(filteredInput) != len(input) { + reqBody["input"] = filteredInput } if len(systemTexts) == 0 { @@ -1137,6 +1155,35 @@ func extractSystemMessagesFromInput(reqBody map[string]any) bool { return true } +// extractLosslessTextFromContent returns text only when the entire content can +// be represented by an instructions string without dropping non-text parts. +func extractLosslessTextFromContent(content any) (string, bool) { + switch v := content.(type) { + case string: + return v, true + case []any: + var b strings.Builder + for _, part := range v { + m, ok := part.(map[string]any) + if !ok { + return "", false + } + typeName, ok := m["type"].(string) + if !ok || (typeName != "text" && typeName != "input_text" && typeName != "output_text") { + return "", false + } + text, ok := m["text"].(string) + if !ok { + return "", false + } + b.WriteString(text) + } + return b.String(), true + default: + return "", false + } +} + func extractPromptLikeInstructionsFromInput(reqBody map[string]any) string { input, ok := reqBody["input"].([]any) if !ok || len(input) == 0 { diff --git a/backend/internal/service/openai_codex_transform_test.go b/backend/internal/service/openai_codex_transform_test.go index 40d0fd4b4..14e3c0734 100644 --- a/backend/internal/service/openai_codex_transform_test.go +++ b/backend/internal/service/openai_codex_transform_test.go @@ -1336,7 +1336,7 @@ func TestExtractSystemMessagesFromInput(t *testing.T) { map[string]any{"role": "user", "content": "hello"}, }, } - result := extractSystemMessagesFromInput(reqBody) + result := extractSystemMessagesFromInput(reqBody, false) require.False(t, result) input, ok := reqBody["input"].([]any) require.True(t, ok) @@ -1352,7 +1352,7 @@ func TestExtractSystemMessagesFromInput(t *testing.T) { map[string]any{"role": "user", "content": "hello"}, }, } - result := extractSystemMessagesFromInput(reqBody) + result := extractSystemMessagesFromInput(reqBody, false) require.True(t, result) input, ok := reqBody["input"].([]any) require.True(t, ok) @@ -1378,7 +1378,7 @@ func TestExtractSystemMessagesFromInput(t *testing.T) { }, }, } - result := extractSystemMessagesFromInput(reqBody) + result := extractSystemMessagesFromInput(reqBody, false) require.True(t, result) require.Equal(t, "Be helpful.", reqBody["instructions"]) input, ok := reqBody["input"].([]any) @@ -1401,7 +1401,7 @@ func TestExtractSystemMessagesFromInput(t *testing.T) { map[string]any{"role": "user", "content": "hi"}, }, } - result := extractSystemMessagesFromInput(reqBody) + result := extractSystemMessagesFromInput(reqBody, false) require.True(t, result) require.Equal(t, "First.\n\nSecond.", reqBody["instructions"]) input, ok := reqBody["input"].([]any) @@ -1427,7 +1427,7 @@ func TestExtractSystemMessagesFromInput(t *testing.T) { map[string]any{"role": "assistant", "content": "Hi there"}, }, } - result := extractSystemMessagesFromInput(reqBody) + result := extractSystemMessagesFromInput(reqBody, false) require.True(t, result) input, ok := reqBody["input"].([]any) require.True(t, ok) @@ -1452,7 +1452,7 @@ func TestExtractSystemMessagesFromInput(t *testing.T) { }, "instructions": "Existing instructions.", } - result := extractSystemMessagesFromInput(reqBody) + result := extractSystemMessagesFromInput(reqBody, false) require.True(t, result) require.Equal(t, "Extracted.\n\nExisting instructions.", reqBody["instructions"]) input, ok := reqBody["input"].([]any) @@ -1461,6 +1461,62 @@ func TestExtractSystemMessagesFromInput(t *testing.T) { require.True(t, ok) require.Equal(t, "developer", msg["role"]) }) + + t.Run("omit losslessly promoted text-only messages", func(t *testing.T) { + reqBody := map[string]any{ + "input": []any{ + map[string]any{"role": "system", "content": "First."}, + map[string]any{ + "role": "system", + "content": []any{ + map[string]any{"type": "text", "text": "Second "}, + map[string]any{"type": "input_text", "text": "and "}, + map[string]any{"type": "output_text", "text": "third."}, + }, + }, + map[string]any{"role": "user", "content": "hi"}, + }, + "instructions": "Existing.", + } + + result := extractSystemMessagesFromInput(reqBody, true) + + require.True(t, result) + require.Equal(t, "First.\n\nSecond and third.\n\nExisting.", reqBody["instructions"]) + input, ok := reqBody["input"].([]any) + require.True(t, ok) + require.Len(t, input, 1) + user, ok := input[0].(map[string]any) + require.True(t, ok) + require.Equal(t, "user", user["role"]) + }) + + t.Run("omit keeps mixed system content as developer", func(t *testing.T) { + reqBody := map[string]any{ + "input": []any{ + map[string]any{ + "role": "system", + "content": []any{ + map[string]any{"type": "input_text", "text": "Inspect this image."}, + map[string]any{"type": "input_image", "image_url": "https://example.com/image.png"}, + }, + }, + map[string]any{"role": "user", "content": "hi"}, + }, + } + + result := extractSystemMessagesFromInput(reqBody, true) + + require.True(t, result) + require.Equal(t, "Inspect this image.", reqBody["instructions"]) + input, ok := reqBody["input"].([]any) + require.True(t, ok) + require.Len(t, input, 2) + developer, ok := input[0].(map[string]any) + require.True(t, ok) + require.Equal(t, "developer", developer["role"]) + require.Len(t, developer["content"], 2) + }) } // TestApplyCodexOAuthTransform_StripsPromptCacheRetention is a regression diff --git a/backend/internal/service/openai_gateway_chat_completions.go b/backend/internal/service/openai_gateway_chat_completions.go index cccec4605..8a8349b5f 100644 --- a/backend/internal/service/openai_gateway_chat_completions.go +++ b/backend/internal/service/openai_gateway_chat_completions.go @@ -196,8 +196,10 @@ func (s *OpenAIGatewayService) ForwardAsChatCompletions( if err := json.Unmarshal(responsesBody, &reqBody); err != nil { return nil, fmt.Errorf("unmarshal for codex transform: %w", err) } + isJSONObjectFormat := strings.EqualFold(strings.TrimSpace(gjson.GetBytes(responsesBody, "text.format.type").String()), "json_object") codexResult := applyCodexOAuthTransformWithOptions(reqBody, codexOAuthTransformOptions{ - SkipDefaultInstructions: !isResponsesShape, + SkipDefaultInstructions: !isResponsesShape, + OmitPromotedSystemMessagesFromInput: !isResponsesShape && !isJSONObjectFormat, }) if !isResponsesShape { ensureCodexOAuthInstructionsField(reqBody) diff --git a/backend/internal/service/openai_gateway_chat_completions_test.go b/backend/internal/service/openai_gateway_chat_completions_test.go index b307aa4ed..ededa9f32 100644 --- a/backend/internal/service/openai_gateway_chat_completions_test.go +++ b/backend/internal/service/openai_gateway_chat_completions_test.go @@ -279,6 +279,79 @@ func TestForwardAsChatCompletions_OAuthDoesNotInjectDefaultInstructions(t *testi require.NotContains(t, string(upstream.lastBody), "Communicate with the user by streaming thinking") } +func forwardOAuthChatCompletionsForUpstreamBody(t *testing.T, body []byte) []byte { + t.Helper() + + gin.SetMode(gin.TestMode) + rec := httptest.NewRecorder() + c, _ := gin.CreateTestContext(rec) + c.Request = httptest.NewRequest(http.MethodPost, "/v1/chat/completions", bytes.NewReader(body)) + c.Request.Header.Set("Content-Type", "application/json") + + upstream := &httpUpstreamRecorder{resp: &http.Response{ + StatusCode: http.StatusBadRequest, + Header: http.Header{"Content-Type": []string{"application/json"}, "x-request-id": []string{"rid_chat_system_promotion"}}, + Body: io.NopCloser(strings.NewReader(`{"error":{"type":"invalid_request_error","message":"stop before response parsing"}}`)), + }} + svc := &OpenAIGatewayService{cfg: &config.Config{}, httpUpstream: upstream} + account := &Account{ + ID: 4, + Name: "openai-oauth", + Platform: PlatformOpenAI, + Type: AccountTypeOAuth, + Concurrency: 1, + Credentials: map[string]any{ + "access_token": "oauth-token", + "chatgpt_account_id": "chatgpt-acc", + }, + } + + result, err := svc.ForwardAsChatCompletions(context.Background(), c, account, body, "", "gpt-5.4") + require.Error(t, err) + require.Nil(t, result) + require.NotEmpty(t, upstream.lastBody) + return upstream.lastBody +} + +func TestForwardAsChatCompletions_OAuthPromotesSystemMessageWithoutDuplication(t *testing.T) { + const systemPrompt = "Unique system prefix for token accounting." + body := []byte(`{"model":"gpt-5.4","messages":[{"role":"system","content":"` + systemPrompt + `"},{"role":"user","content":"hello"}],"stream":false}`) + + upstreamBody := forwardOAuthChatCompletionsForUpstreamBody(t, body) + + require.Equal(t, systemPrompt, gjson.GetBytes(upstreamBody, "instructions").String()) + require.Equal(t, int64(1), gjson.GetBytes(upstreamBody, "input.#").Int()) + require.Equal(t, "user", gjson.GetBytes(upstreamBody, "input.0.role").String()) + require.Equal(t, 1, strings.Count(string(upstreamBody), systemPrompt)) +} + +func TestForwardAsChatCompletions_OAuthJsonObjectKeepsSystemMessageInInput(t *testing.T) { + const systemPrompt = "Return JSON only." + body := []byte(`{"model":"gpt-5.4","messages":[{"role":"system","content":"` + systemPrompt + `"},{"role":"user","content":"symbol data"}],"response_format":{"type":" JSON_OBJECT "},"stream":false}`) + + upstreamBody := forwardOAuthChatCompletionsForUpstreamBody(t, body) + + require.Equal(t, systemPrompt, gjson.GetBytes(upstreamBody, "instructions").String()) + require.Equal(t, int64(2), gjson.GetBytes(upstreamBody, "input.#").Int()) + require.Equal(t, "developer", gjson.GetBytes(upstreamBody, "input.0.role").String()) + require.Equal(t, systemPrompt, gjson.GetBytes(upstreamBody, "input.0.content").String()) + require.Equal(t, 2, strings.Count(string(upstreamBody), systemPrompt)) +} + +func TestForwardAsChatCompletions_OAuthKeepsMixedSystemContentInInput(t *testing.T) { + const systemPrompt = "Inspect this reference image." + body := []byte(`{"model":"gpt-5.4","messages":[{"role":"system","content":[{"type":"text","text":"` + systemPrompt + `"},{"type":"image_url","image_url":{"url":"https://example.com/reference.png"}}]},{"role":"user","content":"hello"}],"stream":false}`) + + upstreamBody := forwardOAuthChatCompletionsForUpstreamBody(t, body) + + require.Equal(t, systemPrompt, gjson.GetBytes(upstreamBody, "instructions").String()) + require.Equal(t, int64(2), gjson.GetBytes(upstreamBody, "input.#").Int()) + require.Equal(t, "developer", gjson.GetBytes(upstreamBody, "input.0.role").String()) + require.Equal(t, int64(2), gjson.GetBytes(upstreamBody, "input.0.content.#").Int()) + require.Equal(t, "input_image", gjson.GetBytes(upstreamBody, "input.0.content.1.type").String()) + require.Equal(t, "https://example.com/reference.png", gjson.GetBytes(upstreamBody, "input.0.content.1.image_url").String()) +} + func TestForwardAsChatCompletions_ClientDisconnectDrainsUpstreamUsage(t *testing.T) { gin.SetMode(gin.TestMode) From 97afb4089c67f5ccbfd594c481c1ef364b0b4f9a Mon Sep 17 00:00:00 2001 From: shaw Date: Mon, 20 Jul 2026 10:46:37 +0800 Subject: [PATCH 2/2] fix(lint): check strings.Builder WriteString return value (errcheck) --- backend/internal/service/openai_codex_transform.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/internal/service/openai_codex_transform.go b/backend/internal/service/openai_codex_transform.go index 9d47c1c56..c2169ab65 100644 --- a/backend/internal/service/openai_codex_transform.go +++ b/backend/internal/service/openai_codex_transform.go @@ -1176,7 +1176,7 @@ func extractLosslessTextFromContent(content any) (string, bool) { if !ok { return "", false } - b.WriteString(text) + _, _ = b.WriteString(text) } return b.String(), true default: