From cf3102cc03a1af89dc41bb3962016deb8d6888f9 Mon Sep 17 00:00:00 2001 From: hkfires <10558748+hkfires@users.noreply.github.com> Date: Fri, 2 Oct 2026 09:02:52 +0800 Subject: [PATCH] refactor(executor): optimize translation logic and improve plugin invocation handling --- internal/runtime/executor/gemini_executor.go | 12 +-- .../gemini_interactions_translate_test.go | 32 ++++---- .../executor/helps/codex_multi_agent_v2.go | 20 +++-- .../helps/codex_multi_agent_v2_test.go | 55 +++++++++----- .../helps/configuration_update_plugin_test.go | 39 ++++++++++ .../helps/request_pair_compat_test.go | 35 +++++++-- .../helps/request_pair_plugin_host_test.go | 76 +++++++++++++++++++ 7 files changed, 208 insertions(+), 61 deletions(-) create mode 100644 internal/runtime/executor/helps/request_pair_plugin_host_test.go diff --git a/internal/runtime/executor/gemini_executor.go b/internal/runtime/executor/gemini_executor.go index d8bcb5df4..805c64cc6 100644 --- a/internal/runtime/executor/gemini_executor.go +++ b/internal/runtime/executor/gemini_executor.go @@ -885,14 +885,14 @@ func translateGeminiInteractionsRequestBody(ctx context.Context, cfg *config.Con } // translateGeminiInteractionsRequestPair translates the working payload and the -// payload-config baseline. Identical inputs are translated once when no plugin -// hooks are installed. The baseline is captured before model and thinking -// mutations, and the working buffer is a separate copy so those mutations cannot -// change it. Distinct inputs and plugin hooks keep the existing order: working -// payload first, then the payload-config source. +// payload-config baseline. Identical inputs are translated once, including plugin +// hooks. The baseline is captured before model and thinking mutations, and the +// working buffer is a separate copy so those mutations cannot change it. Distinct +// inputs keep the existing order: working payload first, then the payload-config +// source. func translateGeminiInteractionsRequestPair(ctx context.Context, cfg *config.Config, model string, payload []byte, opts cliproxyexecutor.Options, stream, isCompat bool) (original, working []byte) { source := geminiInteractionsPayloadConfigInput(opts, payload) - if geminiInteractionsSameByteSlice(payload, source) && !sdktranslator.HasPluginHooks() { + if geminiInteractionsSameByteSlice(payload, source) { original = translateGeminiInteractionsRequestBody(ctx, cfg, model, payload, opts, stream, isCompat) return original, bytes.Clone(original) } diff --git a/internal/runtime/executor/gemini_interactions_translate_test.go b/internal/runtime/executor/gemini_interactions_translate_test.go index 085c00b47..7e6e45cde 100644 --- a/internal/runtime/executor/gemini_interactions_translate_test.go +++ b/internal/runtime/executor/gemini_interactions_translate_test.go @@ -76,9 +76,6 @@ func TestTranslateGeminiInteractionsRequestPairTranslatesSameSliceOnce(t *testin if sdktranslator.HasRequestTransformer(from, to) { t.Fatalf("request transformer %s -> %s is already registered", from, to) } - if sdktranslator.HasPluginHooks() { - t.Fatal("plugin hooks are installed and disable translation reuse") - } const model = "gemini-interactions-count" var calls int @@ -156,7 +153,7 @@ func TestTranslateGeminiInteractionsRequestPairTranslatesDistinctInputs(t *testi } } -func TestTranslateGeminiInteractionsRequestPairPreservesHookOrder(t *testing.T) { +func TestTranslateGeminiInteractionsRequestPairInvokesPluginOncePerInput(t *testing.T) { hooks := &interactionsTranslateHooks{} sdktranslator.SetPluginHooks(hooks) t.Cleanup(func() { sdktranslator.SetPluginHooks(nil) }) @@ -166,22 +163,27 @@ func TestTranslateGeminiInteractionsRequestPairPreservesHookOrder(t *testing.T) const model = "gemini-3.1-flash-lite" payload := []byte(`{"model":"gemini-3.1-flash-lite","messages":[{"role":"user","content":"same"}]}`) opts := cliproxyexecutor.Options{SourceFormat: sdktranslator.FormatOpenAI, OriginalRequest: payload} - base, work := translateGeminiInteractionsRequestPair(ctx, cfg, model, payload, opts, false, true) - if hooks.calls != 2 { - t.Fatalf("plugin hook calls = %d, want 2", hooks.calls) + for _, stream := range []bool{false, true} { + for _, compat := range []bool{false, true} { + hooks.calls = 0 + base, work := translateGeminiInteractionsRequestPair(ctx, cfg, model, payload, opts, stream, compat) + if hooks.calls != 1 { + t.Fatalf("stream=%v compat=%v: plugin calls = %d, want 1", stream, compat, hooks.calls) + } + if got := gjson.GetBytes(work, "plugin_call").Int(); got != 1 { + t.Fatalf("working plugin_call = %d, want 1", got) + } + if !bytes.Equal(base, work) { + t.Fatal("same input produced different plugin results") + } + assertIndependentGeminiInteractionsBuffers(t, payload, base, work) + } } - if got := gjson.GetBytes(work, "plugin_call").Int(); got != 1 { - t.Fatalf("working plugin_call = %d, want 1", got) - } - if got := gjson.GetBytes(base, "plugin_call").Int(); got != 2 { - t.Fatalf("baseline plugin_call = %d, want 2", got) - } - assertIndependentGeminiInteractionsBuffers(t, payload, base, work) detached := bytes.Clone(payload) opts.OriginalRequest = detached before := hooks.calls - base, work = translateGeminiInteractionsRequestPair(ctx, cfg, model, payload, opts, true, false) + base, work := translateGeminiInteractionsRequestPair(ctx, cfg, model, payload, opts, true, false) if hooks.calls != before+2 { t.Fatalf("distinct plugin hook calls = %d, want %d", hooks.calls-before, 2) } diff --git a/internal/runtime/executor/helps/codex_multi_agent_v2.go b/internal/runtime/executor/helps/codex_multi_agent_v2.go index c0fab30b3..c4d15e11e 100644 --- a/internal/runtime/executor/helps/codex_multi_agent_v2.go +++ b/internal/runtime/executor/helps/codex_multi_agent_v2.go @@ -59,12 +59,10 @@ func TranslateRequestEnvelopeWithCodexMultiAgentV2(ctx context.Context, headers // TranslateRequestPairWithCodexMultiAgentV2 translates the untouched baseline // payload and the working payload that later stages mutate in place. Executors -// normally assign the original payload to the request before translating, so both -// translations would rescan the same bytes and produce the same result. Built-in -// request translation is deterministic, so that case is translated once and -// duplicated when no plugin hooks are installed. Hooks retain two invocations -// because they may have request-scoped output or side effects. This removes a -// full extra pass over payloads that can reach tens of megabytes. +// normally assign the original payload to the request before translating, so the +// same input is translated once, including plugin hooks, then copied into a +// separate working buffer. This avoids duplicate plugin side effects and a full +// extra pass over payloads that can reach tens of megabytes. func TranslateRequestPairWithCodexMultiAgentV2(ctx context.Context, headers http.Header, cfg *config.Config, from, to sdktranslator.Format, model string, originalPayload, requestPayload []byte, stream bool) (original, working []byte) { req := sdktranslator.RequestEnvelope{Format: from, Model: model, Stream: stream} return TranslateRequestEnvelopePairWithCodexMultiAgentV2(ctx, headers, cfg, from, to, req, originalPayload, requestPayload) @@ -76,7 +74,7 @@ func TranslateRequestEnvelopePairWithCodexMultiAgentV2(ctx context.Context, head originalReq := req originalReq.Body = originalPayload original = TranslateRequestEnvelopeWithCodexMultiAgentV2(ctx, headers, cfg, from, to, originalReq).Body - if sameByteSlice(originalPayload, requestPayload) && !sdktranslator.HasPluginHooks() { + if sameByteSlice(originalPayload, requestPayload) { // The caller mutates the working copy, so it must not share the baseline array. return original, append([]byte(nil), original...) } @@ -99,7 +97,7 @@ func sameByteSlice(a, b []byte) bool { } // TranslateRequestPairWithAPIKeyModelCompatibility avoids translating identical -// inputs twice while retaining separate buffers and stateful plugin invocations. +// inputs twice while retaining separate buffers and invoking plugins once per input. func TranslateRequestPairWithAPIKeyModelCompatibility(ctx context.Context, headers http.Header, cfg *config.Config, from, to sdktranslator.Format, model string, originalPayload, requestPayload []byte, stream, isCompat bool) (original, working []byte) { original, working, _ = TranslateRequestPairWithAPIKeyModelCompatibilityAndUpdateIntent(ctx, headers, cfg, from, to, model, originalPayload, requestPayload, stream, isCompat) return original, working @@ -108,9 +106,9 @@ func TranslateRequestPairWithAPIKeyModelCompatibility(ctx context.Context, heade // TranslateRequestPairWithAPIKeyModelCompatibilityAndUpdateIntent returns the // normalizer decision for the working payload, not for the baseline payload. func TranslateRequestPairWithAPIKeyModelCompatibilityAndUpdateIntent(ctx context.Context, headers http.Header, cfg *config.Config, from, to sdktranslator.Format, model string, originalPayload, requestPayload []byte, stream, isCompat bool) (original, working []byte, updatesChanged bool) { - original, _ = TranslateRequestWithAPIKeyModelCompatibilityAndUpdateIntent(ctx, headers, cfg, from, to, model, originalPayload, stream, isCompat) - if sameByteSlice(originalPayload, requestPayload) && !sdktranslator.HasPluginHooks() { - return original, append([]byte(nil), original...), false + original, updatesChanged = TranslateRequestWithAPIKeyModelCompatibilityAndUpdateIntent(ctx, headers, cfg, from, to, model, originalPayload, stream, isCompat) + if sameByteSlice(originalPayload, requestPayload) { + return original, append([]byte(nil), original...), updatesChanged } working, updatesChanged = TranslateRequestWithAPIKeyModelCompatibilityAndUpdateIntent(ctx, headers, cfg, from, to, model, requestPayload, stream, isCompat) return original, working, updatesChanged diff --git a/internal/runtime/executor/helps/codex_multi_agent_v2_test.go b/internal/runtime/executor/helps/codex_multi_agent_v2_test.go index cadba9796..fb32c1f99 100644 --- a/internal/runtime/executor/helps/codex_multi_agent_v2_test.go +++ b/internal/runtime/executor/helps/codex_multi_agent_v2_test.go @@ -132,32 +132,45 @@ func TestTranslateRequestPairTranslatesDistinctPayloads(t *testing.T) { } } -func TestTranslateRequestPairPreservesPluginHookInvocations(t *testing.T) { +func TestTranslateRequestPairInvokesPluginOncePerInput(t *testing.T) { hooks := &pairRequestPluginHooks{} sdktranslator.SetPluginHooks(hooks) t.Cleanup(func() { sdktranslator.SetPluginHooks(nil) }) payload := geminiToolHistoryPayload(1) - base, work := TranslateRequestPairWithCodexMultiAgentV2( - context.Background(), - http.Header{}, - &config.Config{}, - sdktranslator.FormatGemini, - sdktranslator.FromString("antigravity"), - "gemini-3.6-flash-high", - payload, - payload, - true, - ) - - if hooks.calls != 2 { - t.Fatalf("plugin hook calls = %d, want 2", hooks.calls) - } - if got := gjson.GetBytes(base, "plugin_call").Int(); got != 1 { - t.Fatalf("baseline plugin_call = %d, want 1", got) - } - if got := gjson.GetBytes(work, "plugin_call").Int(); got != 2 { - t.Fatalf("working plugin_call = %d, want 2", got) + for _, stream := range []bool{false, true} { + for _, distinct := range []bool{false, true} { + hooks.calls = 0 + request := payload + wantCalls := int64(1) + if distinct { + request = geminiToolHistoryPayload(2) + wantCalls = 2 + } + base, work := TranslateRequestPairWithCodexMultiAgentV2( + context.Background(), http.Header{}, &config.Config{}, + sdktranslator.FormatGemini, sdktranslator.FormatAntigravity, + "gemini-3.6-flash-high", payload, request, stream, + ) + if hooks.calls != wantCalls { + t.Fatalf("stream=%v distinct=%v: plugin calls = %d, want %d", stream, distinct, hooks.calls, wantCalls) + } + if got := gjson.GetBytes(base, "plugin_call").Int(); got != 1 { + t.Fatalf("baseline plugin_call = %d, want 1", got) + } + if got := gjson.GetBytes(work, "plugin_call").Int(); got != wantCalls { + t.Fatalf("working plugin_call = %d, want %d", got, wantCalls) + } + if !distinct && !bytes.Equal(base, work) { + t.Fatal("same input produced different plugin results") + } + baselineBefore := bytes.Clone(base) + inputBefore := bytes.Clone(request) + work[0] = 'X' + if !bytes.Equal(base, baselineBefore) || !bytes.Equal(request, inputBefore) { + t.Fatal("working buffer aliases the baseline or input") + } + } } } diff --git a/internal/runtime/executor/helps/configuration_update_plugin_test.go b/internal/runtime/executor/helps/configuration_update_plugin_test.go index 3cf8e2fb7..701c5b5e2 100644 --- a/internal/runtime/executor/helps/configuration_update_plugin_test.go +++ b/internal/runtime/executor/helps/configuration_update_plugin_test.go @@ -1,9 +1,11 @@ package helps_test import ( + "bytes" "context" "testing" + "github.com/router-for-me/CLIProxyAPI/v8/internal/config" "github.com/router-for-me/CLIProxyAPI/v8/internal/registry" helps "github.com/router-for-me/CLIProxyAPI/v8/internal/runtime/executor/helps" _ "github.com/router-for-me/CLIProxyAPI/v8/internal/thinking/provider/claude" @@ -18,9 +20,11 @@ import ( type editUpdatePluginHooks struct { summaryRemovingPluginHooks effort string + calls int } func (h *editUpdatePluginHooks) NormalizeRequest(_ context.Context, _, _ sdktranslator.Format, _ string, body []byte, _ bool) []byte { + h.calls++ if h.effort == "" { out, _ := sjson.DeleteBytes(body, "input.0") return out @@ -57,6 +61,41 @@ func TestPluginConfigurationUpdateNormalizationIsAuthoritative(t *testing.T) { } } +func TestRequestPairPreservesPluginConfigurationUpdateIntent(t *testing.T) { + for _, stream := range []bool{false, true} { + for _, compat := range []bool{false, true} { + for _, distinct := range []bool{false, true} { + hooks := &editUpdatePluginHooks{} + sdktranslator.SetPluginHooks(hooks) + t.Cleanup(func() { sdktranslator.SetPluginHooks(nil) }) + original := []byte(`{"reasoning":{"effort":"medium"},"input":[{"type":"configuration_update","reasoning":{"effort":"low"}},{"role":"user","content":"hi"}]}`) + request := original + wantCalls := 1 + if distinct { + request = []byte(`{"input":[{"role":"user","content":"different"}]}`) + wantCalls = 2 + } + inputBefore := bytes.Clone(request) + base, work, updatesChanged := helps.TranslateRequestPairWithAPIKeyModelCompatibilityAndUpdateIntent(t.Context(), nil, &config.Config{}, sdktranslator.FormatOpenAIResponse, sdktranslator.FormatCodex, "opaque-route", original, request, stream, compat) + if hooks.calls != wantCalls { + t.Fatalf("stream=%v compat=%v distinct=%v: plugin calls = %d, want %d", stream, compat, distinct, hooks.calls, wantCalls) + } + if updatesChanged != !distinct { + t.Fatalf("stream=%v compat=%v distinct=%v: working update intent = %v, want %v", stream, compat, distinct, updatesChanged, !distinct) + } + if !distinct && !bytes.Equal(base, work) { + t.Fatal("same input produced different plugin results") + } + baselineBefore := bytes.Clone(base) + work[0] = 'X' + if !bytes.Equal(base, baselineBefore) || !bytes.Equal(request, inputBefore) { + t.Fatal("working buffer aliases the baseline or input") + } + } + } + } +} + func TestNativeCrossProtocolTranslationWithoutPluginRetainsSourceUpdate(t *testing.T) { const source = `{"model":"private-chat","reasoning":{"effort":"medium"},"input":[{"type":"configuration_update","reasoning":{"effort":"high"}},{"role":"user","content":"hi"}]}` translated := sdktranslator.TranslateRequestEnvelope(t.Context(), sdktranslator.FormatOpenAIResponse, sdktranslator.FormatOpenAI, sdktranslator.RequestEnvelope{Model: "private-chat", Body: []byte(source)}) diff --git a/internal/runtime/executor/helps/request_pair_compat_test.go b/internal/runtime/executor/helps/request_pair_compat_test.go index 2f89d87c2..84a3e8cf5 100644 --- a/internal/runtime/executor/helps/request_pair_compat_test.go +++ b/internal/runtime/executor/helps/request_pair_compat_test.go @@ -45,9 +45,6 @@ func TestTranslateRequestPairWithAPIKeyModelCompatibilityCountsTranslations(t *t if sdktranslator.HasRequestTransformer(from, to) { t.Fatalf("request transformer %s -> %s is already registered", from, to) } - if sdktranslator.HasPluginHooks() { - t.Fatal("plugin hooks are installed and disable translation reuse") - } const model = "compat-count-model" var calls int @@ -103,13 +100,35 @@ func TestTranslateRequestPairWithAPIKeyModelCompatibilityCountsTranslations(t *t } } -func TestCompatibilityRequestPairPreservesHooks(t *testing.T) { +func TestCompatibilityRequestPairInvokesPluginOncePerInput(t *testing.T) { hooks := &pairRequestPluginHooks{} sdktranslator.SetPluginHooks(hooks) t.Cleanup(func() { sdktranslator.SetPluginHooks(nil) }) - request := []byte(`{"model":"test","input":"hello"}`) - base, work := TranslateRequestPairWithAPIKeyModelCompatibility(context.Background(), nil, &config.Config{}, sdktranslator.FormatOpenAIResponse, sdktranslator.FormatOpenAI, "test", request, request, true, true) - if hooks.calls != 2 || bytes.Equal(base, work) { - t.Fatalf("stateful plugin calls must remain independent, calls=%d", hooks.calls) + payload := []byte(`{"model":"test","input":"hello"}`) + for _, stream := range []bool{false, true} { + for _, compat := range []bool{false, true} { + for _, distinct := range []bool{false, true} { + hooks.calls = 0 + request := payload + wantCalls := int64(1) + if distinct { + request = []byte(`{"model":"test","input":"changed"}`) + wantCalls = 2 + } + base, work := TranslateRequestPairWithAPIKeyModelCompatibility(context.Background(), nil, &config.Config{}, sdktranslator.FormatOpenAIResponse, sdktranslator.FormatOpenAI, "test", payload, request, stream, compat) + if hooks.calls != wantCalls { + t.Fatalf("stream=%v compat=%v distinct=%v: plugin calls = %d, want %d", stream, compat, distinct, hooks.calls, wantCalls) + } + if bytes.Equal(base, work) == distinct { + t.Fatal("plugin results do not match input identity") + } + baselineBefore := bytes.Clone(base) + inputBefore := bytes.Clone(request) + work[0] = 'X' + if !bytes.Equal(base, baselineBefore) || !bytes.Equal(request, inputBefore) { + t.Fatal("working buffer aliases the baseline or input") + } + } + } } } diff --git a/internal/runtime/executor/helps/request_pair_plugin_host_test.go b/internal/runtime/executor/helps/request_pair_plugin_host_test.go new file mode 100644 index 000000000..c7abe81f1 --- /dev/null +++ b/internal/runtime/executor/helps/request_pair_plugin_host_test.go @@ -0,0 +1,76 @@ +package helps_test + +import ( + "bytes" + "fmt" + "strings" + "testing" + + "github.com/router-for-me/CLIProxyAPI/v8/internal/config" + "github.com/router-for-me/CLIProxyAPI/v8/internal/pluginhost" + "github.com/router-for-me/CLIProxyAPI/v8/internal/runtime/executor/helps" + sdktranslator "github.com/router-for-me/CLIProxyAPI/v8/sdk/translator" + log "github.com/sirupsen/logrus" + "github.com/tidwall/gjson" +) + +func TestRequestPairWithEmptyPluginHostSanitizesSignaturesOnce(t *testing.T) { + logger := log.StandardLogger() + oldOutput, oldLevel := logger.Out, logger.GetLevel() + var logs bytes.Buffer + logger.SetOutput(&logs) + logger.SetLevel(log.DebugLevel) + t.Cleanup(func() { + logger.SetOutput(oldOutput) + logger.SetLevel(oldLevel) + sdktranslator.SetPluginHooks(nil) + }) + + var items []string + for i := range 10 { + items = append(items, fmt.Sprintf(`{"type":"function_call","call_id":"call_%d","name":"read_file","arguments":"{}"}`, i)) + } + for i := range 10 { + items = append(items, fmt.Sprintf(`{"type":"function_call_output","call_id":"call_%d","output":"ok"}`, i)) + } + items = append(items, `{"role":"user","content":"continue"}`) + payload := []byte(`{"model":"gemini-3.6-flash-high","input":[` + strings.Join(items, ",") + `]}`) + inputBefore := bytes.Clone(payload) + + for _, host := range []sdktranslator.PluginHooks{nil, pluginhost.New()} { + sdktranslator.SetPluginHooks(host) + for _, stream := range []bool{false, true} { + for _, route := range []string{"envelope", "api-key", "api-key-compat"} { + logs.Reset() + var base, work []byte + if route == "envelope" { + req := sdktranslator.RequestEnvelope{Format: sdktranslator.FormatOpenAIResponse, Model: "gemini-3.6-flash-high", Stream: stream} + base, work = helps.TranslateRequestEnvelopePairWithCodexMultiAgentV2(t.Context(), nil, &config.Config{}, sdktranslator.FormatOpenAIResponse, sdktranslator.FormatAntigravity, req, payload, payload) + } else { + base, work = helps.TranslateRequestPairWithAPIKeyModelCompatibility(t.Context(), nil, &config.Config{}, sdktranslator.FormatOpenAIResponse, sdktranslator.FormatAntigravity, "gemini-3.6-flash-high", payload, payload, stream, route == "api-key-compat") + } + const message = "gemini request: sanitized 9 thoughtSignatures" + if count := strings.Count(logs.String(), message); count != 1 { + t.Fatalf("host=%T stream=%v route=%s: sanitation logs = %d, want 1; logs=%s", host, stream, route, count, logs.String()) + } + if !bytes.Equal(base, work) { + t.Fatal("same input produced different translations") + } + parts := gjson.GetBytes(work, "request.contents.0.parts").Array() + if len(parts) != 10 || parts[0].Get("thoughtSignature").String() != "skip_thought_signature_validator" { + t.Fatalf("unexpected parallel tool call layout: %s", work) + } + for _, part := range parts[1:] { + if part.Get("thoughtSignature").Exists() { + t.Fatal("sibling tool call still has a signature") + } + } + baselineBefore := bytes.Clone(base) + work[0] = 'X' + if !bytes.Equal(base, baselineBefore) || !bytes.Equal(payload, inputBefore) { + t.Fatal("working buffer aliases the baseline or input") + } + } + } + } +}