Skip to content

fix: unblock Cursor Composer provider — address review findings + resolve dev conflict#4055

Open
bradhallett wants to merge 12 commits into
router-for-me:devfrom
bradhallett:cursor-composer-direct-routing-fix
Open

fix: unblock Cursor Composer provider — address review findings + resolve dev conflict#4055
bradhallett wants to merge 12 commits into
router-for-me:devfrom
bradhallett:cursor-composer-direct-routing-fix

Conversation

@bradhallett

@bradhallett bradhallett commented Jun 29, 2026

Copy link
Copy Markdown

Follow-up to #3651 (Cursor Composer provider) — addresses bot review findings, resolves merge conflict with dev, and adds regression tests.

What changed

Conflict resolution (rebased onto current dev)

  • sdk/cliproxy/service.go: keeps upstream NewXAIAutoExecutor for xai, adds cursor-composer case

Gemini Code Assist findings addressed

  1. Checksum timestamp bugcursorChecksum was dividing Unix() by 1,000,000, collapsing the timestamp to zero. Switched to UnixMilli() with a deterministic cursorChecksumAt() helper. Added TestCursorChecksumAtUsesTimestamp.
  2. Token cache stampedeexchangeAPIKey now uses a mutex with double-checked locking so concurrent calls share a single upstream exchange. Added TestCursorComposerExchangeAPIKeyCachesConcurrentCalls.
  3. Nil-pointer note on management handlersmanagement.NewHandler always initializes cfg; no nil path exists in the current codebase. Did not add defensive guards that can't be triggered.

Original PR #3651 review blockers

  • EndStream termination: continuereturn after error yield
  • SDK bridge default: removed hardcoded 127.0.0.1:8792/sdk fallback so direct Cursor routing works when sdk-bridge-url is unset (config, synthesizer, and executor all updated)
  • Varint overflow: added shift >= 64 guard in readProtoVarint
  • Large-stream parsing: replaced bufio.Scanner with bufio.Reader (64KB buffer limit)
  • bytesTrimSpace: replaced with bytes.TrimSpace

Interface compatibility

  • Added ForceMapping field + GetForceMapping() to CursorComposerModel to satisfy upstream dev's model alias interface

Tests added

  • TestCursorSessionIDUsesAccessTokenHash (restored)
  • TestCursorComposerSDKBridgeURLEmptyWhenUnset
  • TestCursorComposerSDKBridgeURLNormalizesExplicitValue
  • TestReadProtoVarintOverflow
  • TestParseConnectProtoFramesStopsAfterEndStream
  • TestCursorChecksumAtUsesTimestamp
  • TestCursorComposerExchangeAPIKeyCachesConcurrentCalls
  • TestSanitizeCursorComposerKeysLeavesSDKBridgeURLEmptyWhenUnset
  • TestSanitizeCursorComposerKeysKeepsExplicitSDKBridgeURL
  • TestBytesTrimSpace (darwin)

Verification

  • go test ./... — all pass
  • go build -o test-output ./cmd/server && rm test-output — compiles clean
  • gofmt -w . — formatted

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for Cursor Composer API keys, adding configuration management, API endpoints, transport resolution, and a dedicated executor to handle chat completions and streaming via Cursor's Connect-RPC protocol. Feedback on the changes highlights a critical bug in the checksum timestamp calculation, multiple potential nil pointer dereferences in the management handlers when h.cfg is uninitialized, and a recommendation to use a mutex with double-checked locking to prevent cache stampedes during concurrent token exchanges.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.


