#498 feat: redesigned AI provider picker + hardened Cursor integration

[Anonymous]

Originally created by: adityaharishch

What

Three related pieces of work on the AI-provider layer: a redesigned picker shared by setup and Settings, a substantial hardening of how Meridian invokes cursor-agent, and a last-resort fallback so a permanently-broken agent CLI no longer costs us the summary.

The picker

Setup and Settings now render the same component, so they cannot drift. Three recommended providers (Claude Code, Codex, Cursor) plus bring-your-own-API-key; each opens a detail view that installs the CLI, tests the connection, and sets it as default. Copilot is dropped from the chooser but stays a valid stored value, so legacy users get a switch banner rather than being stranded.

Two behavioural fixes worth calling out:

• The hook no longer bulk-tests every installed CLI on mount. That was what produced a spurious "CONNECTION FAILED" on providers the user had never configured.
• Selecting a provider commits immediately instead of staging a Save. This reverses an earlier staged-Save fix, so the anti-bug invariant is preserved structurally instead: value reads from settings.llm_provider on disk, and the commit only mutates state on success. The two regression tests that pinned the old shape are rewritten against the new invariants.

In-app install runs the vendor installer through the user's login shell (the tray is a Finder-launched .app with the stripped launchd PATH, which has no npm/node). Cursor sign-in uses cursor-agent login with the browser enabled, so the user's subscription is used - no API key, nothing metered.

The Cursor hardening

cursor-agent is a coding agent, but Meridian only ever asks it for inference over untrusted text (coding-agent transcripts, screen OCR). Its defaults are wrong in both directions: too much capability, and a large irrelevant context. src/llm/cursor_cli.rs is now the single source of truth for invoking it.

This also closes a real gap: the summariser built its own argv, so it summarised untrusted transcripts with cursor-agent's default full write + shell tool access. It now inherits the same hardening as the provider backend.

Lever	Effect
--allowed-tools ""	no tools at all; 21,040 -> 9,970 tokens
sandbox HOME	<agent_skills> 190 entries -> 0
--workspace <empty>	stops Cursor injecting the user's ~/CLAUDE.md
--mode ask	read-only, server-enforced
--output-format json	real error / usage-limit detection
CURSOR_API_KEY stripped	subscription guaranteed, never metered

Net: ~21,000 -> ~3,400 input tokens per call, summary quality unchanged.

The non-obvious part of the sandbox: the four .cursor skill entries must be recreated empty and read-only, because cursor-agent self-provisions its built-in skills and plugin cache on startup and would otherwise just repopulate them. The sandbox never touches the user's files - it only creates entries under its own directory, and the real skill directories are simply not linked in.

Every degradation is driven by a detected cause, never a blind retry: unknown flag -> unhardened call, auth failure -> real HOME, unavailable model -> auto. A usage limit deliberately does not retry, because Cursor limits are account-level and a second call would burn quota to hit the same wall.

Self-ingest fix

cursor-agent has no --no-session-persistence, so every call writes a chat into the store the cursor-cli source ingests from. Only the summariser was fingerprinted, so every other AI process routed through Cursor was ingested as developer activity and re-summarised next tick. All Meridian-issued prompts now carry a marker the ingest guard drops.

Summariser fallback

A coding-agent transcript is still summarised by its own CLI - that agent holds the context and its subscription is already paid for. What changes is the outcome when that CLI is permanently unable to answer. Previously the row was simply left pending and dead-lettered after three drains; now it escalates once to the provider the user chose in Settings.

1. the session's own CLI          x2 attempts      (codex -> codex, cursor -> cursor-agent, ...)
2. rate limit?      -> STOP. back the source off, retry on a later tick
3. otherwise failed -> the global provider, ONCE
4. still nothing    -> row stays pending; dead-lettered after 3 drains

Step 2 is the load-bearing one. A RateLimited breaks out of the loop before the fallback code is reachable, so there is no path where a quota triggers a substitute - a quota refills, and is waited out. Only a Failed (crashed, not installed, signed out, subscription ended, unusable output) escalates, and only after every retry is spent.

Two decisions worth a reviewer's eye:

• When the global provider is the CLI that just failed (the normal case), fallback::try_summarise returns Ok(None) and spawns nothing. A third run of the same binary would fail identically.
• A fallback provider hitting its own rate limit is reported as Failed, not RateLimited. That flag drives the per-agent-source backoff, and the fallback is a different account - parking Codex's queue because Claude ran out of quota would punish the wrong subscription. The resolver's provider-level backoff already prevents redialling.

Fallback summaries persist as summary_source = "fallback:<provider>", so a substitute's work is never mistaken for the agent's own. Nothing downstream reads that column today, so the vocabulary change is additive.

