mirror of
https://github.com/Wei-Shaw/sub2api.git
synced 2026-09-24 16:05:44 +08:00
fix(openai): preserve encrypted reasoning across turns on codex OAuth path
PR #2068 dropped every reasoning item from input[] on the OAuth/codex path (store=false). That silently discards encrypted_content -- the out-of-band channel that carries reasoning context across turns under store=false -- degrading multi-turn agent reasoning with no visible error. Reported by @neteroster on PR #2068. The 404 that #2068 worked around ("Item with id 'rs_...' not found") is triggered by the rs_* id lookup under store=false, not by the reasoning item itself -- so the correct fix is to strip the id, not delete the item. Verified end-to-end against the live chatgpt.com codex backend (gpt-5.5) and a real OpenClaw container (api: openai-responses): - bare rs_ id, no encrypted_content -> 404 - id stripped -> 200 - encrypted_content + id stripped -> 200, reasoning context preserved - reasoning items require a summary field (missing -> 400) - real OpenClaw multi-turn agent loop -> all /v1/responses 200, zero 404 Fix: keep the reasoning item, strip only the rs_* id (always, independent of PreserveReferences), preserve encrypted_content/content/summary verbatim, and backfill an empty summary when absent. Tool-call call_id pairing is untouched. Also verified compaction_summary items (cmp_*, the other encrypted_content carrier): they require encrypted_content (missing -> 400) and their id does not 404 when present (kept or stripped), so the existing generic path already handles them safely -- no special-casing needed. Adds regression tests for each verified reasoning contract. Refs #1957, #2068
This commit is contained in:
@@ -1157,11 +1157,41 @@ func filterCodexInputWithOptions(input []any, opts codexInputFilterOptions) []an
|
||||
}
|
||||
typ, _ := m["type"].(string)
|
||||
|
||||
// chatgpt.com codex backend (OAuth path) does not persist reasoning
|
||||
// items because applyCodexOAuthTransform forces store=false. Any rs_*
|
||||
// reference replayed in input is guaranteed to 404 upstream
|
||||
// ("Item with id 'rs_...' not found"). Drop reasoning items entirely.
|
||||
// chatgpt.com codex (OAuth path) runs with store=false (forced by
|
||||
// applyCodexOAuthTransform). Replaying a reasoning item with its rs_*
|
||||
// id but no encrypted_content 404s upstream ("Item with id 'rs_...'
|
||||
// not found") — the 404 is triggered by the id lookup, not by the
|
||||
// reasoning item itself. So strip the id (always, independent of
|
||||
// PreserveReferences) yet keep the item: under store=false
|
||||
// encrypted_content is the official channel for carrying reasoning
|
||||
// context across turns, and dropping the whole item silently degrades
|
||||
// multi-turn agent reasoning. Preserve encrypted_content/content/
|
||||
// summary and every other field verbatim. Upstream additionally
|
||||
// requires a summary field — a missing one is rejected with 400
|
||||
// "Missing required parameter 'input[N].summary'" — so backfill an
|
||||
// empty array when it is absent. Contracts verified end-to-end against
|
||||
// chatgpt.com codex (gpt-5.5); see issue #1957.
|
||||
// compaction_summary items (cmp_*) are the other encrypted_content
|
||||
// carrier. Verified against the live backend: they require
|
||||
// encrypted_content (a missing one is rejected with 400), and with it
|
||||
// present the cmp_* id does not 404 whether kept or stripped. Being
|
||||
// neither reasoning nor tool calls, they flow through the generic path
|
||||
// below (id stripped when !PreserveReferences, encrypted_content
|
||||
// preserved either way), which is safe and needs no special-casing.
|
||||
if typ == "reasoning" {
|
||||
newItem := make(map[string]any, len(m))
|
||||
for key, value := range m {
|
||||
if key == "id" {
|
||||
// rs_* id replayed under store=false 404s; strip it.
|
||||
continue
|
||||
}
|
||||
newItem[key] = value
|
||||
}
|
||||
if summary, ok := newItem["summary"]; !ok || summary == nil {
|
||||
// Upstream requires a summary field; an empty array satisfies it.
|
||||
newItem["summary"] = []any{}
|
||||
}
|
||||
filtered = append(filtered, newItem)
|
||||
continue
|
||||
}
|
||||
|
||||
|
||||
@@ -1343,23 +1343,24 @@ func TestIsInstructionsEmpty(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestFilterCodexInput_DropsReasoningItemsRegardlessOfPreserveReferences(t *testing.T) {
|
||||
// Reasoning items in input[] reference rs_* IDs that were emitted by
|
||||
// chatgpt.com under store=false (forced by applyCodexOAuthTransform).
|
||||
// They are never persisted upstream, so forwarding them produces a
|
||||
// guaranteed 404 ("Item with id 'rs_...' not found"). Drop them
|
||||
// regardless of preserveReferences. See: Wei-Shaw/sub2api issue #1957.
|
||||
|
||||
// TestFilterCodexInput_PreservesReasoningStripsID covers the core OAuth-path
|
||||
// reasoning contract (replaces the earlier "drops reasoning" test, whose
|
||||
// premise was wrong). A reasoning item carrying encrypted_content is the
|
||||
// official channel for replaying reasoning context across turns under
|
||||
// store=false, so it must survive the filter with encrypted_content intact;
|
||||
// only its rs_* id is stripped (always, independent of PreserveReferences)
|
||||
// because a bare rs_* id replayed under store=false 404s upstream. Contracts
|
||||
// 1/2/3, verified end-to-end against chatgpt.com codex (gpt-5.5). See issue
|
||||
// #1957.
|
||||
func TestFilterCodexInput_PreservesReasoningStripsID(t *testing.T) {
|
||||
build := func() []any {
|
||||
return []any{
|
||||
map[string]any{"type": "message", "id": "msg_0", "role": "user", "content": "hi"},
|
||||
map[string]any{
|
||||
"type": "reasoning",
|
||||
"id": "rs_0672f12450da0b9c0169f07220a6c08198b68c2455ced99344",
|
||||
"summary": []any{},
|
||||
"type": "reasoning",
|
||||
"id": "rs_0672f12450da0b9c0169f07220a6c08198b68c2455ced99344",
|
||||
"encrypted_content": "gAAAAAB-enc-payload",
|
||||
"summary": []any{},
|
||||
},
|
||||
map[string]any{"type": "function_call", "id": "fc_1", "call_id": "call_1", "name": "tool"},
|
||||
map[string]any{"type": "function_call_output", "call_id": "call_1", "output": "{}"},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1367,31 +1368,175 @@ func TestFilterCodexInput_DropsReasoningItemsRegardlessOfPreserveReferences(t *t
|
||||
preserve := preserve
|
||||
t.Run(fmt.Sprintf("preserveReferences=%v", preserve), func(t *testing.T) {
|
||||
filtered := filterCodexInput(build(), preserve)
|
||||
require.Len(t, filtered, 1)
|
||||
|
||||
item, ok := filtered[0].(map[string]any)
|
||||
require.True(t, ok)
|
||||
// Contract 2: the reasoning item survives the filter.
|
||||
require.Equal(t, "reasoning", item["type"])
|
||||
// Contract 2: encrypted_content (cross-turn channel) preserved verbatim.
|
||||
require.Equal(t, "gAAAAAB-enc-payload", item["encrypted_content"])
|
||||
// Contract 1/3: rs_* id stripped unconditionally, even when
|
||||
// PreserveReferences=true (id lookup, not the item, triggers the 404).
|
||||
_, hasID := item["id"]
|
||||
require.False(t, hasID)
|
||||
// summary passed through untouched.
|
||||
summary, ok := item["summary"].([]any)
|
||||
require.True(t, ok)
|
||||
require.Len(t, summary, 0)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestFilterCodexInput_BareReasoningStripsIDBackfillsSummary covers contract 1
|
||||
// plus 5: a reasoning item carrying only an rs_* id (no encrypted_content) is
|
||||
// kept as an empty shell with the id stripped, and a missing summary is
|
||||
// backfilled to [] so upstream does not reject it with 400 "Missing required
|
||||
// parameter 'input[N].summary'". Verified against chatgpt.com codex (gpt-5.5).
|
||||
func TestFilterCodexInput_BareReasoningStripsIDBackfillsSummary(t *testing.T) {
|
||||
input := []any{
|
||||
map[string]any{
|
||||
"type": "reasoning",
|
||||
"id": "rs_0672f12450da0b9c0169f07220a6c08198b68c2455ced99344",
|
||||
},
|
||||
}
|
||||
|
||||
filtered := filterCodexInput(input, false)
|
||||
require.Len(t, filtered, 1)
|
||||
|
||||
item, ok := filtered[0].(map[string]any)
|
||||
require.True(t, ok)
|
||||
require.Equal(t, "reasoning", item["type"])
|
||||
// Contract 1: id stripped.
|
||||
_, hasID := item["id"]
|
||||
require.False(t, hasID)
|
||||
// Contract 5: summary backfilled to an empty array.
|
||||
summary, ok := item["summary"].([]any)
|
||||
require.True(t, ok)
|
||||
require.Len(t, summary, 0)
|
||||
}
|
||||
|
||||
// TestFilterCodexInput_ReasoningBackfillsMissingSummary isolates contract 5:
|
||||
// even when a reasoning item carries other content (here encrypted_content),
|
||||
// a missing summary field is always added as [] before forwarding upstream.
|
||||
func TestFilterCodexInput_ReasoningBackfillsMissingSummary(t *testing.T) {
|
||||
input := []any{
|
||||
map[string]any{
|
||||
"type": "reasoning",
|
||||
"id": "rs_abc",
|
||||
"encrypted_content": "gAAAAAB-enc",
|
||||
},
|
||||
}
|
||||
|
||||
filtered := filterCodexInput(input, false)
|
||||
require.Len(t, filtered, 1)
|
||||
|
||||
item, ok := filtered[0].(map[string]any)
|
||||
require.True(t, ok)
|
||||
summary, ok := item["summary"].([]any)
|
||||
require.True(t, ok)
|
||||
require.Len(t, summary, 0)
|
||||
// encrypted_content still preserved alongside the backfilled summary.
|
||||
require.Equal(t, "gAAAAAB-enc", item["encrypted_content"])
|
||||
}
|
||||
|
||||
// TestFilterCodexInput_PreservesReasoningSummaryAndContent verifies that a
|
||||
// non-empty summary is not overwritten and that arbitrary reasoning fields
|
||||
// (e.g. content) survive verbatim — only the id is removed.
|
||||
func TestFilterCodexInput_PreservesReasoningSummaryAndContent(t *testing.T) {
|
||||
summary := []any{
|
||||
map[string]any{"type": "summary_text", "text": "Considered the options."},
|
||||
}
|
||||
content := []any{
|
||||
map[string]any{"type": "reasoning_text", "text": "internal chain"},
|
||||
}
|
||||
input := []any{
|
||||
map[string]any{
|
||||
"type": "reasoning",
|
||||
"id": "rs_abc",
|
||||
"summary": summary,
|
||||
"content": content,
|
||||
"encrypted_content": "gAAAAAB-enc",
|
||||
},
|
||||
}
|
||||
|
||||
filtered := filterCodexInput(input, false)
|
||||
require.Len(t, filtered, 1)
|
||||
|
||||
item, ok := filtered[0].(map[string]any)
|
||||
require.True(t, ok)
|
||||
// Non-empty summary preserved verbatim (not replaced with []).
|
||||
require.Equal(t, summary, item["summary"])
|
||||
// content preserved verbatim.
|
||||
require.Equal(t, content, item["content"])
|
||||
require.Equal(t, "gAAAAAB-enc", item["encrypted_content"])
|
||||
_, hasID := item["id"]
|
||||
require.False(t, hasID)
|
||||
}
|
||||
|
||||
// TestFilterCodexInput_PreservesReasoningInMixedInput exercises contract 7:
|
||||
// reasoning items are stripped of their rs_* ids but kept (with
|
||||
// encrypted_content) while message / function_call / function_call_output
|
||||
// items flow through unchanged, with tool-call pairing (call_id) intact.
|
||||
func TestFilterCodexInput_PreservesReasoningInMixedInput(t *testing.T) {
|
||||
build := func() []any {
|
||||
return []any{
|
||||
map[string]any{"type": "message", "id": "msg_0", "role": "user", "content": "hi"},
|
||||
map[string]any{
|
||||
"type": "reasoning",
|
||||
"id": "rs_1",
|
||||
"encrypted_content": "gAAAAAB-enc-1",
|
||||
"summary": []any{},
|
||||
},
|
||||
map[string]any{
|
||||
"type": "reasoning",
|
||||
"id": "rs_2",
|
||||
"summary": []any{},
|
||||
},
|
||||
// call_id already in fc_ form so the unrelated call_->fc_
|
||||
// normalization does not obscure the pairing assertion.
|
||||
map[string]any{"type": "function_call", "id": "fc_1", "call_id": "fc_1", "name": "tool", "arguments": "{}"},
|
||||
map[string]any{"type": "function_call_output", "call_id": "fc_1", "output": "{}"},
|
||||
}
|
||||
}
|
||||
|
||||
for _, preserve := range []bool{true, false} {
|
||||
preserve := preserve
|
||||
t.Run(fmt.Sprintf("preserveReferences=%v", preserve), func(t *testing.T) {
|
||||
filtered := filterCodexInput(build(), preserve)
|
||||
// Nothing is dropped: both reasoning items are now preserved.
|
||||
require.Len(t, filtered, 5)
|
||||
|
||||
byType := make(map[string][]map[string]any)
|
||||
for _, raw := range filtered {
|
||||
item, ok := raw.(map[string]any)
|
||||
require.True(t, ok)
|
||||
require.NotEqual(t, "reasoning", item["type"],
|
||||
"reasoning items must be dropped from input on the OAuth path")
|
||||
typ, _ := item["type"].(string)
|
||||
byType[typ] = append(byType[typ], item)
|
||||
// No surviving item may carry an rs_* id.
|
||||
if id, ok := item["id"].(string); ok {
|
||||
require.False(t, strings.HasPrefix(id, "rs_"),
|
||||
"no item carrying an rs_* id should survive the filter")
|
||||
}
|
||||
}
|
||||
|
||||
// Sanity check: the non-reasoning items should still be present.
|
||||
gotTypes := make(map[string]int)
|
||||
for _, raw := range filtered {
|
||||
item, ok := raw.(map[string]any)
|
||||
require.True(t, ok)
|
||||
typ, ok := item["type"].(string)
|
||||
require.True(t, ok)
|
||||
gotTypes[typ]++
|
||||
// Both reasoning items kept, ids stripped, summary present.
|
||||
require.Len(t, byType["reasoning"], 2)
|
||||
for _, r := range byType["reasoning"] {
|
||||
_, hasID := r["id"]
|
||||
require.False(t, hasID)
|
||||
_, hasSummary := r["summary"]
|
||||
require.True(t, hasSummary)
|
||||
}
|
||||
require.Equal(t, 1, gotTypes["message"])
|
||||
require.Equal(t, 1, gotTypes["function_call"])
|
||||
require.Equal(t, 1, gotTypes["function_call_output"])
|
||||
require.Equal(t, 0, gotTypes["reasoning"])
|
||||
require.Equal(t, "gAAAAAB-enc-1", byType["reasoning"][0]["encrypted_content"])
|
||||
|
||||
// message / function_call(+output) untouched by reasoning handling.
|
||||
require.Len(t, byType["message"], 1)
|
||||
// Contract 7: tool-call pairing by call_id is unaffected.
|
||||
require.Len(t, byType["function_call"], 1)
|
||||
require.Equal(t, "fc_1", byType["function_call"][0]["call_id"])
|
||||
require.Len(t, byType["function_call_output"], 1)
|
||||
require.Equal(t, "fc_1", byType["function_call_output"][0]["call_id"])
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user