func cursorChecksum(identity string) string {
machineID := sha256Hex("composer-api:cursor-machine:" + identity)
timestamp := uint64(time.Now().Unix() / 1_000_000)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The timestamp calculation is off by a factor of 1,000. time.Now().Unix() returns seconds, but the Cursor checksum algorithm expects milliseconds divided by 1,000,000 (equivalent to seconds divided by 1,000). This will cause the generated checksum to be incorrect and potentially lead to authentication or signature verification failures on the Cursor backend.

Suggested change
timestamp := uint64(time.Now().Unix() / 1_000_000)
timestamp := uint64(time.Now().UnixMilli() / 1_000_000)

Comment on lines +1114 to +1116
h.mu.Lock()
defer h.mu.Unlock()
h.cfg.CursorComposerKey = append([]config.CursorComposerKey(nil), arr...)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If h.cfg is nil, attempting to access h.cfg.CursorComposerKey will cause a nil pointer dereference panic. A defensive check should be added to ensure the configuration is initialized before accessing it.

	h.mu.Lock()
	defer h.mu.Unlock()
	if h.cfg == nil {
		c.JSON(500, gin.H{"error": "configuration not initialized"})
		return
	}
	h.cfg.CursorComposerKey = append([]config.CursorComposerKey(nil), arr...)

Comment on lines +1150 to +1152
h.mu.Lock()
defer h.mu.Unlock()
targetIndex := -1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If h.cfg is nil, accessing h.cfg.CursorComposerKey will cause a nil pointer dereference panic. A defensive check should be added to ensure the configuration is initialized before accessing it.

	h.mu.Lock()
	defer h.mu.Unlock()
	if h.cfg == nil {
		c.JSON(500, gin.H{"error": "configuration not initialized"})
		return
	}
	targetIndex := -1

Comment on lines +1229 to +1231
h.mu.Lock()
defer h.mu.Unlock()
if val := strings.TrimSpace(c.Query("api-key")); val != "" {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If h.cfg is nil, accessing h.cfg.CursorComposerKey will cause a nil pointer dereference panic. A defensive check should be added to ensure the configuration is initialized before accessing it.

	h.mu.Lock()
	defer h.mu.Unlock()
	if h.cfg == nil {
		c.JSON(500, gin.H{"error": "configuration not initialized"})
		return
	}
	if val := strings.TrimSpace(c.Query("api-key")); val != "" {

Comment on lines +42 to +46
type CursorComposerExecutor struct {
cfg *config.Config
identityCache sync.Map
tokenCache sync.Map
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Add a mutex to prevent concurrent duplicate token exchange requests (thundering herd/cache stampede) when multiple requests arrive simultaneously for the same API key.

Suggested change
type CursorComposerExecutor struct {
cfg *config.Config
identityCache sync.Map
tokenCache sync.Map
}
type CursorComposerExecutor struct {
cfg *config.Config
identityCache sync.Map
tokenCache sync.Map
exchangeMu sync.Mutex
}

Comment on lines +343 to +348
func (e *CursorComposerExecutor) exchangeAPIKey(ctx context.Context, auth *cliproxyauth.Auth, apiKey, backendBase string) (string, error) {
if val, ok := e.tokenCache.Load(apiKey); ok {
if entry, ok := val.(cursorComposerTokenCacheEntry); ok && entry.token != "" && time.Now().Before(entry.expiresAt) {
return entry.token, nil
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Implement double-checked locking using the new exchangeMu mutex to serialize concurrent token exchange requests for the same API key, preventing cache stampedes.

func (e *CursorComposerExecutor) exchangeAPIKey(ctx context.Context, auth *cliproxyauth.Auth, apiKey, backendBase string) (string, error) {
	if val, ok := e.tokenCache.Load(apiKey); ok {
		if entry, ok := val.(cursorComposerTokenCacheEntry); ok && entry.token != "" && time.Now().Before(entry.expiresAt) {
			return entry.token, nil
		}
	}

	e.exchangeMu.Lock()
	defer e.exchangeMu.Unlock()

	if val, ok := e.tokenCache.Load(apiKey); ok {
		if entry, ok := val.(cursorComposerTokenCacheEntry); ok && entry.token != "" && time.Now().Before(entry.expiresAt) {
			return entry.token, nil
		}
	}

artile and others added 11 commits June 29, 2026 10:55
Add a Cursor Composer API-key provider that adapts OpenAI-compatible chat requests to Cursor Composer's connect-proto flow, based on the originator reference at https://github.com/standardagents/composer-api.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Resolve Cursor Composer model aliases for API-key configs, cache Cursor identity/token lookups, and harden request/protobuf handling.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Propagate Cursor Connect end-stream errors and treat truncated frame headers as upstream response failures.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Do not advertise image input for Cursor Composer models until image content is forwarded upstream.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Capture VPS + SDK bridge setup, OpenCode client wiring, model registration
pitfalls, and common errors from operational work on this branch.

Co-authored-by: Cursor <cursoragent@cursor.com>
- Route crsr_ keys through optional sdk-bridge-url (local Cursor SDK bridge)
- Add cursor-composer-api-key management routes (alias cursor-api-key)
- Merge default Composer model IDs on auth registration to avoid stale sets
- Share backend/chat/client-version resolution via internal/cursorcomposer
- Document config, bridge mapping, and ops notes in config.example.yaml

Co-authored-by: Cursor <cursoragent@cursor.com>
Keep AGENTS.md unchanged on this branch; deployment notes stay out of repo docs.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@bradhallett bradhallett force-pushed the cursor-composer-direct-routing-fix branch from 3c25d46 to 6527509 Compare June 29, 2026 15:02
@bradhallett bradhallett changed the title fix: unblock Cursor Composer direct routing fix: unblock Cursor Composer provider — address review findings + resolve dev conflict Jun 29, 2026
@bradhallett

Copy link
Copy Markdown
Author

All three findings from the Gemini Code Assist review have been addressed in the latest commits:

1. Checksum timestamp (critical) — Fixed. cursorChecksum was dividing Unix() by 1M, which collapsed the timestamp to zero. Now uses UnixMilli() with a testable cursorChecksumAt() helper. Regression test: TestCursorChecksumAtUsesTimestamp.

2. Nil-pointer in management handlers — Investigated. management.NewHandler() always initializes cfg from a non-nil *config.Config. No nil path exists in the current codebase. Did not add guards for a condition that cannot occur.

3. Token cache stampede — Fixed. exchangeAPIKey now uses a mutex with double-checked locking so concurrent requests for the same API key share a single upstream exchange. Regression test: TestCursorComposerExchangeAPIKeyCachesConcurrentCalls (8 goroutines, asserts exactly 1 upstream request).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants