From a2976eb8a303f11b4ea5177bce9f9ff752634dfc Mon Sep 17 00:00:00 2001 From: Luis Pater Date: Tue, 6 Oct 2026 05:45:17 +0800 Subject: [PATCH] fix(claude): share OAuth tool alias store across scoped executor copies - Pre-allocate `oauthToolAliases` in Claude and Kimi executor constructors so request copies created via `ForAPIKey` reuse the same store - Replace struct-level mutex with package-level mutex for lazy store allocation to prevent copying mutexes by value Closes: #6401 --- internal/runtime/executor/claude_executor.go | 10 ++-- .../claude_executor_tool_alias_scope_test.go | 51 +++++++++++++++++++ .../executor/claude_executor_tool_state.go | 9 +++- internal/runtime/executor/kimi_executor.go | 1 + 4 files changed, 65 insertions(+), 6 deletions(-) create mode 100644 internal/runtime/executor/claude_executor_tool_alias_scope_test.go diff --git a/internal/runtime/executor/claude_executor.go b/internal/runtime/executor/claude_executor.go index 5c6562e77..7cb1f5b9f 100644 --- a/internal/runtime/executor/claude_executor.go +++ b/internal/runtime/executor/claude_executor.go @@ -7,7 +7,6 @@ import ( "fmt" "net/http" "strings" - "sync" "github.com/router-for-me/CLIProxyAPI/v8/internal/config" "github.com/router-for-me/CLIProxyAPI/v8/internal/runtime/executor/helps" @@ -27,8 +26,9 @@ type ClaudeExecutor struct { requestLogProvider string upstreamModelNormalizer func(string) string oauthProfileFetcher claudeOAuthProfileFetcher - oauthToolAliasStoreMu sync.Mutex - oauthToolAliases *claudeOAuthToolAliasStore + // oauthToolAliases is shared by every ForAPIKey copy of this executor; + // constructors allocate it so copies made per request reuse one store. + oauthToolAliases *claudeOAuthToolAliasStore } type claudeOAuthCancellationError struct { @@ -140,7 +140,9 @@ func logClaudeSignatureSanitizeReport(ctx context.Context, baseModel string, rep // omit max_tokens. Prefer registered model metadata before using a fallback. const defaultModelMaxTokens = 1024 -func NewClaudeExecutor(cfg *config.Config) *ClaudeExecutor { return &ClaudeExecutor{cfg: cfg} } +func NewClaudeExecutor(cfg *config.Config) *ClaudeExecutor { + return &ClaudeExecutor{cfg: cfg, oauthToolAliases: &claudeOAuthToolAliasStore{}} +} func (e *ClaudeExecutor) Identifier() string { return "claude" } diff --git a/internal/runtime/executor/claude_executor_tool_alias_scope_test.go b/internal/runtime/executor/claude_executor_tool_alias_scope_test.go new file mode 100644 index 000000000..e157ba751 --- /dev/null +++ b/internal/runtime/executor/claude_executor_tool_alias_scope_test.go @@ -0,0 +1,51 @@ +package executor + +import ( + "testing" + + "github.com/router-for-me/CLIProxyAPI/v8/internal/config" + cliproxyauth "github.com/router-for-me/CLIProxyAPI/v8/sdk/cliproxy/auth" +) + +// API key credentials are scoped per request with ForAPIKey, which copies the +// executor by value (see executorForAuth in sdk/cliproxy/auth). Thread +// continuation aliases saved by one request must be visible to the next one, +// so every scoped copy has to share one alias store. +func TestClaudeOAuthToolAliasStoreIsSharedAcrossForAPIKeyCopies(t *testing.T) { + createBody := []byte(`{"thread":{"type":"create"}}`) + continuationBody := []byte(`{"thread":{"type":"continue","previous_message_id":"msg-shared"}}`) + cases := map[string]cliproxyauth.APIKeyConfigExecutor{ + "claude": NewClaudeExecutor(&config.Config{}), + "kimi": NewKimiExecutor(&config.Config{}), + } + for name, registered := range cases { + t.Run(name, func(t *testing.T) { + first := claudeExecutorForAliasTest(t, registered.ForAPIKey()) + second := claudeExecutorForAliasTest(t, registered.ForAPIKey()) + if first == second { + t.Fatal("ForAPIKey returned the same executor; test needs two scoped copies") + } + first.rememberClaudeOAuthToolAliases(createBody, map[string]string{"alias": "Read"}, "msg-shared") + _, aliases, err := second.prepareClaudeOAuthToolNamesForRequest(continuationBody, claudeMCPAliasOptions{secret: "shared-store"}) + if err != nil { + t.Fatalf("continuation on another scoped copy: %v", err) + } + if aliases["alias"] != "Read" { + t.Fatalf("aliases = %#v, want alias -> Read", aliases) + } + }) + } +} + +func claudeExecutorForAliasTest(t *testing.T, scoped cliproxyauth.ProviderExecutor) *ClaudeExecutor { + t.Helper() + switch executor := scoped.(type) { + case *ClaudeExecutor: + return executor + case *KimiExecutor: + return &executor.ClaudeExecutor + default: + t.Fatalf("unexpected scoped executor %T", scoped) + return nil + } +} diff --git a/internal/runtime/executor/claude_executor_tool_state.go b/internal/runtime/executor/claude_executor_tool_state.go index c36f615a5..9785d7bc3 100644 --- a/internal/runtime/executor/claude_executor_tool_state.go +++ b/internal/runtime/executor/claude_executor_tool_state.go @@ -10,6 +10,11 @@ import ( const claudeOAuthToolAliasStateLimit = 1024 +// claudeOAuthToolAliasStoreInitMu guards lazy allocation for executors built +// without a constructor. It lives outside ClaudeExecutor because ForAPIKey +// copies the executor by value, and a mutex field would be copied with it. +var claudeOAuthToolAliasStoreInitMu sync.Mutex + type claudeOAuthToolAliasStore struct { mu sync.Mutex entries map[string]map[string]string @@ -69,8 +74,8 @@ func cloneClaudeOAuthToolAliasMap(source map[string]string) map[string]string { } func (e *ClaudeExecutor) claudeOAuthToolAliasStore() *claudeOAuthToolAliasStore { - e.oauthToolAliasStoreMu.Lock() - defer e.oauthToolAliasStoreMu.Unlock() + claudeOAuthToolAliasStoreInitMu.Lock() + defer claudeOAuthToolAliasStoreInitMu.Unlock() if e.oauthToolAliases == nil { e.oauthToolAliases = &claudeOAuthToolAliasStore{} } diff --git a/internal/runtime/executor/kimi_executor.go b/internal/runtime/executor/kimi_executor.go index 126e1e812..05e2408c2 100644 --- a/internal/runtime/executor/kimi_executor.go +++ b/internal/runtime/executor/kimi_executor.go @@ -42,6 +42,7 @@ func NewKimiExecutor(cfg *config.Config) *KimiExecutor { cfg: cfg, requestLogProvider: "kimi", upstreamModelNormalizer: normalizeKimiUpstreamModel, + oauthToolAliases: &claudeOAuthToolAliasStore{}, }, cfg: cfg, }