From 7bbfeaf8a7acf2cd5a834dcb0842539fe6aabc2b Mon Sep 17 00:00:00 2001 From: Luis Pater Date: Tue, 15 Sep 2026 03:42:08 +0800 Subject: [PATCH] feat(executor): promote reasoning content as summary and sanitize inputs - Introduce `promoteOpenAIResponsesReasoningTextToSummary` to move `reasoning_text` parts from content to summary when the summary is empty. - Clear `reasoning.content` to comply with Codex schema constraints (`maxItems: 0`). - Implement safeguards for handling cleartext reasoning, preserving valid encrypted content, and stripping invalid `encrypted_content`. - Add comprehensive tests to validate behavior for promoting reasoning texts, preserving existing summaries, and ensuring sanitization rules. Closes: #5825 --- .../executor/openai_responses_signature.go | 97 +++++++++++++++++-- .../openai_responses_signature_test.go | 82 ++++++++++++++++ 2 files changed, 169 insertions(+), 10 deletions(-) diff --git a/internal/runtime/executor/openai_responses_signature.go b/internal/runtime/executor/openai_responses_signature.go index 42842df91..cdacc31e7 100644 --- a/internal/runtime/executor/openai_responses_signature.go +++ b/internal/runtime/executor/openai_responses_signature.go @@ -12,6 +12,42 @@ import ( "github.com/tidwall/sjson" ) +func openaiResponsesReasoningSummaryIsEmpty(summary gjson.Result) bool { + if !summary.Exists() || summary.Type == gjson.Null { + return true + } + return summary.IsArray() && len(summary.Array()) == 0 +} + +func promoteOpenAIResponsesReasoningTextToSummary(itemRaw string, content gjson.Result) (string, error) { + var b strings.Builder + b.WriteByte('[') + n := 0 + for _, part := range content.Array() { + if strings.TrimSpace(part.Get("type").String()) != "reasoning_text" { + continue + } + text := part.Get("text").String() + if text == "" { + continue + } + partJSON, err := sjson.Set(`{"type":"summary_text"}`, "text", text) + if err != nil { + return itemRaw, err + } + if n > 0 { + b.WriteByte(',') + } + b.WriteString(partJSON) + n++ + } + b.WriteByte(']') + if n == 0 { + return itemRaw, nil + } + return sjson.SetRaw(itemRaw, "summary", b.String()) +} + func sanitizeOpenAIResponsesReasoningEncryptedContent(ctx context.Context, provider string, body []byte) []byte { inputResult := util.GetGJSONBytesNoCopy(body, "input") if !inputResult.Exists() || !inputResult.IsArray() { @@ -73,20 +109,49 @@ func sanitizeOpenAIResponsesReasoningEncryptedContent(ctx context.Context, provi itemID = fmt.Sprintf("input[%d]", index) } + nextItem := item.Raw + changed := false + + // Official Codex schema sets maxItems: 0 on reasoning.content. Third-party + // channels replay cleartext thinking there; promote it into summary when + // summary is empty, then force content to []. + content := item.Get("content") + if content.IsArray() && len(content.Array()) > 0 { + if openaiResponsesReasoningSummaryIsEmpty(item.Get("summary")) { + promoted, errPromote := promoteOpenAIResponsesReasoningTextToSummary(nextItem, content) + if errPromote != nil { + helps.LogWithRequestID(ctx).Debugf("%s: failed to promote reasoning_text into summary at input[%d]: %v", provider, index, errPromote) + } else { + nextItem = promoted + } + } + cleared, errClear := sjson.SetRaw(nextItem, "content", "[]") + if errClear != nil { + helps.LogWithRequestID(ctx).Debugf("%s: failed to clear reasoning content at input[%d]: %v", provider, index, errClear) + } else { + nextItem = cleared + changed = true + helps.LogWithRequestID(ctx).Debugf("%s: cleared reasoning content at input[%d] item_id=%q", provider, index, itemID) + } + } + if !encryptedContent.Exists() { if stripOrphanReasoningIDs && item.Get("id").Exists() { - nextItem, err := sjson.Delete(item.Raw, "id") + dropped, err := sjson.Delete(nextItem, "id") if err != nil { helps.LogWithRequestID(ctx).Debugf("%s: failed to drop orphan reasoning id at input[%d]: %v", provider, index, err) - keep(item.Raw) - continue + } else { + nextItem = dropped + changed = true + helps.LogWithRequestID(ctx).Debugf("%s: dropped orphan reasoning id at input[%d] item_id=%q reason=missing encrypted_content with store disabled", provider, index, itemID) } - startRebuild(index) - keep(nextItem) - helps.LogWithRequestID(ctx).Debugf("%s: dropped orphan reasoning id at input[%d] item_id=%q reason=missing encrypted_content with store disabled", provider, index, itemID) + } + if !changed { + keep(item.Raw) continue } - keep(item.Raw) + startRebuild(index) + keep(nextItem) continue } @@ -105,16 +170,28 @@ func sanitizeOpenAIResponsesReasoningEncryptedContent(ctx context.Context, provi reason = fmt.Sprintf("encrypted_content must be a string, got %s", encryptedContent.Type.String()) } if reason == "" { - keep(item.Raw) + if !changed { + keep(item.Raw) + continue + } + startRebuild(index) + keep(nextItem) continue } - nextItem, err := sjson.Delete(item.Raw, "encrypted_content") + dropped, err := sjson.Delete(nextItem, "encrypted_content") if err != nil { helps.LogWithRequestID(ctx).Debugf("%s: failed to drop invalid reasoning encrypted_content at input[%d]: %v", provider, index, err) - keep(item.Raw) + if !changed { + keep(item.Raw) + continue + } + startRebuild(index) + keep(nextItem) continue } + nextItem = dropped + changed = true if stripOrphanReasoningIDs && item.Get("id").Exists() { if nextID, errID := sjson.Delete(nextItem, "id"); errID != nil { helps.LogWithRequestID(ctx).Debugf("%s: failed to drop reasoning id after invalid encrypted_content at input[%d]: %v", provider, index, errID) diff --git a/internal/runtime/executor/openai_responses_signature_test.go b/internal/runtime/executor/openai_responses_signature_test.go index 8c6c8b8da..f39ca23d3 100644 --- a/internal/runtime/executor/openai_responses_signature_test.go +++ b/internal/runtime/executor/openai_responses_signature_test.go @@ -11,6 +11,14 @@ import ( var benchmarkSanitizeOpenAIResponsesReasoningOutput []byte +func assertEmptyReasoningContent(t *testing.T, body []byte, path string) { + t.Helper() + content := gjson.GetBytes(body, path) + if !content.Exists() || !content.IsArray() || len(content.Array()) != 0 { + t.Fatalf("%s should be an empty array for Codex maxItems:0, got %s body=%s", path, content.Raw, body) + } +} + func validOpenAIResponsesReasoningEncryptedContentForTest() string { payload := make([]byte, 1+8+16+16+32) payload[0] = 0x80 @@ -70,6 +78,80 @@ func TestSanitizeOpenAIResponsesReasoningEncryptedContent_KeepsIDsWhenStoreEnabl } } +func TestSanitizeOpenAIResponsesReasoningEncryptedContent_MovesCleartextContentToSummary(t *testing.T) { + // Codex Desktop replays third-party thinking as reasoning.content[reasoning_text]. + // Official Codex upstream rejects that shape with array_above_max_length (maxItems: 0). + body := []byte(`{"store":false,"input":[` + + `{"type":"reasoning","summary":[],"content":[{"type":"reasoning_text","text":"The model thinking process from a previous turn with a third-party provider..."}],"encrypted_content":null},` + + `{"id":"msg_1","type":"message","role":"user","content":[{"type":"input_text","text":"hi"}]}` + + `]}`) + + got := sanitizeOpenAIResponsesReasoningEncryptedContent(context.Background(), "test", body) + + assertEmptyReasoningContent(t, got, "input.0.content") + if gotType := gjson.GetBytes(got, "input.0.summary.0.type").String(); gotType != "summary_text" { + t.Fatalf("summary.0.type = %q, want summary_text; body=%s", gotType, got) + } + if gotText := gjson.GetBytes(got, "input.0.summary.0.text").String(); gotText != "The model thinking process from a previous turn with a third-party provider..." { + t.Fatalf("summary.0.text = %q, want promoted reasoning_text; body=%s", gotText, got) + } + if gjson.GetBytes(got, "input.0.encrypted_content").Exists() { + t.Fatalf("null encrypted_content should still be stripped: %s", got) + } + if gotType := gjson.GetBytes(got, "input.1.content.0.type").String(); gotType != "input_text" { + t.Fatalf("non-reasoning content should stay: %s", got) + } +} + +func TestSanitizeOpenAIResponsesReasoningEncryptedContent_DoesNotDuplicateExistingSummary(t *testing.T) { + body := []byte(`{"store":false,"input":[{"type":"reasoning","summary":[{"type":"summary_text","text":"already summarized"}],"content":[{"type":"reasoning_text","text":"duplicate thinking"}]}]}`) + + got := sanitizeOpenAIResponsesReasoningEncryptedContent(context.Background(), "test", body) + + assertEmptyReasoningContent(t, got, "input.0.content") + if gotLen := len(gjson.GetBytes(got, "input.0.summary").Array()); gotLen != 1 { + t.Fatalf("summary length = %d, want 1 (no duplicate); body=%s", gotLen, got) + } + if gotText := gjson.GetBytes(got, "input.0.summary.0.text").String(); gotText != "already summarized" { + t.Fatalf("existing summary overwritten: %s", got) + } +} + +func TestSanitizeOpenAIResponsesReasoningEncryptedContent_PromotesMultipleReasoningTextParts(t *testing.T) { + body := []byte(`{"store":false,"input":[{"type":"reasoning","summary":[],"content":[{"type":"reasoning_text","text":"step one"},{"type":"reasoning_text","text":"step two"}]}]}`) + + got := sanitizeOpenAIResponsesReasoningEncryptedContent(context.Background(), "test", body) + + assertEmptyReasoningContent(t, got, "input.0.content") + if gotLen := len(gjson.GetBytes(got, "input.0.summary").Array()); gotLen != 2 { + t.Fatalf("summary length = %d, want 2; body=%s", gotLen, got) + } + if gotText := gjson.GetBytes(got, "input.0.summary.0.text").String(); gotText != "step one" { + t.Fatalf("summary.0.text = %q, want step one; body=%s", gotText, got) + } + if gotText := gjson.GetBytes(got, "input.0.summary.1.text").String(); gotText != "step two" { + t.Fatalf("summary.1.text = %q, want step two; body=%s", gotText, got) + } +} + +func TestSanitizeOpenAIResponsesReasoningEncryptedContent_KeepsValidEncryptedContentWhenStrippingContent(t *testing.T) { + valid := validOpenAIResponsesReasoningEncryptedContentForTest() + body := []byte(`{"store":false,"input":[{"id":"rs_good","type":"reasoning","encrypted_content":"` + valid + `","summary":[],"content":[{"type":"reasoning_text","text":"cleartext thinking"}]}]}`) + + got := sanitizeOpenAIResponsesReasoningEncryptedContent(context.Background(), "test", body) + + assertEmptyReasoningContent(t, got, "input.0.content") + if gotID := gjson.GetBytes(got, "input.0.id").String(); gotID != "rs_good" { + t.Fatalf("valid reasoning id = %q, want rs_good; body=%s", gotID, got) + } + if gotEC := gjson.GetBytes(got, "input.0.encrypted_content").String(); gotEC != valid { + t.Fatalf("valid encrypted_content not preserved: %s", got) + } + if gotText := gjson.GetBytes(got, "input.0.summary.0.text").String(); gotText != "cleartext thinking" { + t.Fatalf("summary.0.text = %q, want promoted reasoning_text; body=%s", gotText, got) + } +} + func TestSanitizeOpenAIResponsesReasoningEncryptedContent_NoopReturnsOriginalBody(t *testing.T) { valid := validOpenAIResponsesReasoningEncryptedContentForTest() body := []byte(`{"store":false,"input":[{"id":"rs_good","type":"reasoning","encrypted_content":"` + valid + `","summary":[]},{"role":"user","content":"hi"}]}`)