diff --git a/internal/runtime/executor/codex_executor_signature_test.go b/internal/runtime/executor/codex_executor_signature_test.go index 0702dd6c..4b69984a 100644 --- a/internal/runtime/executor/codex_executor_signature_test.go +++ b/internal/runtime/executor/codex_executor_signature_test.go @@ -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) } diff --git a/internal/runtime/executor/openai_responses_signature.go b/internal/runtime/executor/openai_responses_signature.go index 716574bf..8f5c847c 100644 --- a/internal/runtime/executor/openai_responses_signature.go +++ b/internal/runtime/executor/openai_responses_signature.go @@ -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) } diff --git a/internal/runtime/executor/openai_responses_signature_test.go b/internal/runtime/executor/openai_responses_signature_test.go new file mode 100644 index 00000000..9ba1eb28 --- /dev/null +++ b/internal/runtime/executor/openai_responses_signature_test.go @@ -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") + } +}