Add chat thread font-size setting - #518
raphaelluethy wants to merge 91 commits into
Conversation
- Add per-provider model options, defaults, and slug aliases - Add provider-aware model normalization/resolution helpers - Preserve Codex-only constants/functions for backward compatibility - Extend tests to cover Claude aliases and provider-specific fallback behavior
- introduce `ClaudeCodeAdapter` service and live layer wiring - map runtime/session/request failures into provider adapter error types - add coverage for validation, session-not-found mapping, lifecycle forwarding, and event passthrough
Add provider-service routing coverage for explicit claudeCode sessions. Co-authored-by: codex <codex@users.noreply.github.com>
Add restart semantics when requested provider changes and cover with tests. Co-authored-by: codex <codex@users.noreply.github.com>
Add provider-aware model options and include provider in turn-start dispatch. Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
- add `@anthropic-ai/claude-agent-sdk` dependency for `apps/server` - replace placeholder Claude adapter with live session/query/event handling - add comprehensive adapter tests for runtime events, approvals, resume, rollback, and model overrides
- Replace manual prompt async iterator with `Stream.fromQueue(...).toAsyncIterable` - Use `Ref` for shared session context in tool approval callbacks - Simplify timestamp/ID generation and close sessions via queue shutdown
- Stop synthesizing `resume` from generated thread IDs - Persist resume session ID from query messages when available - Validate resume/sessionId values as UUIDs before reuse - Add tests for valid UUID resume passthrough and no synthesized resume
Co-authored-by: codex <codex@users.noreply.github.com>
- Do not pass `resumeCursor` when restarting a session after changing providers - Treat synthetic Claude thread IDs (`claude-thread-*`) as unscoped during runtime ingestion - Add tests covering provider-switch restart behavior and Claude turn lifecycle acceptance
Co-authored-by: codex <codex@users.noreply.github.com>
- Preserve active turn state when `session.started`/`thread.started` arrive mid-turn - Emit `message.delta` from assistant text when stream deltas are missing, then complete the message - Add Claude native SDK NDJSON observability logging and wire its log path in server layers - Expand ingestion/adapter tests to cover mid-turn lifecycle and delta fallback behavior
- Default Claude sessions to bypass permissions when approval policy is `never`, while preserving explicit `permissionMode` - Hide/send reasoning effort only for providers that support it in ChatView - Add coverage for Claude permission-mode derivation and precedence, and update runtime event model docs
- add per-session `sessionSequence` on provider runtime events and persist activity `sequence` - migrate `projection_thread_activities` with nullable `sequence` column + index - sort server/web activity projections by sequence fallback to timestamp/id - allow provider sessions/turns to start before a real threadId is emitted
- define `CursorAdapter` service contract and Cursor stream-json schema types - add `CursorCliStreamEvent` decoding tests for system/thinking/tool/result/retry events - add implementation plan for Cursor provider integration Co-authored-by: codex <codex@users.noreply.github.com>
- Accept `cursor` as a first-class provider in orchestration, persistence, and session directory flows - Update model/provider inference and normalization to handle Cursor model aliases - Revamp chat provider/model picker with Cursor-specific trait controls and add coverage in tests
- add `scripts/cursor-acp-probe.mjs` to run ACP protocol probing scenarios - update `package.json` scripts for probe execution - include generated probe summaries/transcripts under `.tmp/acp-probe/`
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
… codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
…hored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
- expose workspace target discovery/opening across contracts, server Open service, WS API, and web chat UI - add `thread.command-execution.terminate` flow through decider, reactor, and provider service with capability gating - enrich runtime tool activities with `runtimeItemId` and normalized command text; update adapters/tests for new capability surface
… strip Extract shared clipboard helper to replace direct navigator.clipboard calls across components. Add "Copy path" menu item to the Open picker dropdown. Move RunningCommandsStrip above the input bar.
Add GPT-5.4 (medium/high/xhigh) and Kimi K2.5 to Cursor model families, slug aliases, and capability maps.
Add spawnClaudeCodeProcess override that uses process.execPath with ELECTRON_RUN_AS_NODE=1 so the Claude Code SDK child process works inside packaged Electron apps where node is not in PATH.
Add support for capturing and enriching tool metadata throughout the tool lifecycle. Extract tool input data from assistant messages and include it in both tool.started and tool.completed activities. Enhance tool detail summaries to support file paths, patterns, and other common input formats. Improve tool title generation with human-readable names derived from tool identifiers.
- add small/medium/large chat font size option in chat settings - apply selected size across markdown, code blocks, and plain text messages - include reset-to-default support for the new setting
|
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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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. Comment |
| return aliases[trimmed] ?? (trimmed as ModelSlug); | ||
| } |
There was a problem hiding this comment.
src/model.ts:265
normalizeModelSlug returns inherited object properties like constructor or toString when trimmed matches those keys, instead of returning the trimmed string. The removed typeof check previously filtered these out, but the new ?? operator treats truthy functions/objects as valid return values. This violates the ModelSlug return type contract and will crash downstream code that expects a string.
const aliases = MODEL_SLUG_ALIASES_BY_PROVIDER[provider] as Record<string, ModelSlug>;
- return aliases[trimmed] ?? (trimmed as ModelSlug);
+ const aliased = aliases[trimmed];
+ return typeof aliased === "string" ? aliased : (trimmed as ModelSlug);🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file packages/shared/src/model.ts around lines 265-266:
`normalizeModelSlug` returns inherited object properties like `constructor` or `toString` when `trimmed` matches those keys, instead of returning the trimmed string. The removed `typeof` check previously filtered these out, but the new `??` operator treats truthy functions/objects as valid return values. This violates the `ModelSlug` return type contract and will crash downstream code that expects a string.
Evidence trail:
packages/shared/src/model.ts lines 264-265 at REVIEWED_COMMIT: `const aliases = MODEL_SLUG_ALIASES_BY_PROVIDER[provider] as Record<string, ModelSlug>; return aliases[trimmed] ?? (trimmed as ModelSlug);`
git_diff MERGE_BASE..REVIEWED_COMMIT path=packages/shared/src/model.ts shows old code: `const aliased = aliases[trimmed]; return typeof aliased === "string" ? aliased : (trimmed as ModelSlug);`
packages/contracts/src/model.ts lines 112-145: MODEL_SLUG_ALIASES_BY_PROVIDER is a plain object literal (inherits from Object.prototype)
packages/contracts/src/model.ts line 103: `export type ModelSlug = BuiltInModelSlug | (string & {});` (string type)
| const detail = asTrimmedString(payload.detail); | ||
| const command = asTrimmedString(payload.command) ?? detail; |
There was a problem hiding this comment.
🟡 Medium src/runningCommandExecutions.ts:72
When a tool.updated event omits the command field (common after the initial tool.started), the code sets command to asTrimmedString(payload.command) ?? detail. This overwrites the actual command name with the detail string (e.g., a log message), so the UI displays log output as the command title. Consider falling back to existing?.command before using detail.
- const command = asTrimmedString(payload.command) ?? detail;
+ const command = asTrimmedString(payload.command) ?? existing?.command ?? detail;🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/web/src/runningCommandExecutions.ts around lines 72-73:
When a `tool.updated` event omits the `command` field (common after the initial `tool.started`), the code sets `command` to `asTrimmedString(payload.command) ?? detail`. This overwrites the actual command name with the `detail` string (e.g., a log message), so the UI displays log output as the command title. Consider falling back to `existing?.command` before using `detail`.
Evidence trail:
apps/web/src/runningCommandExecutions.ts lines 70-84 at REVIEWED_COMMIT: Line 70 retrieves `existing = byItemId.get(itemId)`, line 72 sets `command = asTrimmedString(payload.command) ?? detail` with no fallback to `existing?.command`, lines 77-84 write the object with this potentially-overwritten command value. Compare to line 81 which correctly preserves `existing?.startedAt`.
| if (decision === "accept") { | ||
| return allowOnce?.optionId ?? allowAlways?.optionId; | ||
| } | ||
| return rejectOnce?.optionId ?? options[0]?.optionId; |
There was a problem hiding this comment.
🟡 Medium Layers/CursorAdapter.ts:220
When decision is "cancel" or "decline" and reject-once is not in options, line 220 returns options[0]?.optionId. If the first option is allow-always, the function grants permission despite the user declining. Consider returning undefined for cancel to prevent unintended grants, and handle decline similarly or ensure a reject option exists.
- return rejectOnce?.optionId ?? options[0]?.optionId;
+ if (decision === "decline") {
+ return rejectOnce?.optionId ?? options[0]?.optionId;
+ }
+ return undefined;🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/provider/Layers/CursorAdapter.ts around line 220:
When `decision` is `"cancel"` or `"decline"` and `reject-once` is not in `options`, line 220 returns `options[0]?.optionId`. If the first option is `allow-always`, the function grants permission despite the user declining. Consider returning `undefined` for `cancel` to prevent unintended grants, and handle `decline` similarly or ensure a reject option exists.
Evidence trail:
apps/server/src/provider/Layers/CursorAdapter.ts lines 206-222 at REVIEWED_COMMIT: Function `selectCursorPermissionOption` handles decisions. Lines 214-219 handle `acceptForSession` and `accept`. Line 220 handles `decline` and `cancel` with `return rejectOnce?.optionId ?? options[0]?.optionId;`. If `rejectOnce` is undefined and `options[0].optionId` is `allow-always`, the function returns `allow-always` when user declined/canceled.
| function classifyRequestType(toolName: string): CanonicalRequestType { | ||
| const normalized = toolName.toLowerCase(); | ||
| if (normalized === "read" || normalized.includes("read file") || normalized.includes("view")) { | ||
| return "file_read_approval"; |
There was a problem hiding this comment.
🟢 Low Layers/ClaudeCodeAdapter.ts:280
classifyRequestType checks normalized.includes("read file") which requires a space, so tool names like read_file or readFile fail the check. These are then classified via classifyToolItemType which matches the substring "file" and returns file_change, causing the system to request file_change_approval (write permission) instead of file_read_approval for read-only operations. Consider using a broader pattern that matches common naming conventions like read_file, readFile, or readfile.
- if (normalized === "read" || normalized.includes("read file") || normalized.includes("view")) {
+ if (
+ normalized === "read" ||
+ normalized === "read_file" ||
+ normalized === "readfile" ||
+ normalized.includes("read_file") ||
+ normalized.includes("readfile") ||
+ normalized.includes("read file") ||
+ normalized.includes("view")
+ ) {🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/provider/Layers/ClaudeCodeAdapter.ts around lines 280-283:
`classifyRequestType` checks `normalized.includes("read file")` which requires a space, so tool names like `read_file` or `readFile` fail the check. These are then classified via `classifyToolItemType` which matches the substring "file" and returns `file_change`, causing the system to request `file_change_approval` (write permission) instead of `file_read_approval` for read-only operations. Consider using a broader pattern that matches common naming conventions like `read_file`, `readFile`, or `readfile`.
Evidence trail:
apps/server/src/provider/Layers/ClaudeCodeAdapter.ts lines 280-287 (classifyRequestType function with `normalized.includes("read file")` check), lines 253-277 (classifyToolItemType function with `normalized.includes("file")` returning `file_change`). Commit: REVIEWED_COMMIT.
| const normalized = toMessage(cause, "").toLowerCase(); | ||
| if (normalized.includes("unknown session") || normalized.includes("not found")) { |
There was a problem hiding this comment.
🟢 Low Layers/ClaudeCodeAdapter.ts:497
The substring check "not found" in toSessionError matches error messages unrelated to sessions, such as "File not found" or "Command not found". This causes operational errors to be misclassified as ProviderAdapterSessionNotFoundError, potentially triggering incorrect session invalidation or termination when the session is actually healthy. Consider tightening the check to match only session-specific error patterns.
- if (normalized.includes("unknown session") || normalized.includes("not found")) {
+ if (normalized.includes("unknown session")) {Also found in 1 other location(s)
apps/server/src/provider/Layers/CursorAdapter.ts:130
The error mapping logic in
toSessionErroruses an overly broad substring checknormalized.includes("not found")to classify session errors. This causes common operational errors (e.g., "File not found", "Command not found") to be misclassified as aProviderAdapterSessionNotFoundError. This replaces the actual error message with a confusing "Unknown adapter thread" message and may trigger incorrect session teardown logic upstream.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/provider/Layers/ClaudeCodeAdapter.ts around lines 497-498:
The substring check `"not found"` in `toSessionError` matches error messages unrelated to sessions, such as "File not found" or "Command not found". This causes operational errors to be misclassified as `ProviderAdapterSessionNotFoundError`, potentially triggering incorrect session invalidation or termination when the session is actually healthy. Consider tightening the check to match only session-specific error patterns.
Evidence trail:
ClaudeCodeAdapter.ts lines 493-511 show the `toSessionError` function with the broad `normalized.includes("not found")` check at line 498. Compare to CodexAdapter.ts line 63 which uses the more specific `normalized.includes("unknown provider session")` instead. The `toRequestError` function at lines 515-529 shows how the session error classification propagates.
Also found in 1 other location(s):
- apps/server/src/provider/Layers/CursorAdapter.ts:130 -- The error mapping logic in `toSessionError` uses an overly broad substring check `normalized.includes("not found")` to classify session errors. This causes common operational errors (e.g., "File not found", "Command not found") to be misclassified as a `ProviderAdapterSessionNotFoundError`. This replaces the actual error message with a confusing "Unknown adapter thread" message and may trigger incorrect session teardown logic upstream.
| assistantItemId: asProviderItemId(yield* Random.nextUUIDv4), | ||
| startedToolCalls: new Set(), | ||
| toolCalls: new Map(), | ||
| items: [], |
There was a problem hiding this comment.
🟢 Low Layers/CursorAdapter.ts:1300
CursorTurnState.items is initialized as an empty array but never populated—content from agent_message_chunk, agent_thought_chunk, and tool_call events flows to runtime events without being accumulated. This causes readThread to return snapshots with empty items arrays, losing all message and tool call history. Consider pushing content to turnState.items in handleSessionUpdateNotification before emitting events.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/provider/Layers/CursorAdapter.ts around line 1300:
`CursorTurnState.items` is initialized as an empty array but never populated—content from `agent_message_chunk`, `agent_thought_chunk`, and `tool_call` events flows to runtime events without being accumulated. This causes `readThread` to return snapshots with empty `items` arrays, losing all message and tool call history. Consider pushing content to `turnState.items` in `handleSessionUpdateNotification` before emitting events.
Evidence trail:
- apps/server/src/provider/Layers/CursorAdapter.ts:70-76 - CursorTurnState interface definition with `items: Array<unknown>`
- apps/server/src/provider/Layers/CursorAdapter.ts:1295-1302 - turnState initialization with `items: []`
- apps/server/src/provider/Layers/CursorAdapter.ts:487 - only reference to `turnState.items` is spreading it: `items: [...turnState.items]`
- git_grep for `.items.push` returned no results
- apps/server/src/provider/Layers/CursorAdapter.ts:661-729 - `handleSessionUpdateNotification` emits events but never pushes to items
- apps/server/src/provider/Layers/CursorAdapter.ts:463-465 - copies empty items to context.turns
- apps/server/src/provider/Layers/CursorAdapter.ts:1400-1409 - readThread returns turns with (empty) items
| }); | ||
|
|
||
| this.#stdoutRl = readline.createInterface({ input: this.#child.stdout }); | ||
| this.#stderrRl = readline.createInterface({ input: this.#child.stderr }); | ||
|
|
||
| this.#stdoutRl.on("line", (line) => this.#handleStdoutLine(line)); | ||
| this.#stderrRl.on("line", (line) => { | ||
| this.#onMessage({ | ||
| ts: nowIso(), | ||
| channel: "stderr", | ||
| line, | ||
| }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
🟢 Low scripts/cursor-acp-probe.mjs:76
If the agent command doesn't exist or fails to spawn, Node.js emits an 'error' event on this.#child that is never handled. This throws an uncaught exception that crashes the process before the top-level handler at line 508 can run. Additionally, #closed stays false, so pending promises hang until timeout instead of failing immediately. Consider adding an 'error' handler that sets #closed and rejects pending promises.
this.#stdoutRl = readline.createInterface({ input: this.#child.stdout });
this.#stderrRl = readline.createInterface({ input: this.#child.stderr });
+ this.#child.on('error', (err) => {
+ this.#closed = true;
+ for (const [id, pending] of this.#pending.entries()) {
+ this.#pending.delete(id);
+ pending.reject(err);
+ }
+ });
+
this.#stdoutRl.on('line', (line) => this.#handleStdoutLine(line));🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file scripts/cursor-acp-probe.mjs around lines 76-89:
If the `agent` command doesn't exist or fails to spawn, Node.js emits an `'error'` event on `this.#child` that is never handled. This throws an uncaught exception that crashes the process before the top-level handler at line 508 can run. Additionally, `#closed` stays `false`, so pending promises hang until timeout instead of failing immediately. Consider adding an `'error'` handler that sets `#closed` and rejects pending promises.
Evidence trail:
scripts/cursor-acp-probe.mjs lines 60-107 (viewed REVIEWED_COMMIT): spawn at line 71, only 'exit' handler at line 91, no 'error' handler. git_grep for '.on\(['"]error' pattern returned no results. Lines 508-511 show top-level `.catch()` handler on `run()`. Node.js EventEmitter semantics: unhandled 'error' events throw uncaught exceptions.
Sorry, this should have gone to origin not upstream 🫠