[AiCaptain357]

⚠️ base-red inherited: #17219 — upstream’s release-branch failure tracker is still open.

Summary

Cursor model sync delegates discovery over an authenticated loopback request.
When Cursor HTTP discovery is unavailable, that hop appears local and can reach the
host CLI fallback, although direct remote requests cannot use that CLI fallback.

Carry authenticated deny-only context while retaining the independent loopback
check. Preserve trusted original locality for the in-process fallback too, so
local requests and independently local scheduler sync still work when self-fetch fails. HTTP discovery,
cached/local catalogs, cursor-api and scheduler/reactive policy retain their behavior.

Target branch

• Active release/v3.8.53, based on b2a6410.

Proof of fix

• Identical frozen tests/unit/api/cursor-model-sync-locality.spec.ts:
  before 12 failed | 19 passed (31); after 31/31 pass.

Validation

• Change type: provider. Only three application modules, two regression specs
  and one changelog fragment.
• tests/unit/api/cursor-model-sync-denial.spec.ts: 13/13 pass.
• Existing focused native tests: 156 pass, 1 host-dependent skip, 0 fail.
• Core typecheck, scoped ESLint, formatting, whitespace, changelog and Git hooks pass.

Regression tests use real handlers, isolated temporary data and recording process/network
boundaries. No real Cursor CLI/provider or production probe. Full repository lint,
unrelated test matrix and production build were not run for this change; broader CI remains pending.

[coderabbitai]

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

• Bug Fixes
  • Cursor model discovery now respects trusted request locality and restrictions on launching the local CLI. When local discovery is not permitted, the app uses cached or local results when available, or returns an access-denied response.
  • In-process model sync now preserves trusted locality information, preventing client-supplied claims from granting local CLI access.
• Tests
  • Added coverage for locality checks, access denial, fallback behavior, and successful remote discovery.

Cursor model sync now carries peer locality and authenticated local-spawn denial through delegated requests and in-process fallback. Cursor Agent discovery checks both conditions before spawning the local CLI. The changes also add denial and locality tests and a changelog entry.

Changes: Cursor model-sync locality

Layer / File(s)	Summary
Authenticated denial propagation src/shared/services/modelSyncScheduler.ts, src/app/api/providers/[id]/sync-models/route.ts, tests/unit/api/cursor-model-sync-denial.spec.ts	The internal-header helper can add an authenticated local-spawn denial marker. The sync route sets the marker based on peer locality and forwards locality during in-process fallback. Tests cover marker authentication, propagation, transport failures, and HTTP discovery.
Locality-gated model discovery src/app/api/providers/[id]/models/route.ts, tests/unit/api/cursor-model-sync-locality.spec.ts, changelog.d/fixes/0-cursor-sync-locality.md	The models route checks peer locality and the denial marker before Cursor Agent discovery. Tests cover locality handling across sync, fallback, direct model requests, and provider variants. The changelog describes the locality behavior.

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

Change: Bug fix

Merge Risk: Minimal · up to d22f9

The reviewed locality change is mergeable after normal checks; the scheduler retains its local CLI fallback.

Security architecture risk: Low · up to d22f9

The change restricts remote access to host-side discovery while preserving intended local behavior. No introduced security weakness was established, but production ingress and interruption behavior were not validated end to end.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — The protected capability is discovery through the serving host's Cursor CLI/login. Successful synchronization can affect both the requested connection's catalog and provider-wide custom-model overlays and aliases, so downstream state is broader than one catalog row.

Security Findings and Attack Paths

• inferred — The relevant base attack path was a nonlocal caller admitted by existing management authentication invoking sync, followed by an authenticated loopback discovery request reaching host CLI fallback when HTTP discovery was unavailable. The new denial propagation blocks that path at the spawn gate. This is a security improvement, not an introduced or worsened concern.

Trust Boundaries and Controls

