Skip to content

fix(server): honor Cursor long-context selections - #18685

Closed
emcee6798 wants to merge 2 commits into
pingdotgg:mainfrom
emcee6798:t3code/fix-orchestrator-issue-15788
Closed

emcee6798 wants to merge 2 commits into
pingdotgg:mainfrom
emcee6798:t3code/fix-orchestrator-issue-15788

Conversation

@emcee6798

Copy link
Copy Markdown

Problem

Cursor SDK requests omit RequestedModel.maxMode, so Grok 4.7 at 500K fails with AI Model Not Found, while other long-context selections can use the wrong tier. This is the bug reported in #15788 and originally fixed by @MatthewFeroz in #15884.

Change

This reapplies #15884 to current main after the Cursor provider moved into packages/provider-cursor. Selections above 300K set Max Mode; standard tiers remain unchanged. All implementation credit and development effort belong to @MatthewFeroz and the contributors to #15884. This replacement PR exists only to resolve the merge conflicts, and the original commit remains authored by Matthew Feroz.

Scope and approval

This is the same focused obvious-bug fix as #15884, with no unrelated cleanup. Relevant feedback from the original PR is preserved by narrowly patching only agent.v1.RequestedModel, retaining existing values, and testing the real SDK request path. The separate default-option issue in #16149 and generic provider-error presentation are intentionally out of scope.

Verification

From packages/provider-cursor:

  • vp test run src/server/CursorSdk.test.ts src/server/sdk.test.ts — 8 tests passed, including real SDK request checks for Grok 4.7 at 500K and 256K.
  • Targeted format and lint checks passed.
  • vp run typecheck passed with existing unrelated Effect suggestions.

No live authenticated Cursor turn was run on this rebased commit. #15884 contains the original macOS and Linux live verification.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 12, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The production change is a focused Cursor request fix with targeted regression coverage, but the new test file adds an @effect-diagnostics directive that suppresses static-analysis diagnostics. That suppression requires human review under the repository’s review criteria.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fe2764af-d2f6-4c8d-ad2c-a4baf330f6fe

📥 Commits

Reviewing files that changed from the base of the PR and between c047776 and 3958063.


📒 Files selected for processing (1)
  • packages/provider-cursor/src/server/sdk.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.



📝 Walkthrough

Walkthrough

The Cursor SDK protobuf initializer now enables Max Mode for RequestedModel instances with context tiers above 300,000 when Max Mode is not already enabled. Tests cover real SDK requests and synthetic protobuf serialization.

Changes

Cursor Max Mode Context Handling

Layer / File(s) Summary
Enable Max Mode for qualifying context tiers
packages/provider-cursor/src/server/sdk.ts
The wrapped protobuf initializer enables maxMode when the target is a RequestedModel, maxMode is false, and the context tier exceeds 300,000.
Verify SDK and protobuf behavior
packages/provider-cursor/src/server/sdk.test.ts
Child-process probes and assertions cover real SDK requests and synthetic serialization, including unchanged source objects and an unrelated message type.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge


Merge Risk | ⚪ Minimal · up to 39580

Merge Risk: ⚪ Minimal · up to 39580

This change turns on Max Mode for Cursor selections above 300K context and leaves other tiers unchanged. No actionable merge-blocking risk was found, and the added tests cover the threshold and unchanged cases.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 39580

The behavior change is narrow, with no demonstrated expansion of access or weakening of protections. Live authenticated behavior has not been verified on this revision.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The override can affect every qualifying RequestedModel initialized through the shared SDK protobuf runtime in the process, rather than only the reported model selection. Its exact type-name guard limits the added mutation to that request type.

Trust Boundaries and Controls

  • observed — The routed runProbe, maxMode, and sourceUnchanged entities belong to the test harness and assertions. runProbe launches a fixed probe program with test-selected modes; these ranges do not introduce a remotely callable production entrypoint.
  • inferred — Context selection now influences the request tier, but the inspected change does not assign credentials, change agent identity, or modify sandbox policy. Production option construction is unchanged against the full PR base; downstream entitlement enforcement remains outside the verified evidence.

Resilience and Maintainability Implications

  • observed — The test harness isolates interception in a fresh child process, disposes each created agent in a finally block, and scopes child execution with a five-second timeout. These mechanisms contain test-only overrides rather than changing production lifecycle controls.

Pre-merge checks | Passed 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title is concise, uses the conventional commit format, and clearly describes the main fix for Cursor long-context selections.
Description check Passed The description covers the problem, change, scope and approval rationale, verification steps, test results, and known verification limits. It is focused and consistent with the changeset.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@emcee6798

Copy link
Copy Markdown
Author

Addressed the diagnostic-suppression finding in 3958063. The new SDK regression test now runs its subprocess probes through Effect’s scoped child-process service and timeout, so it no longer adds an @effect-diagnostics suppression. The SDK send and serialization assertions are unchanged.

The 8 targeted tests passed, along with targeted lint and formatting and the Cursor package typecheck. Typecheck reported only existing suggestions in other files.

@emcee6798

emcee6798 commented Oct 12, 2026 •

Copy link
Copy Markdown
Author

The workflows for the latest head, 3958063, are waiting for maintainer approval. All four report action_required and have no started jobs or failure logs. Could a maintainer approve these workflow runs?

The diagnostic-suppression fix is pushed, and the 8 targeted tests, targeted lint and formatting, and Cursor package typecheck passed locally.

A repository maintainer also needs to request a Macroscope re-review of 3958063. The manual review request was rejected because the author lacks repository permissions. The original diagnostic-suppression finding has been addressed in that commit, but Macroscope has not evaluated the updated head.

@emcee6798

Copy link
Copy Markdown
Author

@macroscope-app review

@macroscopeapp

macroscopeapp Bot commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

@emcee6798

Copy link
Copy Markdown
Author

Superseded by #18700 to correct Git identity. The replacement has identical file contents and commit messages, preserves Matthew Feroz as author of the original fix, and uses emcee6798 as committer on both commits. The replacement commits passed the targeted checks before pushing.

@emcee6798 emcee6798 closed this Oct 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants