Skip to content

Commit 55d0706

Browse files
hginghging
authored
[fix] review agent fix[fix] review agent fix (#255)
1 parent fd93783 commit 55d0706

File tree

4 files changed

+60
-44
lines changed

4 files changed

+60
-44
lines changed

‎internal/adapter/provider/cliproxyapi_codex/adapter.go‎

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,14 @@ import (
1414
"time"
1515

1616
"github.com/awsl-project/maxx/internal/adapter/provider"
17+
"github.com/awsl-project/maxx/internal/codexutil"
1718
"github.com/awsl-project/maxx/internal/domain"
1819
"github.com/awsl-project/maxx/internal/flow"
1920
"github.com/awsl-project/maxx/internal/usage"
2021
"github.com/router-for-me/CLIProxyAPI/v6/sdk/cliproxy/auth"
2122
"github.com/router-for-me/CLIProxyAPI/v6/sdk/cliproxy/executor"
2223
"github.com/router-for-me/CLIProxyAPI/v6/sdk/exec"
2324
"github.com/router-for-me/CLIProxyAPI/v6/sdk/translator"
24-
"github.com/tidwall/gjson"
25-
"github.com/tidwall/sjson"
2625
)
2726

2827
// TokenCache caches access tokens
@@ -254,26 +253,7 @@ func (a *CLIProxyAPICodexAdapter) Execute(c *flow.Ctx, p *domain.Provider) error
254253
}
255254

256255
func sanitizeCodexPayload(body []byte) []byte {
257-
if input := gjson.GetBytes(body, "input"); input.IsArray() {
258-
for i, item := range input.Array() {
259-
itemType := item.Get("type").String()
260-
if itemType != "message" {
261-
if item.Get("role").Exists() {
262-
body, _ = sjson.DeleteBytes(body, fmt.Sprintf("input.%d.role", i))
263-
}
264-
}
265-
if itemType == "function_call" {
266-
if id := item.Get("id").String(); id != "" && !strings.HasPrefix(id, "fc_") {
267-
body, _ = sjson.SetBytes(body, fmt.Sprintf("input.%d.id", i), "fc_"+id)
268-
}
269-
}
270-
if itemType == "function_call_output" {
271-
if !item.Get("output").Exists() {
272-
body, _ = sjson.SetBytes(body, fmt.Sprintf("input.%d.output", i), "")
273-
}
274-
}
275-
}
276-
}
256+
body = codexutil.NormalizeCodexInput(body)
277257
return body
278258
}
279259

‎internal/adapter/provider/codex/adapter.go‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616

1717
"github.com/awsl-project/maxx/internal/adapter/provider"
1818
cliproxyapi "github.com/awsl-project/maxx/internal/adapter/provider/cliproxyapi_codex"
19+
"github.com/awsl-project/maxx/internal/codexutil"
1920
"github.com/awsl-project/maxx/internal/domain"
2021
"github.com/awsl-project/maxx/internal/flow"
2122
"github.com/awsl-project/maxx/internal/usage"
@@ -572,26 +573,7 @@ func applyCodexRequestTuning(c *flow.Ctx, body []byte) (string, []byte) {
572573
if !gjson.GetBytes(body, "instructions").Exists() {
573574
body, _ = sjson.SetBytes(body, "instructions", "")
574575
}
575-
if input := gjson.GetBytes(body, "input"); input.IsArray() {
576-
for i, item := range input.Array() {
577-
itemType := item.Get("type").String()
578-
if itemType != "message" {
579-
if item.Get("role").Exists() {
580-
body, _ = sjson.DeleteBytes(body, fmt.Sprintf("input.%d.role", i))
581-
}
582-
}
583-
if itemType == "function_call" {
584-
if id := item.Get("id").String(); id != "" && !strings.HasPrefix(id, "fc_") {
585-
body, _ = sjson.SetBytes(body, fmt.Sprintf("input.%d.id", i), "fc_"+id)
586-
}
587-
}
588-
if itemType == "function_call_output" {
589-
if !item.Get("output").Exists() {
590-
body, _ = sjson.SetBytes(body, fmt.Sprintf("input.%d.output", i), "")
591-
}
592-
}
593-
}
594-
}
576+
body = codexutil.NormalizeCodexInput(body)
595577

596578
return cacheID, body
597579
}

‎internal/adapter/provider/codex/adapter_test.go‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package codex
22

