From 65cf04165876688b2e11b364d27a0211de5257c4 Mon Sep 17 00:00:00 2001 From: tan-yong-sheng <64836390+tan-yong-sheng@users.noreply.github.com> Date: Sun, 28 Jun 2026 00:40:57 +0800 Subject: [PATCH 1/4] fix(cursor): print login URL on stdout regardless of logging-to-file When logging-to-file is true, ConfigureLogOutput redirects all logrus output to ~/.cli-proxy-api/logs/main.log. The cursor login URL was therefore hidden in the log file and the program appeared to hang with no output on the terminal. Switch the four user-facing prompts (Starting / URL / Waiting / Success) in sdk/auth/cursor.go from log.* to fmt.Println/Printf so they always reach stdout, regardless of the logging-to-file setting. log.Warnf for the browser-open failure is kept on logrus since it is diagnostic, not user-facing. Fixes kaitranntt/CLIProxyAPIPlus#122 --- sdk/auth/cursor.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/sdk/auth/cursor.go b/sdk/auth/cursor.go index 792eb6063..04d074a9a 100644 --- a/sdk/auth/cursor.go +++ b/sdk/auth/cursor.go @@ -46,9 +46,14 @@ func (a CursorAuthenticator) Login(ctx context.Context, cfg *config.Config, opts return nil, fmt.Errorf("cursor: failed to generate auth params: %w", err) } - // Display the login URL - log.Info("Starting Cursor authentication...") - log.Infof("Please visit this URL to log in: %s", authParams.LoginURL) + // Display the login URL. These four prompts are user-facing CLI output + // for a one-shot login command, so they go to stdout via fmt (bypassing + // logrus) -- otherwise logging-to-file:true would hide them inside + // ~/.cli-proxy-api/logs/main.log and the user would see the program + // appear to hang (issue #122). The browser-open warning stays on logrus + // because it is a diagnostic, not a prompt. + fmt.Println("Starting Cursor authentication...") + fmt.Printf("Please visit this URL to log in: %s\n", authParams.LoginURL) // Try to open the browser automatically if !opts.NoBrowser { @@ -59,7 +64,7 @@ func (a CursorAuthenticator) Login(ctx context.Context, cfg *config.Config, opts } } - log.Info("Waiting for Cursor authorization...") + fmt.Println("Waiting for Cursor authorization...") // Poll for the auth result tokens, err := cursorauth.PollForAuth(ctx, authParams.UUID, authParams.Verifier) @@ -73,7 +78,7 @@ func (a CursorAuthenticator) Login(ctx context.Context, cfg *config.Config, opts sub := cursorauth.ParseJWTSub(tokens.AccessToken) subHash := cursorauth.SubToShortHash(sub) - log.Info("Cursor authentication successful!") + fmt.Println("Cursor authentication successful!") metadata := map[string]any{ "type": "cursor", From 69d09fe597f3996e78e3189b3d2a4d5c29fc9b27 Mon Sep 17 00:00:00 2001 From: tan-yong-sheng <64836390+tan-yong-sheng@users.noreply.github.com> Date: Sun, 28 Jun 2026 00:42:47 +0800 Subject: [PATCH 2/4] fix(cursor): cache last-good model list and refresh stale fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FetchCursorModels previously returned GetCursorFallbackModels() on every failure (DNS timeout, non-2xx, empty parse, etc.). The hardcoded list was also months out of date — it contained composer-2, claude-3.5-sonnet, gpt-4o, cursor-small, gemini-2.5-pro — none of which Cursor serves today. Combined effect: a single transient GetUsableModels failure (observed at 2026-06-27 21:53:33, dns i/o timeout to api2.cursor.sh) caused the registry reconcile to overwrite the live 119-model list with the stale 6-model fallback, permanently removing composer-2.5 and friends from the registry until the auth file was reloaded. User calls to cursor/composer-2.5 then returned an instant 503 with no executor trace. Two changes: 1. Add a package-level cache keyed by auth.ID. On a successful live fetch, store the result. On failure, return the cached list when available; fall back to GetCursorFallbackModels only when no prior fetch has populated the cache. A transient blip can no longer collapse the registry. 2. Refresh GetCursorFallbackModels to current Cursor model ids (composer-2.5, composer-2.5-fast, gpt-5.3-codex, gpt-5.2, gpt-5.5-high, gpt-5.4-high, claude-opus-4-8-thinking-high, claude-opus-4-7-thinking-high, claude-4.5-sonnet, gemini-3.1-pro). Verified live against api2.cursor.sh on 2026-06-27. Adds cursor_models_cache_test.go covering both the cache-or-fallback preference and the freshness of the hardcoded list. --- internal/runtime/executor/cursor_executor.go | 78 ++++++++++--- .../executor/cursor_models_cache_test.go | 106 ++++++++++++++++++ 2 files changed, 170 insertions(+), 14 deletions(-) create mode 100644 internal/runtime/executor/cursor_models_cache_test.go diff --git a/internal/runtime/executor/cursor_executor.go b/internal/runtime/executor/cursor_executor.go index eb1748fd1..0ffb0c350 100644 --- a/internal/runtime/executor/cursor_executor.go +++ b/internal/runtime/executor/cursor_executor.go @@ -1479,11 +1479,51 @@ func decodeMcpArgsToJSON(args map[string][]byte) string { // --- Model Discovery --- -// FetchCursorModels retrieves available models from Cursor's API. +// cursorModelsCache stores the last successful model list per auth so a +// transient GetUsableModels failure does not collapse the model registry to +// the hardcoded fallback (which can drop models the user actually calls, +// e.g. composer-2.5). Keyed by auth.ID (the auth file name). +var ( + cursorModelsCacheMu sync.RWMutex + cursorModelsCache = make(map[string][]*registry.ModelInfo) +) + +// cursorModelsOrFallback returns the cached model list for authID when one +// exists, otherwise the hardcoded fallback. The fallback is only ever used +// when no prior successful fetch has populated the cache for this auth. +func cursorModelsOrFallback(authID string) []*registry.ModelInfo { + if authID != "" { + cursorModelsCacheMu.RLock() + cached, ok := cursorModelsCache[authID] + cursorModelsCacheMu.RUnlock() + if ok && len(cached) > 0 { + return cached + } + } + return GetCursorFallbackModels() +} + +// cacheCursorModels records a successful model fetch for future fallback use. +func cacheCursorModels(authID string, models []*registry.ModelInfo) { + if authID == "" || len(models) == 0 { + return + } + cursorModelsCacheMu.Lock() + cursorModelsCache[authID] = models + cursorModelsCacheMu.Unlock() +} + +// FetchCursorModels retrieves available models from Cursor's API. On failure, +// returns the last successful model list cached for this auth when available; +// only falls back to GetCursorFallbackModels() when no cached list exists. +// This prevents a transient network blip from permanently shrinking the model +// registry to stale hardcoded names. func FetchCursorModels(ctx context.Context, auth *cliproxyauth.Auth, cfg *config.Config) []*registry.ModelInfo { + authID := auth.ID + accessToken := cursorAccessToken(auth) if accessToken == "" { - return GetCursorFallbackModels() + return cursorModelsOrFallback(authID) } ctx, cancel := context.WithTimeout(ctx, 5*time.Second) @@ -1497,7 +1537,7 @@ func FetchCursorModels(ctx context.Context, auth *cliproxyauth.Auth, cfg *config cursorAPIURL+cursorModelsPath, bytes.NewReader(emptyReq)) if err != nil { log.Debugf("cursor: failed to create models request: %v", err) - return GetCursorFallbackModels() + return cursorModelsOrFallback(authID) } h2Req.Header.Set("Content-Type", "application/proto") @@ -1511,24 +1551,26 @@ func FetchCursorModels(ctx context.Context, auth *cliproxyauth.Auth, cfg *config resp, err := client.Do(h2Req) if err != nil { log.Debugf("cursor: models request failed: %v", err) - return GetCursorFallbackModels() + return cursorModelsOrFallback(authID) } defer resp.Body.Close() if resp.StatusCode < 200 || resp.StatusCode >= 300 { log.Debugf("cursor: models request returned status %d", resp.StatusCode) - return GetCursorFallbackModels() + return cursorModelsOrFallback(authID) } body, err := io.ReadAll(resp.Body) if err != nil { - return GetCursorFallbackModels() + return cursorModelsOrFallback(authID) } models := parseModelsResponse(body) if len(models) == 0 { - return GetCursorFallbackModels() + return cursorModelsOrFallback(authID) } + + cacheCursorModels(authID, models) return models } @@ -1655,15 +1697,23 @@ func parseModelEntry(data []byte) *registry.ModelInfo { return info } -// GetCursorFallbackModels returns hardcoded fallback models. +// GetCursorFallbackModels returns hardcoded fallback models used when no +// cached live-fetch result is available. Kept current with Cursor's actual +// catalog so a transient GetUsableModels failure does not remove models the +// user can still call (notably composer-2.5). Update when Cursor retires or +// renames any of these ids; verified live against api2.cursor.sh on 2026-06-27. func GetCursorFallbackModels() []*registry.ModelInfo { return []*registry.ModelInfo{ - {ID: "composer-2", Object: "model", OwnedBy: "cursor", Type: cursorAuthType, DisplayName: "Composer 2", ContextLength: 200000, MaxCompletionTokens: 64000, Thinking: ®istry.ThinkingSupport{Max: 50000, DynamicAllowed: true}}, - {ID: "claude-4-sonnet", Object: "model", OwnedBy: "cursor", Type: cursorAuthType, DisplayName: "Claude 4 Sonnet", ContextLength: 200000, MaxCompletionTokens: 64000, Thinking: ®istry.ThinkingSupport{Max: 50000, DynamicAllowed: true}}, - {ID: "claude-3.5-sonnet", Object: "model", OwnedBy: "cursor", Type: cursorAuthType, DisplayName: "Claude 3.5 Sonnet", ContextLength: 200000, MaxCompletionTokens: 8192}, - {ID: "gpt-4o", Object: "model", OwnedBy: "cursor", Type: cursorAuthType, DisplayName: "GPT-4o", ContextLength: 128000, MaxCompletionTokens: 16384}, - {ID: "cursor-small", Object: "model", OwnedBy: "cursor", Type: cursorAuthType, DisplayName: "Cursor Small", ContextLength: 200000, MaxCompletionTokens: 64000}, - {ID: "gemini-2.5-pro", Object: "model", OwnedBy: "cursor", Type: cursorAuthType, DisplayName: "Gemini 2.5 Pro", ContextLength: 1000000, MaxCompletionTokens: 65536, Thinking: ®istry.ThinkingSupport{Max: 50000, DynamicAllowed: true}}, + {ID: "composer-2.5", Object: "model", OwnedBy: "cursor", Type: cursorAuthType, DisplayName: "Composer 2.5", ContextLength: 200000, MaxCompletionTokens: 64000, Thinking: ®istry.ThinkingSupport{Max: 50000, DynamicAllowed: true}}, + {ID: "composer-2.5-fast", Object: "model", OwnedBy: "cursor", Type: cursorAuthType, DisplayName: "Composer 2.5 Fast", ContextLength: 200000, MaxCompletionTokens: 64000}, + {ID: "gpt-5.3-codex", Object: "model", OwnedBy: "cursor", Type: cursorAuthType, DisplayName: "Codex 5.3", ContextLength: 256000, MaxCompletionTokens: 32768}, + {ID: "gpt-5.2", Object: "model", OwnedBy: "cursor", Type: cursorAuthType, DisplayName: "GPT-5.2", ContextLength: 256000, MaxCompletionTokens: 32768}, + {ID: "gpt-5.5-high", Object: "model", OwnedBy: "cursor", Type: cursorAuthType, DisplayName: "GPT-5.5 High", ContextLength: 256000, MaxCompletionTokens: 32768}, + {ID: "gpt-5.4-high", Object: "model", OwnedBy: "cursor", Type: cursorAuthType, DisplayName: "GPT-5.4 High", ContextLength: 256000, MaxCompletionTokens: 32768}, + {ID: "claude-opus-4-8-thinking-high", Object: "model", OwnedBy: "cursor", Type: cursorAuthType, DisplayName: "Claude Opus 4.8 Thinking High", ContextLength: 200000, MaxCompletionTokens: 64000, Thinking: ®istry.ThinkingSupport{Max: 50000, DynamicAllowed: true}}, + {ID: "claude-opus-4-7-thinking-high", Object: "model", OwnedBy: "cursor", Type: cursorAuthType, DisplayName: "Claude Opus 4.7 Thinking High", ContextLength: 200000, MaxCompletionTokens: 64000, Thinking: ®istry.ThinkingSupport{Max: 50000, DynamicAllowed: true}}, + {ID: "claude-4.5-sonnet", Object: "model", OwnedBy: "cursor", Type: cursorAuthType, DisplayName: "Claude 4.5 Sonnet", ContextLength: 200000, MaxCompletionTokens: 8192}, + {ID: "gemini-3.1-pro", Object: "model", OwnedBy: "cursor", Type: cursorAuthType, DisplayName: "Gemini 3.1 Pro", ContextLength: 1000000, MaxCompletionTokens: 65536, Thinking: ®istry.ThinkingSupport{Max: 50000, DynamicAllowed: true}}, } } diff --git a/internal/runtime/executor/cursor_models_cache_test.go b/internal/runtime/executor/cursor_models_cache_test.go new file mode 100644 index 000000000..32f2b31d7 --- /dev/null +++ b/internal/runtime/executor/cursor_models_cache_test.go @@ -0,0 +1,106 @@ +package executor + +import ( + "testing" + + "github.com/router-for-me/CLIProxyAPI/v7/internal/registry" +) + +// withCursorModelsCache swaps the package-level cache for the duration of a test +// and restores the previous state on cleanup. +func withCursorModelsCache(t *testing.T, seed map[string][]*registry.ModelInfo) { + t.Helper() + cursorModelsCacheMu.Lock() + prev := cursorModelsCache + cursorModelsCache = make(map[string][]*registry.ModelInfo, len(seed)) + for k, v := range seed { + // copy to avoid test mutation leaking across runs + dup := make([]*registry.ModelInfo, len(v)) + copy(dup, v) + cursorModelsCache[k] = dup + } + cursorModelsCacheMu.Unlock() + t.Cleanup(func() { + cursorModelsCacheMu.Lock() + cursorModelsCache = prev + cursorModelsCacheMu.Unlock() + }) +} + +// TestCursorModelsOrFallback_PrefersCacheOverHardcoded ensures that when a +// previous successful fetch cached models for an auth, a subsequent fetch +// failure returns the cached models instead of the hardcoded fallback. +// This prevents the live->fallback->live churn that removes working models +// (e.g. composer-2.5) from the registry after a transient network blip. +func TestCursorModelsOrFallback_PrefersCacheOverHardcoded(t *testing.T) { + cached := []*registry.ModelInfo{ + {ID: "composer-2.5", Object: "model", OwnedBy: "cursor", Type: cursorAuthType, DisplayName: "Composer 2.5"}, + } + withCursorModelsCache(t, map[string][]*registry.ModelInfo{ + "auth-with-cache": cached, + }) + + got := cursorModelsOrFallback("auth-with-cache") + if len(got) != 1 || got[0].ID != "composer-2.5" { + t.Fatalf("expected cached [composer-2.5], got %+v", got) + } + + // Unknown auth id with no cache entry must fall through to the hardcoded list. + fb := cursorModelsOrFallback("never-seen-auth") + if len(fb) == 0 { + t.Fatal("expected non-empty hardcoded fallback for unknown auth") + } +} + +// TestGetCursorFallbackModels_IsCurrent guards against the hardcoded list +// drifting to stale model ids (e.g. claude-3.5-sonnet, gpt-4o) and against +// it losing the models users actually call (e.g. composer-2.5). +func TestGetCursorFallbackModels_IsCurrent(t *testing.T) { + fb := GetCursorFallbackModels() + if len(fb) == 0 { + t.Fatal("hardcoded fallback must not be empty") + } + + ids := make(map[string]bool, len(fb)) + for _, m := range fb { + ids[m.ID] = true + } + + // Must contain: the model the user actually calls, plus a representative + // slice of current Cursor model families. + mustHave := []string{ + "composer-2.5", + "composer-2.5-fast", + "gpt-5.3-codex", + "gpt-5.2", + "claude-opus-4-8-thinking-high", + "gemini-3.1-pro", + } + for _, id := range mustHave { + if !ids[id] { + t.Errorf("hardcoded fallback missing required current model %q; have %v", id, mapKeys(ids)) + } + } + + // Must NOT contain: model ids Cursor no longer serves (verified by a live + // GetUsableModels call returning 0 occurrences of these names). + mustNotHave := []string{ + "claude-3.5-sonnet", + "gpt-4o", + "cursor-small", + "gemini-2.5-pro", + } + for _, id := range mustNotHave { + if ids[id] { + t.Errorf("hardcoded fallback still contains stale model %q (Cursor no longer serves it)", id) + } + } +} + +func mapKeys(m map[string]bool) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + return out +} From 59b83e020d93ecda05c3faeb3e652e48bf0f0f91 Mon Sep 17 00:00:00 2001 From: tan-yong-sheng <64836390+tan-yong-sheng@users.noreply.github.com> Date: Sun, 28 Jun 2026 00:43:09 +0800 Subject: [PATCH 3/4] fix(cursor): honor oauth-model-alias by sending resolved model to Run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cursor executor used parsed.Model (the model name extracted from the translated request payload) when building the upstream Run request to api2.cursor.sh. For OpenAI-source requests the translator is skipped entirely (the from!=openai guard at line 293), so parsed.Model always equalled whatever the client sent — including an alias name like "cursor/composer-2.5" defined under oauth-model-alias in config.yaml. The conductor does resolve the alias correctly: applyOAuthModelAlias in sdk/cliproxy/auth/conductor.go sets the upstream model name on req.Model. But because parsed.Model still held the alias, the protobuf Run request was encoded with the alias as ModelId. Cursor's Run endpoint does not know "cursor/composer-2.5" (only "composer-2.5") and returned Connect error not_found. Calling the upstream model name directly worked because no alias was involved and the payload already held the real name. Fix: give buildRunRequestParams a third modelOverride parameter. When non-empty it is used as ModelId; otherwise parsed.Model is used so existing call sites behave unchanged. All four call sites (Execute + three sites in ExecuteStream's checkpoint branches) pass req.Model. parsed.Model is untouched, so the OpenAI response still echoes the client-facing alias name as before. Adds cursor_executor_buildrequest_test.go covering override wins over parsed.Model, empty override falls back, whitespace override falls back, and override wins even when parsed.Model is empty. --- internal/runtime/executor/cursor_executor.go | 21 +++++-- .../cursor_executor_buildrequest_test.go | 57 +++++++++++++++++++ 2 files changed, 72 insertions(+), 6 deletions(-) create mode 100644 internal/runtime/executor/cursor_executor_buildrequest_test.go diff --git a/internal/runtime/executor/cursor_executor.go b/internal/runtime/executor/cursor_executor.go index 0ffb0c350..6a31c8fcf 100644 --- a/internal/runtime/executor/cursor_executor.go +++ b/internal/runtime/executor/cursor_executor.go @@ -296,7 +296,7 @@ func (e *CursorExecutor) Execute(ctx context.Context, auth *cliproxyauth.Auth, r parsed := parseOpenAIRequest(payload) ccSessId := extractClaudeCodeSessionId(req.Payload) conversationId := deriveConversationId(apiKeyFromContext(ctx), ccSessId, parsed.SystemPrompt) - params := buildRunRequestParams(parsed, conversationId) + params := buildRunRequestParams(parsed, conversationId, req.Model) requestBytes := cursorproto.EncodeRunRequest(params) framedRequest := cursorproto.FrameConnectMessage(requestBytes, 0) @@ -455,7 +455,7 @@ func (e *CursorExecutor) ExecuteStream(ctx context.Context, auth *cliproxyauth.A saved, hasCheckpoint := e.checkpoints[checkpointKey] e.mu.Unlock() - params := buildRunRequestParams(parsed, conversationId) + params := buildRunRequestParams(parsed, conversationId, req.Model) if hasCheckpoint && saved.data != nil && saved.authID == authID { // Same auth — use checkpoint normally @@ -479,7 +479,7 @@ func (e *CursorExecutor) ExecuteStream(ctx context.Context, auth *cliproxyauth.A e.mu.Unlock() if len(parsed.ToolResults) > 0 || len(parsed.Turns) > 0 { flattenConversationIntoUserText(parsed) - params = buildRunRequestParams(parsed, conversationId) + params = buildRunRequestParams(parsed, conversationId, req.Model) } } else if len(parsed.ToolResults) > 0 || len(parsed.Turns) > 0 { // Fallback: no checkpoint available (cold resume / proxy restart). @@ -487,7 +487,7 @@ func (e *CursorExecutor) ExecuteStream(ctx context.Context, auth *cliproxyauth.A // Cursor's turns encoding is not reliably read by the model, but userText always works. log.Debugf("cursor: no checkpoint, flattening %d turns + %d tool results into userText", len(parsed.Turns), len(parsed.ToolResults)) flattenConversationIntoUserText(parsed) - params = buildRunRequestParams(parsed, conversationId) + params = buildRunRequestParams(parsed, conversationId, req.Model) } requestBytes := cursorproto.EncodeRunRequest(params) framedRequest := cursorproto.FrameConnectMessage(requestBytes, 0) @@ -1289,9 +1289,18 @@ func parseDataURL(url string) *cursorproto.ImageData { } } -func buildRunRequestParams(parsed *parsedOpenAIRequest, conversationId string) *cursorproto.RunRequestParams { +func buildRunRequestParams(parsed *parsedOpenAIRequest, conversationId string, modelOverride string) *cursorproto.RunRequestParams { + // modelOverride lets callers supply an upstream-resolved model name (e.g. the + // result of resolving an oauth-model-alias entry) while keeping the original + // client-facing model name in parsed.Model for response formatting. When empty, + // fall back to parsed.Model so existing call sites that don't need an override + // behave unchanged. + modelId := strings.TrimSpace(modelOverride) + if modelId == "" { + modelId = parsed.Model + } params := &cursorproto.RunRequestParams{ - ModelId: parsed.Model, + ModelId: modelId, SystemPrompt: parsed.SystemPrompt, UserText: parsed.UserText, MessageId: uuid.New().String(), diff --git a/internal/runtime/executor/cursor_executor_buildrequest_test.go b/internal/runtime/executor/cursor_executor_buildrequest_test.go new file mode 100644 index 000000000..d6de28a09 --- /dev/null +++ b/internal/runtime/executor/cursor_executor_buildrequest_test.go @@ -0,0 +1,57 @@ +package executor + +import "testing" + +// TestBuildRunRequestParams_ModelOverride verifies that buildRunRequestParams +// honors the modelOverride parameter when supplied (so the upstream Cursor Run +// call receives the resolved model name from an oauth-model-alias entry), +// and falls back to parsed.Model when the override is empty or whitespace. +func TestBuildRunRequestParams_ModelOverride(t *testing.T) { + tests := []struct { + name string + parsedModel string + override string + wantModelId string + }{ + { + name: "override wins over parsed.Model", + parsedModel: "cursor/composer-2.5", + override: "composer-2.5", + wantModelId: "composer-2.5", + }, + { + name: "empty override falls back to parsed.Model", + parsedModel: "composer-2.5", + override: "", + wantModelId: "composer-2.5", + }, + { + name: "whitespace override falls back to parsed.Model", + parsedModel: "composer-2.5", + override: " \t ", + wantModelId: "composer-2.5", + }, + { + name: "override wins even when parsed.Model is empty", + parsedModel: "", + override: "composer-2.5", + wantModelId: "composer-2.5", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + parsed := &parsedOpenAIRequest{Model: tc.parsedModel} + params := buildRunRequestParams(parsed, "conv-123", tc.override) + if params == nil { + t.Fatal("buildRunRequestParams returned nil") + } + if params.ModelId != tc.wantModelId { + t.Errorf("ModelId = %q, want %q", params.ModelId, tc.wantModelId) + } + if params.ConversationId != "conv-123" { + t.Errorf("ConversationId = %q, want %q", params.ConversationId, "conv-123") + } + }) + } +} From c996e20c7a7f2c6aeee539bc70aa646d2000e82c Mon Sep 17 00:00:00 2001 From: tan-yong-sheng <64836390+tan-yong-sheng@users.noreply.github.com> Date: Sun, 28 Jun 2026 01:16:09 +0800 Subject: [PATCH 4/4] fix(cursor): harden model cache against nil auth and slice aliasing Addresses three findings from PR #1 review (Gemini Code Assist): 1. FetchCursorModels dereferenced auth.ID before any nil check, so a nil auth (legal per the function signature; caller in service.go currently passes a non-nil auth but the signature allows nil) would panic at line 1536 and crash the registry reconcile goroutine. Add an explicit nil guard at function entry that returns the hardcoded fallback. 2. cursorModelsOrFallback returned the cached slice directly. If a caller replaced a slice element via the returned slice (e.g. 'got[0] = newEntry'), the change propagated to the cache and corrupted future failure-path fetches. Return a shallow copy instead. ModelInfo pointer fields (e.g. *ThinkingSupport) are still shared -- callers must treat returned models as immutable, which is the existing convention used by registry reconcile. 3. cacheCursorModels stored the caller's slice directly. If the caller mutated its own slice header (append, element replace) after caching, the cache was corrupted. Store a shallow copy. Adds three regression tests covering each finding: - TestFetchCursorModels_NilAuthReturnsFallback - TestCursorModelsOrFallback_ReturnIsDefensivelyCopied - TestCacheCursorModels_StoresDefensiveCopy Verified go test -race on the executor package. --- internal/runtime/executor/cursor_executor.go | 22 ++++++- .../executor/cursor_models_cache_test.go | 63 +++++++++++++++++++ 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/internal/runtime/executor/cursor_executor.go b/internal/runtime/executor/cursor_executor.go index 6a31c8fcf..6aae1c5a8 100644 --- a/internal/runtime/executor/cursor_executor.go +++ b/internal/runtime/executor/cursor_executor.go @@ -1500,25 +1500,37 @@ var ( // cursorModelsOrFallback returns the cached model list for authID when one // exists, otherwise the hardcoded fallback. The fallback is only ever used // when no prior successful fetch has populated the cache for this auth. +// +// The returned slice is a shallow copy of the cached entry: callers may +// safely replace slice elements without corrupting the cache. Callers must +// still treat ModelInfo fields as read-only -- a full deep copy would also +// clone each entry's *ThinkingSupport, which is unnecessary given that +// downstream consumers (registry reconcile) treat models as immutable. func cursorModelsOrFallback(authID string) []*registry.ModelInfo { if authID != "" { cursorModelsCacheMu.RLock() cached, ok := cursorModelsCache[authID] cursorModelsCacheMu.RUnlock() if ok && len(cached) > 0 { - return cached + dup := make([]*registry.ModelInfo, len(cached)) + copy(dup, cached) + return dup } } return GetCursorFallbackModels() } // cacheCursorModels records a successful model fetch for future fallback use. +// A shallow copy of the slice is stored so the caller can freely mutate its +// own slice header (append, element replacement) without corrupting the cache. func cacheCursorModels(authID string, models []*registry.ModelInfo) { if authID == "" || len(models) == 0 { return } + dup := make([]*registry.ModelInfo, len(models)) + copy(dup, models) cursorModelsCacheMu.Lock() - cursorModelsCache[authID] = models + cursorModelsCache[authID] = dup cursorModelsCacheMu.Unlock() } @@ -1527,7 +1539,13 @@ func cacheCursorModels(authID string, models []*registry.ModelInfo) { // only falls back to GetCursorFallbackModels() when no cached list exists. // This prevents a transient network blip from permanently shrinking the model // registry to stale hardcoded names. +// +// A nil auth is tolerated by returning the hardcoded fallback; this avoids a +// panic in registry reconcile paths where auth may legitimately be nil. func FetchCursorModels(ctx context.Context, auth *cliproxyauth.Auth, cfg *config.Config) []*registry.ModelInfo { + if auth == nil { + return GetCursorFallbackModels() + } authID := auth.ID accessToken := cursorAccessToken(auth) diff --git a/internal/runtime/executor/cursor_models_cache_test.go b/internal/runtime/executor/cursor_models_cache_test.go index 32f2b31d7..3d3e98419 100644 --- a/internal/runtime/executor/cursor_models_cache_test.go +++ b/internal/runtime/executor/cursor_models_cache_test.go @@ -1,6 +1,7 @@ package executor import ( + "context" "testing" "github.com/router-for-me/CLIProxyAPI/v7/internal/registry" @@ -52,6 +53,68 @@ func TestCursorModelsOrFallback_PrefersCacheOverHardcoded(t *testing.T) { } } +// TestFetchCursorModels_NilAuthReturnsFallback guards against the auth +// pointer being nil. cursorAccessToken already nil-guards, but +// authID := auth.ID is evaluated before that helper is called, so a nil +// auth would panic. The function should degrade to the hardcoded fallback +// instead of crashing the goroutine that reconciles the model registry. +func TestFetchCursorModels_NilAuthReturnsFallback(t *testing.T) { + got := FetchCursorModels(context.Background(), nil, nil) + if got == nil { + t.Fatal("FetchCursorModels must return a non-nil slice for nil auth, got nil") + } + if len(got) == 0 { + t.Fatal("FetchCursorModels must return the hardcoded fallback for nil auth, got empty slice") + } +} + +// TestCursorModelsOrFallback_ReturnIsDefensivelyCopied guards against +// the returned cache slice aliasing the stored one. If the caller +// replaces a slice element (e.g. `got[0] = newEntry`), the cache must +// remain intact; otherwise a later failure-path fetch could return the +// caller's mutated value instead of the real cached list. +func TestCursorModelsOrFallback_ReturnIsDefensivelyCopied(t *testing.T) { + original := ®istry.ModelInfo{ID: "cached-original", Object: "model", OwnedBy: "cursor", Type: cursorAuthType} + withCursorModelsCache(t, map[string][]*registry.ModelInfo{ + "auth-iso-get": {original}, + }) + + got := cursorModelsOrFallback("auth-iso-get") + if len(got) != 1 || got[0].ID != "cached-original" { + t.Fatalf("setup: expected [cached-original], got %+v", got) + } + + // Caller replaces slice element via the returned slice; the cache must + // still hold the original element after this. + got[0] = ®istry.ModelInfo{ID: "caller-replaced"} + + again := cursorModelsOrFallback("auth-iso-get") + if len(again) != 1 || again[0].ID != "cached-original" { + t.Fatalf("cache was corrupted by caller element replacement: got %+v", again) + } +} + +// TestCacheCursorModels_StoresDefensiveCopy guards against the cache +// aliasing the caller's slice. If the caller replaces a slice element +// after caching, the cache must remain intact. +func TestCacheCursorModels_StoresDefensiveCopy(t *testing.T) { + withCursorModelsCache(t, nil) + + models := []*registry.ModelInfo{ + {ID: "original", Object: "model", OwnedBy: "cursor", Type: cursorAuthType}, + } + cacheCursorModels("auth-iso-set", models) + + // Caller replaces slice element after caching; the cache must still + // hold the original element. + models[0] = ®istry.ModelInfo{ID: "caller-replaced"} + + got := cursorModelsOrFallback("auth-iso-set") + if len(got) != 1 || got[0].ID != "original" { + t.Fatalf("cache was corrupted by caller slice mutation: got %+v", got) + } +} + // TestGetCursorFallbackModels_IsCurrent guards against the hardcoded list // drifting to stale model ids (e.g. claude-3.5-sonnet, gpt-4o) and against // it losing the models users actually call (e.g. composer-2.5).