fix(executor): strip orphan reasoning ids when store is disabled (#4285)

* fix(executor): strip orphan reasoning ids when store is disabled

Codex backend requires store=false and treats reasoning items with an
id but no usable encrypted_content as store lookups, which 404 with
"Item with id not found". Drop those orphan ids after removing invalid
encrypted_content (and when encrypted_content is already missing),
while preserving ids when store=true.

* fix(executor): skip id delete when reasoning item has no id

Avoid sjson.Delete on encrypted_content drops that never carried an id,
matching the orphan-id path's Exists() guard.
This commit is contained in:
sususu98
2026-07-14 00:35:48 +08:00
committed by GitHub
parent 7f6d491e70
commit 194fbce4b4
3 changed files with 129 additions and 12 deletions
@@ -65,9 +65,15 @@ func TestCodexExecutorDropsInvalidReasoningEncryptedContentFromFinalRequest(t *t
if gjson.GetBytes(gotBody, "input.0.encrypted_content").Exists() {
t.Fatalf("invalid reasoning encrypted_content exists, want removed; body=%s", string(gotBody))
}
if gjson.GetBytes(gotBody, "input.0.id").Exists() {
t.Fatalf("invalid reasoning id should be stripped under store=false default; body=%s", string(gotBody))
}
if gjson.GetBytes(gotBody, "input.1.encrypted_content").Exists() {
t.Fatalf("non-string reasoning encrypted_content exists, want removed; body=%s", string(gotBody))
}
if gjson.GetBytes(gotBody, "input.1.id").Exists() {
t.Fatalf("non-string reasoning id should be stripped under store=false default; body=%s", string(gotBody))
}
if got := gjson.GetBytes(gotBody, "input.2.encrypted_content").String(); got != validEncryptedContent {
t.Fatalf("valid reasoning encrypted_content = %q, want preserved", got)
}
@@ -21,6 +21,13 @@ func sanitizeOpenAIResponsesReasoningEncryptedContent(ctx context.Context, provi
provider = "openai responses upstream"
}
// Codex backend rejects store=true and does not persist items when store=false.
// A reasoning item that still carries an id without usable encrypted_content is
// treated as a store lookup and returns:
// Item with id '...' not found. Items are not persisted when `store` is set to false.
// Strip those orphan ids unless the request explicitly opts into store=true.
stripOrphanReasoningIDs := !gjson.GetBytes(body, "store").Bool()
items := inputResult.Array()
// rebuilt accumulates the edited "input" array as JSON array bytes. It
@@ -40,6 +47,18 @@ func sanitizeOpenAIResponsesReasoningEncryptedContent(ctx context.Context, provi
rebuilt = append(rebuilt, raw...)
itemsWritten++
}
startRebuild := func(index int) {
if rebuilt != nil {
return
}
// First item that needs editing: start the buffer and backfill
// it with the raw JSON of every preceding item.
rebuilt = make([]byte, 0, len(inputResult.Raw))
rebuilt = append(rebuilt, '[')
for i := range index {
keep(items[i].Raw)
}
}
for index, item := range items {
if strings.TrimSpace(item.Get("type").String()) != "reasoning" {
@@ -48,7 +67,24 @@ func sanitizeOpenAIResponsesReasoningEncryptedContent(ctx context.Context, provi
}
encryptedContent := item.Get("encrypted_content")
itemID := strings.TrimSpace(item.Get("id").String())
if itemID == "" {
itemID = fmt.Sprintf("input[%d]", index)
}
if !encryptedContent.Exists() {
if stripOrphanReasoningIDs && item.Get("id").Exists() {
nextItem, err := sjson.Delete(item.Raw, "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
}
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)
continue
}
keep(item.Raw)
continue
}
@@ -78,22 +114,17 @@ func sanitizeOpenAIResponsesReasoningEncryptedContent(ctx context.Context, provi
keep(item.Raw)
continue
}
if rebuilt == nil {
// First item that needs editing: start the buffer and backfill
// it with the raw JSON of every preceding item.
rebuilt = make([]byte, 0, len(inputResult.Raw))
rebuilt = append(rebuilt, '[')
for i := range index {
keep(items[i].Raw)
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)
} else {
nextItem = nextID
}
}
startRebuild(index)
keep(nextItem)
itemID := strings.TrimSpace(item.Get("id").String())
if itemID == "" {
itemID = fmt.Sprintf("input[%d]", index)
}
helps.LogWithRequestID(ctx).Debugf("%s: dropped invalid reasoning encrypted_content at input[%d] item_id=%q reason=%s", provider, index, itemID, reason)
}
@@ -0,0 +1,80 @@
package executor
import (
"context"
"encoding/base64"
"testing"
"github.com/tidwall/gjson"
)
func validOpenAIResponsesReasoningEncryptedContentForTest() string {
payload := make([]byte, 1+8+16+16+32)
payload[0] = 0x80
for i := 9; i < len(payload); i++ {
payload[i] = byte(i)
}
return base64.RawURLEncoding.EncodeToString(payload)
}
func TestSanitizeOpenAIResponsesReasoningEncryptedContent_StripsOrphanIDsWhenStoreDisabled(t *testing.T) {
valid := validOpenAIResponsesReasoningEncryptedContentForTest()
body := []byte(`{"store":false,"input":[` +
`{"id":"rs_bad","type":"reasoning","encrypted_content":"bad","summary":[]},` +
`{"id":"rs_orphan","type":"reasoning","summary":[]},` +
`{"id":"rs_good","type":"reasoning","encrypted_content":"` + valid + `","summary":[]},` +
`{"id":"msg_1","type":"message","role":"user","content":"hi"}` +
`]}`)
got := sanitizeOpenAIResponsesReasoningEncryptedContent(context.Background(), "test", body)
if gjson.GetBytes(got, "input.0.encrypted_content").Exists() {
t.Fatalf("invalid encrypted_content still present: %s", got)
}
if gjson.GetBytes(got, "input.0.id").Exists() {
t.Fatalf("invalid reasoning id should be stripped when store=false: %s", got)
}
if gjson.GetBytes(got, "input.1.id").Exists() {
t.Fatalf("orphan reasoning id should be stripped when store=false: %s", got)
}
if gotID := gjson.GetBytes(got, "input.2.id").String(); gotID != "rs_good" {
t.Fatalf("valid reasoning id = %q, want rs_good; body=%s", gotID, got)
}
if gotEC := gjson.GetBytes(got, "input.2.encrypted_content").String(); gotEC != valid {
t.Fatalf("valid encrypted_content not preserved: %s", got)
}
if gotID := gjson.GetBytes(got, "input.3.id").String(); gotID != "msg_1" {
t.Fatalf("non-reasoning id should stay: %s", got)
}
}
func TestSanitizeOpenAIResponsesReasoningEncryptedContent_KeepsIDsWhenStoreEnabled(t *testing.T) {
body := []byte(`{"store":true,"input":[` +
`{"id":"rs_bad","type":"reasoning","encrypted_content":"bad","summary":[]},` +
`{"id":"rs_orphan","type":"reasoning","summary":[]}` +
`]}`)
got := sanitizeOpenAIResponsesReasoningEncryptedContent(context.Background(), "test", body)
if gjson.GetBytes(got, "input.0.encrypted_content").Exists() {
t.Fatalf("invalid encrypted_content still present: %s", got)
}
if gotID := gjson.GetBytes(got, "input.0.id").String(); gotID != "rs_bad" {
t.Fatalf("store=true should keep reasoning id after dropping invalid encrypted_content, got %q body=%s", gotID, got)
}
if gotID := gjson.GetBytes(got, "input.1.id").String(); gotID != "rs_orphan" {
t.Fatalf("store=true should keep orphan reasoning id, got %q body=%s", gotID, 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"}]}`)
got := sanitizeOpenAIResponsesReasoningEncryptedContent(context.Background(), "test", body)
if string(got) != string(body) {
t.Fatalf("noop path should return original body unchanged\ngot=%s\nwant=%s", got, body)
}
if len(got) > 0 && len(body) > 0 && &got[0] != &body[0] {
t.Fatalf("noop path should return the original body slice")
}
}