Version pin

cursor.com/install is a rolling script with no version flag. Pinned to the build this was verified against, by rewriting version strings by pattern - a literal swap would silently no-op the day Cursor publishes a new build and hand the user "latest" again.

Verification

• fmt, clippy -D warnings, 707 Rust tests, tray check, UI build, 252 UI tests - all green
• Live smoke tests against a real Cursor subscription throughout; token counts and the empty <agent_skills> block verified by decoding the actual persisted request

[Anonymous]

Originally posted by: coderabbitai[bot]

[!IMPORTANT]

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7d25c0e7-e9cd-40f0-874d-5b48e9c67b37

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:
- [ ] 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

- [ ] Create PR with unit tests - [ ] Commit unit tests in branch `feat/ai-provider-picker-redesign`

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

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

[Anonymous]

Originally posted by: adityaharishch

Thanks - this was a genuinely good review. I verified every finding against the code rather than taking them on trust, and all eight were real. All are now fixed in 4bada8dc, 3168ed97, 7b0f59ab.

Two of them (#4, [#8]) were tests that couldn't fail, which is the worst kind of finding to receive and the most useful to get. I reproduced both before fixing them.

Blocking

#1 Windows build. Confirmed in the CI log - error[E0425] in the check step, and test / check tray never ran. Applied your fix. I could not cross-compile locally to prove it (no MSVC toolchain here), so this rests on the reasoning plus the next CI run.

#2 Stale model override in the wizard. Confirmed, including the Cursor consequence you traced - a non-empty cfg.model suppresses DEFAULT_MODEL, so the wizard path could move a user off the ZDR-eligible model. Fixed by sharing the field-building (providerChoiceFields in llm-providers.ts), not by copying the line, since the divergence is the finding. custom keeps its model - that one is a real per-endpoint setting rather than a leftover.

High

#3 Recovery not shared. You were right that this is the same drift one level up. The ladder now lives in cursor_cli::run_hardened, which takes the per-attempt closure; each call site supplies a CursorCallError impl so a rate limit still returns None and degrades nothing. Eight tests drive the ladder directly - healthy path takes one attempt, each lever drops on its own detected cause, a rate limit never degrades, and it terminates once the levers are spent.

#4 Invariants pinned on the compliant surface. Reproduced: I rewrote page.tsx back to { llm_provider: id } and the suite stayed green. Both test files now read app/setup/page.tsx, and with that mutation applied the two new assertions fail. model-picker now pins the rule on the shared builder both surfaces call.

Medium

#5 Optimistic state / superseded-write race. Fixed at the root: setup commits first and moves the UI on success, matching Settings. setProvider returns the promise (and Wiz is typed Promise<void>), so the shared detail view can render "Switching…" and surface a failure in the wizard too.

#6 temp_dir() inside %USERPROFILE%. Real. Added scratch_root(), which prefers %PUBLIC% when temp resolves under the home and warns if neither is outside it. Your suspicion about the test was right in mechanism - it re-derived the path and compared raw strings - so it now asserts on what the product computes, through a canonicalising is_under.

Related bug this surfaced: the sandbox set only HOME, but cursor-agent is a Node bundle and Node reads USERPROFILE on Windows - so the whole skills sandbox was a no-op there regardless of the path. Both are set now, with a test.

#7 Pin fails open. Agreed, and thanks for actually fetching the installer. install_provider now compares cursor-agent --version against the pin and warns on mismatch, naming the constant to update. Deliberately does not fail the install - an unpinned CLI still works, because the ladder degrades on an unknown flag.

#8 Tautological marker test. Reproduced exactly as you did. Prompt building is extracted to build_prompt, and the test asserts on that. Verified by mutation: removing the marker now fails. Also took your point that the contract is contains, not position - the assertion no longer over-specifies.

Minor

All four fixed: login stdout is now drained as it arrives and included in the timeout message (with the three copies of the output-tail trimming folded into one helper); cwd pinned to neutral_workspace(); the shared-heading claim made true by actually sharing it; stale model-UI comments corrected; and the brittle not.toContain('pending') now matches the identifier.

Verification

fmt, clippy -D warnings, 717 Rust tests (+13), tray check, UI build, 258 UI tests (+6).

One thing I'd flag: #1 and [#6] are the two I could not fully verify locally, since both are Windows-specific and I have no MSVC toolchain. [#1] is mechanical, but the %PUBLIC% path in [#6] is reasoned rather than measured - if you have a Windows box, that is the piece worth a real run.

[Anonymous]

Ticket changed by: adityaharishch

• status: open --> closed