• observed — The ingress pipeline removes client-supplied trusted headers and recomputes locality from validated peer stamps. Sync defaults absent locality to remote, while the final discovery gate independently requires loopback locality and absence of authenticated denial. Internal authentication therefore does not itself confer local CLI authority. Deployment safety depends on retaining this ingress boundary.

Resilience and Maintainability Implications

• inferred — Repeated and concurrent requests retain their own denial context because authorization headers are captured per invocation rather than in the shared readiness state. Catalog persistence still comprises sequential helper calls, and the outer error path returns 500 without an orchestrator-level rollback. Cancellation and full transaction guarantees remain unestablished; these characteristics predate the PR and were not shown to weaken its new authorization gate.

Pre-merge checks: 6 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 6 passed

Check name	Status	Explanation
Title check		The title follows the Conventional Commits format with type "fix" and scope "providers". It names the main change: preserving caller locality during Cursor model sync. The raw summary and objectives s…
Description check		The description covers the main template sections: Summary, Target Branch, Validation, and proof of fix. It explains the problem, the fix, and the tests run. It does not fill in the Related Issues sec…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Tests For Production Code		The PR changes production code in three src/ files. It adds two test suites under tests/unit/api/ with 757 new lines. The suites exercise model-sync locality, denial headers, fallback behavior, and CL…
No Ai Attribution		No prohibited attribution appears. All three commits in the reviewed range have no Co-Authored-By: trailer or Generated with <AI tool> footer. The authored PR body also contains neither marker.

Full details: Title check

Explanation

The title follows the Conventional Commits format with type "fix" and scope "providers". It names the main change: preserving caller locality during Cursor model sync. The raw summary and objectives show the changeset adds locality and local-spawn denial checks to the Cursor model sync routes and helpers.

Full details: Description check

Explanation

The description covers the main template sections: Summary, Target Branch, Validation, and proof of fix. It explains the problem, the fix, and the tests run. It does not fill in the Related Issues section, which lists no issue reference (only a base-red tracker note mentioning #17219). It also omits the Tests Added Or Updated list, the Coverage Notes, and the Reviewer Notes. The Target Branch note names release/v3.8.53, but the template asks for the active release/v3.8.x branch. The author also states that full lint and the production build were not run, which bears on checklist items the template lists. These gaps are non-critical, so the description is mostly complete.

Resolution

Optional improvements: list the Related Issues (or state none), list the changed test files under Tests Added Or Updated, add Coverage Notes for the src/ changes, and add Reviewer Notes covering the header-based local-spawn denial and the unrun full lint and production build.

Full details: Tests For Production Code

Explanation

The PR changes production code in three src/ files. It adds two test suites under tests/unit/api/ with 757 new lines. The suites exercise model-sync locality, denial headers, fallback behavior, and CLI-spawn gating, so the required tests were added.

Full details: Docstring Coverage

Explanation

Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 14 functions across 5 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

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

Usage-based review receipt

• Mode: Continue automatically
• Reviewed files: 6
• Waived: $1.50 (charged $0.00)
• View usage details

Note

This review exceeded your plan’s limits and used usage-based reviews—free during trial, billed after paid activation unless disabled. Manage usage-based reviews.

Recent review info

Run configuration

• Source mode: Auto
• Repository: cheaper-inference/OmniRoute
• Branch: release/v3.8.53
• File: .coderabbit.yaml
• Commit: f000c11
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: d6b73748-3d19-414c-955a-1adef45802ca

Commits

Reviewing files that changed from the base of the PR and between f000c11 and d22f93e.

Files selected for processing: 6

• changelog.d/fixes/0-cursor-sync-locality.md
• src/app/api/providers/[id]/models/route.ts
• src/app/api/providers/[id]/sync-models/route.ts
• src/shared/services/modelSyncScheduler.ts
• tests/unit/api/cursor-model-sync-denial.spec.ts
• tests/unit/api/cursor-model-sync-locality.spec.ts

Limit details: You’ve used all 10 included reviews currently available.