Add Cursor provider sessions and model selection#1355
Add Cursor provider sessions and model selection#1355juliusmarminge wants to merge 38 commits intomainfrom
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Merge never clears cached provider model options
- Replaced
??fallback withkey in incomingcheck so explicitly-present-but-undefined provider keys now clear cached values, and added cache deletion when merge produces undefined.
- Replaced
- ✅ Fixed: Mock agent test uses strict equal with extra fields
- Changed
toEqualtotoMatchObjectso the assertion tolerates the extramodesfield returned by the mock agent.
- Changed
Or push these changes by commenting:
@cursor push beb68c40d7
Preview (beb68c40d7)
diff --git a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
--- a/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
+++ b/apps/server/src/orchestration/Layers/ProviderCommandReactor.ts
@@ -50,20 +50,17 @@
cached: ProviderModelOptions | undefined,
incoming: ProviderModelOptions | undefined,
): ProviderModelOptions | undefined {
- if (!cached && !incoming) {
- return undefined;
+ if (incoming === undefined) return cached;
+ if (cached === undefined) return incoming;
+
+ const providerKeys = ["codex", "claudeAgent", "cursor"] as const;
+ const next: Record<string, unknown> = {};
+ for (const key of providerKeys) {
+ const value = key in incoming ? incoming[key] : cached[key];
+ if (value !== undefined) {
+ next[key] = value;
+ }
}
- const next = {
- ...(incoming?.codex !== undefined || cached?.codex !== undefined
- ? { codex: incoming?.codex ?? cached?.codex }
- : {}),
- ...(incoming?.claudeAgent !== undefined || cached?.claudeAgent !== undefined
- ? { claudeAgent: incoming?.claudeAgent ?? cached?.claudeAgent }
- : {}),
- ...(incoming?.cursor !== undefined || cached?.cursor !== undefined
- ? { cursor: incoming?.cursor ?? cached?.cursor }
- : {}),
- } satisfies Partial<ProviderModelOptions>;
return Object.keys(next).length > 0 ? (next as ProviderModelOptions) : undefined;
}
@@ -405,8 +402,12 @@
threadModelOptions.get(input.threadId),
input.modelOptions,
);
- if (mergedModelOptions !== undefined) {
- threadModelOptions.set(input.threadId, mergedModelOptions);
+ if (input.modelOptions !== undefined) {
+ if (mergedModelOptions !== undefined) {
+ threadModelOptions.set(input.threadId, mergedModelOptions);
+ } else {
+ threadModelOptions.delete(input.threadId);
+ }
}
const normalizedInput = toNonEmptyProviderInput(input.messageText);
const normalizedAttachments = input.attachments ?? [];
diff --git a/apps/server/src/provider/acp/AcpJsonRpcConnection.test.ts b/apps/server/src/provider/acp/AcpJsonRpcConnection.test.ts
--- a/apps/server/src/provider/acp/AcpJsonRpcConnection.test.ts
+++ b/apps/server/src/provider/acp/AcpJsonRpcConnection.test.ts
@@ -32,7 +32,7 @@
cwd: process.cwd(),
mcpServers: [],
});
- expect(newResult).toEqual({ sessionId: "mock-session-1" });
+ expect(newResult).toMatchObject({ sessionId: "mock-session-1" });
const promptResult = yield* conn.request("session/prompt", {
sessionId: "mock-session-1",| : {}), | ||
| } satisfies Partial<ProviderModelOptions>; | ||
| return Object.keys(next).length > 0 ? (next as ProviderModelOptions) : undefined; | ||
| } |
There was a problem hiding this comment.
Merge never clears cached provider model options
Medium Severity
mergeThreadModelOptions uses incoming?.codex ?? cached?.codex for each provider key. The ?? operator treats both undefined and the absence of a key identically, so there's no way for a turn to clear a previously-cached provider's model options back to undefined. Once a provider's options are set (e.g., cursor: { fastMode: true }), subsequent turns that omit that provider key will always inherit the stale cached value. This matters because mergedModelOptions is forwarded to providerService.sendTurn, so traits like fastMode become permanently sticky at the orchestration layer even when the user explicitly removes them.
| cwd: process.cwd(), | ||
| mcpServers: [], | ||
| }); | ||
| expect(newResult).toEqual({ sessionId: "mock-session-1" }); |
There was a problem hiding this comment.
Mock agent test uses strict equal with extra fields
Low Severity
The test asserts expect(newResult).toEqual({ sessionId: "mock-session-1" }) but the mock agent's session/new handler returns { sessionId, modes: modeState() } which includes an additional modes property. toEqual performs a deep strict equality check, so this assertion will fail. It likely needs toMatchObject instead.
- Introduce Cursor ACP adapter and model selection probe - Preserve cursor session resume state across model changes - Propagate provider and runtime tool metadata through orchestration and UI Made-with: Cursor
Replace the hardcoded client-side CURSOR_MODEL_CAPABILITY_BY_FAMILY map with server-provided ModelCapabilities, matching the Codex/Claude pattern. - Add CursorProvider snapshot service with BUILT_IN_MODELS and per-model capabilities; register it in ProviderRegistry alongside Codex/Claude. - Delete CursorTraitsPicker and route Cursor through the generic TraitsPicker, adding cursor support for the reasoning/effort key. - Add normalizeCursorModelOptionsWithCapabilities to providerModels. Made-with: Cursor
12f825a to
10129ed
Compare
apps/web/src/components/chat/CompactComposerControlsMenu.browser.tsx
Outdated
Show resolved
Hide resolved
| const defaultCursorReasoning = | ||
| DEFAULT_REASONING_EFFORT_BY_PROVIDER.cursor as CursorReasoningOption; | ||
|
|
||
| const cursor: CursorModelOptions | undefined = |
There was a problem hiding this comment.
🟢 Low src/composerDraftStore.ts:511
When cursorCandidate is non-null but contains no valid options, cursor becomes {} instead of undefined. The null check on line 523 (cursor === undefined) fails for empty objects, causing normalizeProviderModelOptions to return { cursor: {} } instead of null. This is inconsistent with codex and claude, which return undefined when no options exist. Consider checking Object.keys(cursor).length > 0 or using a definedness flag to ensure empty objects are treated as absent.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/composerDraftStore.ts around line 511:
When `cursorCandidate` is non-null but contains no valid options, `cursor` becomes `{}` instead of `undefined`. The null check on line 523 (`cursor === undefined`) fails for empty objects, causing `normalizeProviderModelOptions` to return `{ cursor: {} }` instead of `null`. This is inconsistent with `codex` and `claude`, which return `undefined` when no options exist. Consider checking `Object.keys(cursor).length > 0` or using a definedness flag to ensure empty objects are treated as absent.
Evidence trail:
apps/web/src/composerDraftStore.ts lines 459-466 (codex pattern), lines 486-492 (claude pattern), lines 511-521 (cursor pattern), lines 522-528 (return logic). Verified at commit REVIEWED_COMMIT.
…tion Instead of restarting the ACP process when the model changes mid-thread, use session/set_config_option to switch models within a live session. Update sessionModelSwitch to "in-session" and add probe tests to verify the real agent supports this method. Made-with: Cursor
Made-with: Cursor # Conflicts: # apps/web/src/components/chat/CompactComposerControlsMenu.browser.tsx # apps/web/src/components/chat/ProviderModelPicker.browser.tsx # apps/web/src/components/chat/ProviderModelPicker.tsx # apps/web/src/components/chat/TraitsPicker.tsx # apps/web/src/components/chat/composerProviderRegistry.test.tsx # apps/web/src/composerDraftStore.ts # packages/contracts/src/model.ts # packages/shared/src/model.test.ts # packages/shared/src/model.ts
apps/web/src/components/chat/CompactComposerControlsMenu.browser.tsx
Outdated
Show resolved
Hide resolved
| (CURSOR_REASONING_OPTIONS as readonly string[]).includes(cursorReasoningRaw) | ||
| ? (cursorReasoningRaw as CursorReasoningOption) | ||
| : undefined; | ||
| const cursorFastMode = cursorCandidate?.fastMode === true; |
There was a problem hiding this comment.
🟢 Low src/composerDraftStore.ts:509
cursorFastMode is set via cursorCandidate?.fastMode === true, so explicit fastMode: false values are silently discarded instead of being preserved. Contrast with codexFastMode and claudeFastMode which preserve both true and false via ternary chains. Consider using the same ternary pattern for cursorFastMode so that false values are retained.
| const cursorFastMode = cursorCandidate?.fastMode === true; | |
| const cursorFastMode = | |
| cursorCandidate?.fastMode === true | |
| ? true | |
| : cursorCandidate?.fastMode === false | |
| ? false | |
| : undefined; |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/composerDraftStore.ts around line 509:
`cursorFastMode` is set via `cursorCandidate?.fastMode === true`, so explicit `fastMode: false` values are silently discarded instead of being preserved. Contrast with `codexFastMode` and `claudeFastMode` which preserve both `true` and `false` via ternary chains. Consider using the same ternary pattern for `cursorFastMode` so that `false` values are retained.
Evidence trail:
apps/web/src/composerDraftStore.ts lines 509 (cursorFastMode definition), 525 (usage), 449-457 (codexFastMode ternary chain), 462 (codexFastMode usage), 480-485 (claudeFastMode ternary chain), 498 (claudeFastMode usage) at REVIEWED_COMMIT
- Removed unused CursorModelOptions and related logic from ChatView. - Updated model selection handling to map concrete Cursor slugs to server-provided options. - Simplified ProviderModelPicker by eliminating unnecessary cursor-related state and logic. - Adjusted tests to reflect changes in model selection behavior for Cursor provider. Made-with: Cursor
apps/web/src/components/chat/CompactComposerControlsMenu.browser.tsx
Outdated
Show resolved
Hide resolved
- Add a standalone ACP probe script for initialize/auth/session/new - Switch Cursor provider status checks to `agent about` for version and auth - Log the ACP session/new result in the probe test
- Canonicalize Claude and Cursor dispatch model slugs - Update provider model selection, defaults, and tests
- route Cursor commit/PR/branch generation through the agent CLI - resolve separate ACP and agent model IDs for Cursor models - improve git action failure logging and surface command output
| }); | ||
| }); | ||
|
|
||
| const startSession: CursorAdapterShape["startSession"] = (input) => |
There was a problem hiding this comment.
🟡 Medium Layers/CursorAdapter.ts:310
In sendTurn, if promptParts is empty and validation fails at line 712, the function returns an error after already mutating ctx.activeTurnId, updating ctx.session, and emitting a turn.started event. This leaves the session with an orphaned activeTurnId and a dangling turn.started event with no matching completion event. Consider moving the validation before any state mutations and event emissions.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/provider/Layers/CursorAdapter.ts around line 310:
In `sendTurn`, if `promptParts` is empty and validation fails at line 712, the function returns an error after already mutating `ctx.activeTurnId`, updating `ctx.session`, and emitting a `turn.started` event. This leaves the session with an orphaned `activeTurnId` and a dangling `turn.started` event with no matching completion event. Consider moving the validation before any state mutations and event emissions.
Evidence trail:
apps/server/src/provider/Layers/CursorAdapter.ts lines 624-720 at REVIEWED_COMMIT:
- Line 642-648: State mutations (`ctx.activeTurnId = turnId`, `ctx.session = ...`)
- Line 668-677: `turn.started` event emitted via `offerRuntimeEvent`
- Line 679-709: `promptParts` array built
- Line 711-717: Validation check `if (promptParts.length === 0)` returns error
- Line 737-746: `turn.completed` event only emitted after successful prompt
The validation check at line 711 happens AFTER state mutations (line 642-648) and event emission (line 668-677), confirming the claim.
| @@ -3018,6 +3025,14 @@ export function fromJsonSchemaMultiDocument(document: JsonSchema.MultiDocument<" | ||
| return js.allOf.reduce((acc, curr) => combine(acc, recur(curr)), out) | ||
| } | ||
|
|
||
| + if (hasAnyOf) { | ||
| + out = combine({ _tag: "Union", types: js.anyOf.map((type) => recur(type)), mode: "anyOf" }, out) | ||
| + } | ||
| + | ||
| + if (hasOneOf) { | ||
| + out = combine({ _tag: "Union", types: js.oneOf.map((type) => recur(type)), mode: "oneOf" }, out) | ||
| + } | ||
| + | ||
| return out |
There was a problem hiding this comment.
🟢 Low patches/effect@4.0.0-beta.41.patch:21
When a JSON Schema contains both allOf and anyOf/oneOf, the anyOf/oneOf constraints are silently dropped. At lines 21-22, the allOf handling returns early with return js.allOf.reduce(...), so the new anyOf/oneOf handling at lines 25-31 never executes. Consider restructuring so allOf, anyOf, and oneOf are all processed before returning.
if (Array.isArray(js.allOf)) {
- return js.allOf.reduce((acc, curr) => combine(acc, recur(curr)), out)
+ out = js.allOf.reduce((acc, curr) => combine(acc, recur(curr)), out)
}
if (hasAnyOf) {
- out = combine({ _tag: "Union", types: js.anyOf.map((type) => recur(type)), mode: "anyOf" }, out)
+ out = combine(out, { _tag: "Union", types: js.anyOf.map((type) => recur(type)), mode: "anyOf" })
}
if (hasOneOf) {
- out = combine({ _tag: "Union", types: js.oneOf.map((type) => recur(type)), mode: "oneOf" }, out)
+ out = combine(out, { _tag: "Union", types: js.oneOf.map((type) => recur(type)), mode: "oneOf" })
}
return out🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file patches/effect@4.0.0-beta.41.patch around lines 21-33:
When a JSON Schema contains both `allOf` and `anyOf`/`oneOf`, the `anyOf`/`oneOf` constraints are silently dropped. At lines 21-22, the `allOf` handling returns early with `return js.allOf.reduce(...)`, so the new `anyOf`/`oneOf` handling at lines 25-31 never executes. Consider restructuring so `allOf`, `anyOf`, and `oneOf` are all processed before returning.
- add request and protocol logging for ACP sessions - validate session config values before sending them - keep provider turn-start failures on the thread session - add coverage for ACP runtime and orchestration errors Co-authored-by: codex <codex@users.noreply.github.com>
| switch (upd.sessionUpdate) { | ||
| case "current_mode_update": { | ||
| modeId = upd.currentModeId; | ||
| events.push({ | ||
| _tag: "ModeChanged", | ||
| modeId, | ||
| }); | ||
| break; | ||
| } |
There was a problem hiding this comment.
🟡 Medium acp/AcpRuntimeModel.ts:406
In parseSessionUpdateEvent, the current_mode_update case assigns modeId directly from upd.currentModeId without trimming whitespace. This causes a mismatch when the server sends a padded mode ID like " code " — the returned modeId won't match the trimmed IDs stored in the session state from parseSessionModeState, so mode state updates may fail. Consider trimming the value: modeId = upd.currentModeId.trim();
case "current_mode_update": {
- modeId = upd.currentModeId;
+ modeId = upd.currentModeId.trim();
events.push({🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/provider/acp/AcpRuntimeModel.ts around lines 406-414:
In `parseSessionUpdateEvent`, the `current_mode_update` case assigns `modeId` directly from `upd.currentModeId` without trimming whitespace. This causes a mismatch when the server sends a padded mode ID like `" code "` — the returned `modeId` won't match the trimmed IDs stored in the session state from `parseSessionModeState`, so mode state updates may fail. Consider trimming the value: `modeId = upd.currentModeId.trim();`
Evidence trail:
apps/server/src/provider/acp/AcpRuntimeModel.ts lines 406-414: `parseSessionUpdateEvent` - `modeId = upd.currentModeId;` without trim (line 408)
apps/server/src/provider/acp/AcpRuntimeModel.ts lines 114-141: `parseSessionModeState` - `modes.currentModeId.trim()` (line 119) and `mode.id.trim()` (line 124)
| <Tooltip> | ||
| <TooltipTrigger | ||
| className="block min-w-0 w-full text-left" | ||
| title={displayText} | ||
| aria-label={displayText} | ||
| > |
There was a problem hiding this comment.
🟢 Low chat/MessagesTimeline.tsx:884
The title={displayText} attribute on TooltipTrigger creates a native browser tooltip that appears alongside the custom TooltipPopup, causing duplicate overlapping tooltips on hover. Since TooltipPopup handles the tooltip functionality and aria-label is already provided for accessibility, the title attribute duplicates functionality and should be removed.
- <TooltipTrigger
- className="block min-w-0 w-full text-left"
- title={displayText}
- aria-label={displayText}
- >🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/components/chat/MessagesTimeline.tsx around lines 884-889:
The `title={displayText}` attribute on `TooltipTrigger` creates a native browser tooltip that appears alongside the custom `TooltipPopup`, causing duplicate overlapping tooltips on hover. Since `TooltipPopup` handles the tooltip functionality and `aria-label` is already provided for accessibility, the `title` attribute duplicates functionality and should be removed.
Evidence trail:
apps/web/src/components/chat/MessagesTimeline.tsx lines 885-904 at REVIEWED_COMMIT: Shows `TooltipTrigger` with `title={displayText}` (line 888), `aria-label={displayText}` (line 889), and `TooltipPopup` component (lines 902-904). The `title` attribute is standard HTML that triggers native browser tooltips, while `TooltipPopup` provides custom tooltip functionality - both would display simultaneously on hover.
- propagate session cancel into pending ACP approvals - emit cancelled turn state when interruptions resolve as cancelled - stop adding default ACP config traits for built-in Cursor models
- move ACP runtime event mapping into shared helpers - keep unrecognized Cursor ACP model slugs unchanged
- derive ACP adapter raw sources from shared runtime event sources - keep canonical request mapping typed consistently
| ); | ||
| }; | ||
|
|
||
| const handleRequestEncoded = (message: RpcMessage.RequestEncoded) => { |
There was a problem hiding this comment.
🟡 Medium src/protocol.ts:218
Malformed session/update or session/elicitation_complete notification payloads terminate the entire protocol connection instead of being logged and skipped. When decodeSessionUpdate (line 221) or decodeElicitationComplete (line 241) fails, the AcpProtocolParseError propagates through Effect.forEach (line 373), causing Stream.runForEach (line 336) to fail and trigger the Effect.catch block (line 379) that ends stdin processing. Extension notifications (lines 260-264) are dispatched without validation and survive malformed payloads, but these known notification types tear down the connection due to a single bad message. Consider catching and logging the parse error for known notification types instead of letting them propagate, similar to how Effect.tapErrorTag (line 362) handles logging for the general case.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/effect-acp/src/protocol.ts around line 218:
Malformed `session/update` or `session/elicitation_complete` notification payloads terminate the entire protocol connection instead of being logged and skipped. When `decodeSessionUpdate` (line 221) or `decodeElicitationComplete` (line 241) fails, the `AcpProtocolParseError` propagates through `Effect.forEach` (line 373), causing `Stream.runForEach` (line 336) to fail and trigger the `Effect.catch` block (line 379) that ends stdin processing. Extension notifications (lines 260-264) are dispatched without validation and survive malformed payloads, but these known notification types tear down the connection due to a single bad message. Consider catching and logging the parse error for known notification types instead of letting them propagate, similar to how `Effect.tapErrorTag` (line 362) handles logging for the general case.
Evidence trail:
packages/effect-acp/src/protocol.ts lines 211-255 (handleRequestEncoded - session_update decode at 211-232 with Effect.mapError creating AcpProtocolParseError, elicitation_complete at 234-250 similarly, extension notifications at 251-255 with no validation), line 373 (Effect.forEach with routeDecodedMessage that propagates errors), lines 336-378 (Stream.runForEach on stdin with Effect.tapErrorTag that only logs but doesn't catch), lines 379-393 (Effect.catch that posts ClientProtocolError ending processing)
Co-authored-by: codex <codex@users.noreply.github.com>
- Cancel pending approvals when stopping a Cursor session - Resolve pending user-input waits so sendTurn can exit cleanly - Add regression coverage for both stop paths
- Emit decoded and raw protocol log events for notifications - Cover logOutgoing behavior with a protocol test
- Drop the `./child-process` subpath export - Stop building `src/child-process.ts` in package scripts
- Encode outgoing messages through ACP request envelopes - Add coverage for notification encoding failures
- Thread child-process exit codes into protocol shutdown - Distinguish stream end, decode failure, and process exit errors - Add protocol coverage for pending requests and late responses
| processExit: handle.exitCode.pipe( | ||
| Effect.map(Number), | ||
| Effect.mapError( | ||
| (cause) => | ||
| new AcpError.AcpProcessExitedError({ | ||
| cause, | ||
| }), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
🟡 Medium src/client.ts:287
Effect.map(Number) converts null to 0, so processes killed by signal (which report null exit code) are incorrectly reported as exiting cleanly with code 0. handleTermination in protocol.ts explicitly distinguishes null from 0 — null creates AcpProcessExitedError({}) while 0 creates AcpProcessExitedError({ code: 0 }). Consider preserving the null value so signal-terminated processes are correctly identified.
- processExit: handle.exitCode.pipe(
- Effect.map(Number),
- Effect.mapError(
+ processExit: handle.exitCode.pipe(
+ Effect.mapError(🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/effect-acp/src/client.ts around lines 287-295:
`Effect.map(Number)` converts `null` to `0`, so processes killed by signal (which report `null` exit code) are incorrectly reported as exiting cleanly with code 0. `handleTermination` in `protocol.ts` explicitly distinguishes `null` from `0` — `null` creates `AcpProcessExitedError({})` while `0` creates `AcpProcessExitedError({ code: 0 })`. Consider preserving the `null` value so signal-terminated processes are correctly identified.
Evidence trail:
- packages/effect-acp/src/client.ts lines 287-288 (commit REVIEWED_COMMIT): `processExit: handle.exitCode.pipe(Effect.map(Number), ...)` converts exit code to Number
- packages/effect-acp/src/protocol.ts line 45: `processExit?: Effect.Effect<number | null, AcpError.AcpProcessExitedError>` explicitly supports `null`
- packages/effect-acp/src/protocol.ts lines 452-456: explicit `code === null` check creates different error objects for `null` vs `0`
- JavaScript behavior: `Number(null) === 0`



Summary
Testing
bun fmtbun lintbun typecheckbun run testNote
Medium Risk
Introduces a new provider adapter and session lifecycle (spawned CLI process + JSON-RPC) and changes orchestration routing/session restart behavior, which can affect turn execution and session persistence across providers.
Overview
Adds Cursor as a first-class provider end-to-end: a new ACP JSON-RPC transport (
AcpJsonRpcConnection) plusCursorAdapterLivethat spawnsagent acp, managessession/new|load|prompt, maps ACP updates/tool calls/approvals into runtime events, and restarts sessions when the effective Cursor model changes.Updates orchestration to persist
thread.provider, route turns by explicit provider even for shared model slugs, treat Cursor model switching as restart-session, preserveresumeCursoracross Cursor restarts, and merge per-threadmodelOptionsacross turns.Extends runtime ingestion to include structured
dataontool.completedactivities, wires Cursor into the adapter registry, server layers, session directory persistence, and provider health checks.Updates the web app to support Cursor settings and UX: adds
customCursorModels, Cursor model-family picker + trait controls (reasoning/fast/thinking/opus tier), avoids redundant tool entry previews, and persists a sticky provider/model selection when creating new threads.Written by Cursor Bugbot for commit 12f825a. This will update automatically on new commits. Configure here.
Note
Add Cursor provider sessions and model selection across client and server
CursorAdapterLivespawns the Cursor agent via ACP JSON-RPC, maps ACP errors to provider adapter errors, and is registered in the defaultProviderAdapterRegistry.AcpJsonRpcConnection— a new JSON-RPC framing layer over a child process with serialized writes, in-flight request tracking, and a notifications stream.CursorModelOptionsschema (reasoning, fastMode, thinking, claudeOpusTier), and model slug/alias resolution inpackages/contractsandpackages/shared.stickyProviderpersistence and Cursor model options normalization; theProviderCommandReactornow routes turns by explicit provider and merges per-providermodelOptionsacross turns.CursorTraitsMenuContentandCursorTraitsPickerUI components for selecting Opus tier, reasoning level, and fast mode; theProviderModelPickernow displays and dispatches Cursor family slugs.checkCursorProviderStatus.resumeCursoris now preserved when only the model changes and cleared only on provider change.📊 Macroscope summarized 12f825a. 42 files reviewed, 14 issues evaluated, 7 issues filtered, 3 comments posted
🗂️ Filtered Issues
apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.test.ts — 0 comments posted, 2 evaluated, 2 filtered
projector.tschange unconditionally setsprovider: payload.providerwhen constructing theOrchestrationThreadobject forthread.createdevents. For events persisted before this change (or events where thethread.createcommand did not includeprovider),payload.providerwill beundefined. The decider indecider.tsconditionally spreadsproviderinto the event payload (...(command.provider !== undefined ? { provider: command.provider } : {})), meaning the key is intentionally absent when not specified. However, the projector now always injects the key with a potentiallyundefinedvalue. If theOrchestrationThreadschema definesprovideras anoptionalKey(key can be absent but must be a string when present) rather thanoptional(which also acceptsundefined), this will cause adecodeForEventfailure during event replay at startup for any existingthread.createdevents that lack aproviderfield. [ Out of scope (triage) ]"preserves completed tool metadata on projected tool activities"within the samedescribe("ProviderRuntimeIngestion", ...)block. The diff inserts a complete copy of the test (lines 682–737 after patch) immediately before the original test that already existed. Both tests have the same name, emit the same events, and assert the same expectations. While vitest will run both without crashing, this is an unintentional duplication that bloats test execution time and will cause confusion when one fails. [ Failed validation ]apps/server/src/provider/Layers/CursorAdapter.test.ts — 0 comments posted, 2 evaluated, 1 filtered
process.env.T3_ACP_EMIT_TOOL_CALLSis set at line 281, but theEffect.ensuringcleanup (lines 405-415) only wrapsprogram, not the setup code at lines 283-313. If the adapter lookup,Deferred.make, orStream.runForEach(...).pipe(Effect.forkChild)fails beforeprogramruns, the environment variable will not be restored. This could pollute subsequent tests with the modifiedT3_ACP_EMIT_TOOL_CALLSvalue. [ Cross-file consolidated ]apps/server/src/provider/Layers/CursorAdapter.ts — 1 comment posted, 4 evaluated, 2 filtered
aliasis only lowercased but not normalized (special characters not replaced with spaces), whilenormalizeModeSearchText(mode)replaces all non-alphanumeric characters with spaces. This asymmetry means an alias like"plan-mode"would fail to match a mode whose normalized text contains"plan mode"because"plan mode".includes("plan-mode")isfalse. If any of the alias constants (e.g.,ACP_PLAN_MODE_ALIASES) contain hyphens or other special characters, partial matches will silently fail. [ Out of scope (triage) ]parseSessionUpdate, whenparams.updateis a record containingmodeId/currentModeIdbutsessionUpdateis undefined andcontentis a record without atextstring property, the parsedmodeIdis discarded at line 649. The function returns{}even thoughmodeIdwas successfully parsed at lines 561-566. This causes the caller'sif (p.modeId)check to fail, missing a mode state update. The finalreturn {}at line 649 should includemodeIdlike the other return paths do. [ Cross-file consolidated ]apps/web/src/composerDraftStore.ts — 0 comments posted, 1 evaluated, 1 filtered
cursorClaudeOpusTiervalue"high"is validated as valid (line 431-432) but never included in the output. Line 445 only includesclaudeOpusTierwhen it equals"max", meaning"high"is silently dropped. The condition should becursorClaudeOpusTier ? { claudeOpusTier: cursorClaudeOpusTier } : {}to preserve both valid values. [ Out of scope (triage) ]packages/shared/src/model.ts — 0 comments posted, 1 evaluated, 1 filtered
normalizeCursorModelOptions, the logic for persistingthinkingonly handlessel.thinking === false, but forclaude-4.6-sonnetthedefaultThinkingisfalse(line 111). If a user enables thinking for sonnet (sel.thinking === true), this non-default value is not included in the returned options. The condition at line 352 should compare against the model's specific default:sel.thinking !== cap.defaultThinkingrather than just checking forfalse. [ Cross-file consolidated ]