33
import (
44
"net/http"
5+
"strings"
56
"testing"
67

78
"github.com/awsl-project/maxx/internal/domain"
@@ -14,7 +15,7 @@ func TestApplyCodexRequestTuning(t *testing.T) {
1415
c.Set(flow.KeyOriginalClientType, domain.ClientTypeClaude)
1516
c.Set(flow.KeyOriginalRequestBody, []byte(`{"metadata":{"user_id":"user-123"}}`))
1617

17-
body := []byte(`{"model":"gpt-5","stream":false,"instructions":"x","previous_response_id":"r1","prompt_cache_retention":123,"safety_identifier":"s1","max_output_tokens":77,"input":[{"type":"message","role":"user","content":"hi"},{"type":"function_call","role":"assistant","name":"t","arguments":"{}"},{"role":"tool","call_id":"c1","output":"ok"}]}`)
18+
body := []byte(`{"model":"gpt-5","stream":false,"instructions":"x","previous_response_id":"r1","prompt_cache_retention":123,"safety_identifier":"s1","max_output_tokens":77,"input":[{"type":"message","role":"user","content":"hi"},{"type":"function_call","role":"assistant","name":"t","arguments":"{}","id":"toolu_01"},{"type":"function_call","name":"t2","arguments":"{}"},{"type":"function_call_output","call_id":"c1"},{"role":"tool","call_id":"c1","output":"ok"}]}`)
1819
cacheID, tuned := applyCodexRequestTuning(c, body)
1920

2021
if cacheID == "" {
@@ -44,9 +45,19 @@ func TestApplyCodexRequestTuning(t *testing.T) {
4445
if gjson.GetBytes(tuned, "input.0.role").String() != "user" {
4546
t.Fatalf("expected role to be preserved for message input")
4647
}
47-
if gjson.GetBytes(tuned, "input.1.role").Exists() || gjson.GetBytes(tuned, "input.2.role").Exists() {
48+
if gjson.GetBytes(tuned, "input.1.role").Exists() || gjson.GetBytes(tuned, "input.2.role").Exists() || gjson.GetBytes(tuned, "input.3.role").Exists() || gjson.GetBytes(tuned, "input.4.role").Exists() {
4849
t.Fatalf("expected role to be removed for non-message inputs")
4950
}
51+
if gjson.GetBytes(tuned, "input.1.id").String() != "fc_toolu_01" {
52+
t.Fatalf("expected function_call id to be prefixed with fc_")
53+
}
54+
missingID := gjson.GetBytes(tuned, "input.2.id").String()
55+
if !strings.HasPrefix(missingID, "fc_") || missingID == "fc_" {
56+
t.Fatalf("expected generated function_call id to be set and prefixed with fc_")
57+
}
58+
if gjson.GetBytes(tuned, "input.3.output").String() != "" {
59+
t.Fatalf("expected missing function_call_output output to default to empty string")
60+
}
5061
}
5162

5263
func TestApplyCodexHeadersFiltersSensitiveAndPreservesUA(t *testing.T) {

‎internal/codexutil/normalize.go‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package codexutil
2+
3+
import (
4+
"fmt"
5+
"strings"
6+
7+
"github.com/google/uuid"
8+
"github.com/tidwall/gjson"
9+
"github.com/tidwall/sjson"
10+
)
11+
12+
func NormalizeCodexInput(body []byte) []byte {
13+
input := gjson.GetBytes(body, "input")
14+
if !input.IsArray() {
15+
return body
16+
}
17+
18+
for i, item := range input.Array() {
19+
itemType := item.Get("type").String()
20+
if itemType != "message" {
21+
if item.Get("role").Exists() {
22+
body, _ = sjson.DeleteBytes(body, fmt.Sprintf("input.%d.role", i))
23+
}
24+
}
25+
if itemType == "function_call" {
26+
id := strings.TrimSpace(item.Get("id").String())
27+
switch {
28+
case strings.HasPrefix(id, "fc_"):
29+
case id == "":
30+
body, _ = sjson.SetBytes(body, fmt.Sprintf("input.%d.id", i), "fc_"+uuid.NewString())
31+
default:
32+
body, _ = sjson.SetBytes(body, fmt.Sprintf("input.%d.id", i), "fc_"+id)
33+
}
34+
}
35+
if itemType == "function_call_output" {
36+
if !item.Get("output").Exists() {
37+
body, _ = sjson.SetBytes(body, fmt.Sprintf("input.%d.output", i), "")
38+
}
39+
}
40+
}
41+
42+
return body
43+
}

0 commit comments

Comments
 (0)