Conversation
Replace silent failure when Claude credentials are missing with an inline auth prompt. Adds getAuthStatus query and setApiKey mutation to the ai-chat router, a new AuthPrompt component, and gates the ChatInputFooter on auth status so users can paste an API key directly. Also removes the durable-session package, streams app, and simplifies session management.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR removes the Streams app and the durable-session package, strips Streams-related env/CSP/CI/build wiring, deletes desktop session-manager agent/ and streaming modules, replaces ai-chat getConfig with auth endpoints, and adds an AuthPrompt with conditional UI rendering. Changes
Sequence Diagram(s)(Skipped — changes are broad removals and do not introduce a new multi-component sequential flow that benefits from a diagram.) Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/lib/trpc/routers/ai-chat/index.ts (1)
39-50:⚠️ Potential issue | 🟡 MinorModule-level credential resolution only supports OAuth, but
setApiKeyaccepts plain API keys.Lines 41-49 log a warning and skip non-OAuth credentials at startup. But
setApiKeyhappily sets a plain API key viasetAnthropicAuthToken. This inconsistency may confuse developers — clarify whether plain API keys are supported or not, and align module-level init with the mutation's behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts` around lines 39 - 50, The module-level credential resolution currently only accepts OAuth credentials (cliCredentials with kind === "oauth") but the setter setAnthropicAuthToken / mutation setApiKey supports plain API keys, creating an inconsistency; update the startup logic around getCredentialsFromConfig/getCredentialsFromKeychain and cliCredentials so that it also accepts plain API keys (e.g., cliCredentials.kind === "apiKey") and calls setAnthropicAuthToken(cliCredentials.apiKey) for both kinds, and adjust the console message to reflect whether OAuth or API key was used (remove the warning that ignores non-OAuth creds). Alternatively, if you prefer to disallow plain API keys, change the mutation setApiKey to validate/reject non-OAuth credentials and keep the startup warning — pick one approach and make the module-level init (cliCredentials check), the setAnthropicAuthToken call, and the log text consistent.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/hooks/useClaudeCodeHistory/useClaudeCodeHistory.ts (1)
28-33:⚠️ Potential issue | 🟡 MinorRemove unsafe
as UIMessage[]cast and properly convertClaudeSessionMessagetoUIMessage.The tRPC router returns
ClaudeSessionMessage[]with apartsfield structure, but line 29 casts it directly toUIMessage[]without validation or conversion. This suppresses type mismatches between the two shapes. When merging withliveMessages(trueUIMessage[]), structural incompatibilities could cause runtime issues. Either convert the messages explicitly using a hydration function or update the tRPC router to return the correctUIMessageshape.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/hooks/useClaudeCodeHistory/useClaudeCodeHistory.ts` around lines 28 - 33, The code in useClaudeCodeHistory constructs allMessages by casting claudeMessages to UIMessage[] unsafely; replace the cast with an explicit conversion from ClaudeSessionMessage to UIMessage (or change the tRPC return type) so shapes match before merging with liveMessages. Implement or call a hydration/mapper function (e.g., hydrateClaudeToUI or mapClaudeSessionMessage) to transform each ClaudeSessionMessage.parts into the UIMessage.content/author fields and validate required properties, then use the resulting UIMessage[] when building allMessages instead of the as UIMessage[] cast. Ensure the mapper is applied where claudeMessages is used (inside the useMemo for allMessages) so downstream code only sees proper UIMessage objects.
🧹 Nitpick comments (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/ChatInterface.tsx (1)
22-22: Auth query has no error/loading handling but the fallback is sensible.
getAuthStatus.useQuery()is called withoutenabled,retry, orrefetchIntervaloptions. WhileauthStatus?.authenticated === falsegracefully defaults to showingChatInputFooterduring loading/error (sinceundefined !== false), consider whether the auth status should be re-polled after the app regains focus or on an interval, since credentials could be added externally (e.g., viaclaude loginin a terminal).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/ChatInterface.tsx` at line 22, The auth query call electronTrpc.aiChat.getAuthStatus.useQuery currently has no query options; update it to handle background refetching so credentials added externally are picked up—pass options such as refetchOnWindowFocus: true (to refetch when app regains focus), and optionally refetchInterval (e.g., 60_000) and retry (e.g., 1) to control polling and retries; keep the existing fallback to show ChatInputFooter when authStatus is undefined/false.apps/desktop/src/lib/trpc/routers/ai-chat/index.ts (1)
304-320: Both branches ofsetApiKeyare identical — theisOauthcheck is dead logic.The
if (isOauth)andelsebranches both callsetAnthropicAuthToken(input.apiKey)— only the log message differs. Either collapse into a single path, or if different handling is intended for API keys vs OAuth tokens, implement the distinction.Suggested simplification
setApiKey: publicProcedure .input(z.object({ apiKey: z.string().min(1) })) .mutation(({ input }) => { - const isOauth = input.apiKey.startsWith("sk-ant-oat"); - if (isOauth) { - setAnthropicAuthToken(input.apiKey); - console.log( - "[ai-chat/setApiKey] Set OAuth token via setAnthropicAuthToken", - ); - } else { - setAnthropicAuthToken(input.apiKey); - console.log( - "[ai-chat/setApiKey] Set API key via setAnthropicAuthToken", - ); - } + const isOauth = input.apiKey.startsWith("sk-ant-oat"); + setAnthropicAuthToken(input.apiKey); + console.log( + `[ai-chat/setApiKey] Set ${isOauth ? "OAuth token" : "API key"} via setAnthropicAuthToken`, + ); return { success: true }; }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts` around lines 304 - 320, In setApiKey (publicProcedure in the ai-chat router) the isOauth check is dead: both branches call setAnthropicAuthToken(input.apiKey) and only differ by console.log. Replace the duplicated branches by a single call to setAnthropicAuthToken(input.apiKey) and then log once (either a generic message or a conditional log based on isOauth), or if OAuth tokens require different handling implement that distinct behavior instead of identical calls; update/remove the isOauth branching accordingly.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/AuthPrompt/AuthPrompt.tsx (1)
13-15: Clear the API key input on successful submission.After a successful mutation, the password field retains the key in component state. Consider clearing it for hygiene.
Suggested fix
onSuccess: () => { setError(null); + setApiKey(""); void utils.aiChat.getAuthStatus.invalidate(); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/AuthPrompt/AuthPrompt.tsx` around lines 13 - 15, In the AuthPrompt component's onSuccess handler (the function that currently calls setError(null) and void utils.aiChat.getAuthStatus.invalidate()), also clear the API key input state so the password field is emptied after a successful submission; add a call to the state setter that holds the input (e.g., setApiKey('') or setPassword('')) inside onSuccess in AuthPrompt so the input is reset when the mutation succeeds.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts`:
- Around line 288-302: getAuthStatus currently only checks
getExistingClaudeCredentials (config/keychain) so it stays false after setApiKey
stores the token only in-memory via setAnthropicAuthToken; update getAuthStatus
to also inspect the in-memory token state (call or import the same accessor that
exposes the in-memory token used by setAnthropicAuthToken) and return
authenticated:true when that token exists, or alternatively modify setApiKey to
persist the credential the same way getExistingClaudeCredentials expects
(config/keychain) so that getExistingClaudeCredentials will return a credential;
reference getAuthStatus, getExistingClaudeCredentials, setApiKey,
setAnthropicAuthToken and AuthPrompt when making the change.
In `@apps/desktop/src/lib/trpc/routers/ai-chat/utils/auth/auth.ts`:
- Around line 116-135: The code path that parses raw macOS Keychain JSON
(ClaudeConfigFile) silently ignores valid JSON that lacks
claudeAiOauth.accessToken; update the logic in the auth parsing block (the
JSON.parse branch in auth.ts) to reuse the same extraction logic used by
getCredentialsFromConfig (or explicitly check for other fields such as apiKey
and oauth_access_token) and return the appropriate credential object
(kind/source) when found, and if no known fields are present emit a clear
warning log mentioning the parsed JSON keys and that no usable credential was
found (instead of falling through silently); reference ClaudeConfigFile,
claudeAiOauth, and getCredentialsFromConfig to locate where to apply this
change.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/AuthPrompt/AuthPrompt.tsx`:
- Around line 12-20: The setApiKey backend mutation currently always returns
success so the front-end setApiKeyMutation onError never fires; update the
backend setApiKey implementation (the setApiKey function that calls
setAnthropicAuthToken) to perform a lightweight validation against Anthropic
(e.g., call a models/list or similar minimal endpoint) after calling
setAnthropicAuthToken and before returning { success: true }, and throw an error
if the validation fails so the frontend setApiKeyMutation.onError can surface
the invalid-key state and utils.aiChat.getAuthStatus.invalidate() remains
correct.
---
Outside diff comments:
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts`:
- Around line 39-50: The module-level credential resolution currently only
accepts OAuth credentials (cliCredentials with kind === "oauth") but the setter
setAnthropicAuthToken / mutation setApiKey supports plain API keys, creating an
inconsistency; update the startup logic around
getCredentialsFromConfig/getCredentialsFromKeychain and cliCredentials so that
it also accepts plain API keys (e.g., cliCredentials.kind === "apiKey") and
calls setAnthropicAuthToken(cliCredentials.apiKey) for both kinds, and adjust
the console message to reflect whether OAuth or API key was used (remove the
warning that ignores non-OAuth creds). Alternatively, if you prefer to disallow
plain API keys, change the mutation setApiKey to validate/reject non-OAuth
credentials and keep the startup warning — pick one approach and make the
module-level init (cliCredentials check), the setAnthropicAuthToken call, and
the log text consistent.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/hooks/useClaudeCodeHistory/useClaudeCodeHistory.ts`:
- Around line 28-33: The code in useClaudeCodeHistory constructs allMessages by
casting claudeMessages to UIMessage[] unsafely; replace the cast with an
explicit conversion from ClaudeSessionMessage to UIMessage (or change the tRPC
return type) so shapes match before merging with liveMessages. Implement or call
a hydration/mapper function (e.g., hydrateClaudeToUI or mapClaudeSessionMessage)
to transform each ClaudeSessionMessage.parts into the UIMessage.content/author
fields and validate required properties, then use the resulting UIMessage[] when
building allMessages instead of the as UIMessage[] cast. Ensure the mapper is
applied where claudeMessages is used (inside the useMemo for allMessages) so
downstream code only sees proper UIMessage objects.
---
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts`:
- Around line 304-320: In setApiKey (publicProcedure in the ai-chat router) the
isOauth check is dead: both branches call setAnthropicAuthToken(input.apiKey)
and only differ by console.log. Replace the duplicated branches by a single call
to setAnthropicAuthToken(input.apiKey) and then log once (either a generic
message or a conditional log based on isOauth), or if OAuth tokens require
different handling implement that distinct behavior instead of identical calls;
update/remove the isOauth branching accordingly.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/ChatInterface.tsx`:
- Line 22: The auth query call electronTrpc.aiChat.getAuthStatus.useQuery
currently has no query options; update it to handle background refetching so
credentials added externally are picked up—pass options such as
refetchOnWindowFocus: true (to refetch when app regains focus), and optionally
refetchInterval (e.g., 60_000) and retry (e.g., 1) to control polling and
retries; keep the existing fallback to show ChatInputFooter when authStatus is
undefined/false.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/AuthPrompt/AuthPrompt.tsx`:
- Around line 13-15: In the AuthPrompt component's onSuccess handler (the
function that currently calls setError(null) and void
utils.aiChat.getAuthStatus.invalidate()), also clear the API key input state so
the password field is emptied after a successful submission; add a call to the
state setter that holds the input (e.g., setApiKey('') or setPassword(''))
inside onSuccess in AuthPrompt so the input is reset when the mutation succeeds.
Comment on lines
+288
to
302
| getAuthStatus: publicProcedure.query(() => { | ||
| const creds = getExistingClaudeCredentials(); | ||
| if (creds) { | ||
| return { | ||
| authenticated: true as const, | ||
| source: creds.source, | ||
| kind: creds.kind, | ||
| }; | ||
| } | ||
| return { | ||
| proxyUrl: env.NEXT_PUBLIC_STREAMS_URL, | ||
| authToken: token, | ||
| authenticated: false as const, | ||
| source: null, | ||
| kind: null, | ||
| }; | ||
| }), |
Contributor
There was a problem hiding this comment.
getAuthStatus will return false even after a successful setApiKey call — auth prompt will never dismiss.
getAuthStatus calls getExistingClaudeCredentials(), which only reads from config files and the macOS Keychain. However, setApiKey (Lines 304-320) only stores the key in-memory via setAnthropicAuthToken. After the client invalidates getAuthStatus, the re-query still finds no persisted credentials, so authenticated remains false and the AuthPrompt reappears.
Either getAuthStatus needs to also check the in-memory token state, or setApiKey needs to persist the credential to a config file / keychain so that getExistingClaudeCredentials can find it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts` around lines 288 - 302,
getAuthStatus currently only checks getExistingClaudeCredentials
(config/keychain) so it stays false after setApiKey stores the token only
in-memory via setAnthropicAuthToken; update getAuthStatus to also inspect the
in-memory token state (call or import the same accessor that exposes the
in-memory token used by setAnthropicAuthToken) and return authenticated:true
when that token exists, or alternatively modify setApiKey to persist the
credential the same way getExistingClaudeCredentials expects (config/keychain)
so that getExistingClaudeCredentials will return a credential; reference
getAuthStatus, getExistingClaudeCredentials, setApiKey, setAnthropicAuthToken
and AuthPrompt when making the change.
Comment on lines
+116
to
+135
| if (raw) { | ||
| try { | ||
| const config: ClaudeConfigFile = JSON.parse(raw); | ||
| if (config.claudeAiOauth?.accessToken) { | ||
| console.log( | ||
| "[claude/auth] Found OAuth credentials in macOS Keychain (Claude Code-credentials)", | ||
| ); | ||
| return { | ||
| apiKey: config.claudeAiOauth.accessToken, | ||
| source: "keychain", | ||
| kind: "oauth", | ||
| }; | ||
| } | ||
| } catch { | ||
| // Not valid JSON — treat as raw API key | ||
| console.log( | ||
| "[claude/auth] Found raw credentials in macOS Keychain (Claude Code-credentials)", | ||
| ); | ||
| return { apiKey: raw, source: "keychain", kind: "apiKey" }; | ||
| } |
Contributor
There was a problem hiding this comment.
Valid JSON without claudeAiOauth.accessToken is silently ignored.
If the keychain entry parses as JSON but lacks claudeAiOauth?.accessToken, execution falls through without returning a credential or logging a warning. Other valid fields (e.g., apiKey, oauth_access_token) present in the JSON would be skipped, unlike the config-file path which checks multiple fields. If the JSON shape varies, consider checking additional fields like getCredentialsFromConfig does, or at minimum log a warning.
Proposed fix
try {
const config: ClaudeConfigFile = JSON.parse(raw);
if (config.claudeAiOauth?.accessToken) {
console.log(
"[claude/auth] Found OAuth credentials in macOS Keychain (Claude Code-credentials)",
);
return {
apiKey: config.claudeAiOauth.accessToken,
source: "keychain",
kind: "oauth",
};
}
+ // Check other known fields in the JSON
+ const apiKey = config.apiKey || config.api_key;
+ const oauthToken = config.oauthAccessToken || config.oauth_access_token;
+ if (apiKey) {
+ console.log("[claude/auth] Found API key in macOS Keychain (Claude Code-credentials)");
+ return { apiKey, source: "keychain", kind: "apiKey" };
+ }
+ if (oauthToken) {
+ console.log("[claude/auth] Found OAuth token in macOS Keychain (Claude Code-credentials)");
+ return { apiKey: oauthToken, source: "keychain", kind: "oauth" };
+ }
+ console.warn("[claude/auth] Claude Code-credentials JSON found but no recognized credential fields");
} catch {
// Not valid JSON — treat as raw API key📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (raw) { | |
| try { | |
| const config: ClaudeConfigFile = JSON.parse(raw); | |
| if (config.claudeAiOauth?.accessToken) { | |
| console.log( | |
| "[claude/auth] Found OAuth credentials in macOS Keychain (Claude Code-credentials)", | |
| ); | |
| return { | |
| apiKey: config.claudeAiOauth.accessToken, | |
| source: "keychain", | |
| kind: "oauth", | |
| }; | |
| } | |
| } catch { | |
| // Not valid JSON — treat as raw API key | |
| console.log( | |
| "[claude/auth] Found raw credentials in macOS Keychain (Claude Code-credentials)", | |
| ); | |
| return { apiKey: raw, source: "keychain", kind: "apiKey" }; | |
| } | |
| if (raw) { | |
| try { | |
| const config: ClaudeConfigFile = JSON.parse(raw); | |
| if (config.claudeAiOauth?.accessToken) { | |
| console.log( | |
| "[claude/auth] Found OAuth credentials in macOS Keychain (Claude Code-credentials)", | |
| ); | |
| return { | |
| apiKey: config.claudeAiOauth.accessToken, | |
| source: "keychain", | |
| kind: "oauth", | |
| }; | |
| } | |
| // Check other known fields in the JSON | |
| const apiKey = config.apiKey || config.api_key; | |
| const oauthToken = config.oauthAccessToken || config.oauth_access_token; | |
| if (apiKey) { | |
| console.log("[claude/auth] Found API key in macOS Keychain (Claude Code-credentials)"); | |
| return { apiKey, source: "keychain", kind: "apiKey" }; | |
| } | |
| if (oauthToken) { | |
| console.log("[claude/auth] Found OAuth token in macOS Keychain (Claude Code-credentials)"); | |
| return { apiKey: oauthToken, source: "keychain", kind: "oauth" }; | |
| } | |
| console.warn("[claude/auth] Claude Code-credentials JSON found but no recognized credential fields"); | |
| } catch { | |
| // Not valid JSON — treat as raw API key | |
| console.log( | |
| "[claude/auth] Found raw credentials in macOS Keychain (Claude Code-credentials)", | |
| ); | |
| return { apiKey: raw, source: "keychain", kind: "apiKey" }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/ai-chat/utils/auth/auth.ts` around lines
116 - 135, The code path that parses raw macOS Keychain JSON (ClaudeConfigFile)
silently ignores valid JSON that lacks claudeAiOauth.accessToken; update the
logic in the auth parsing block (the JSON.parse branch in auth.ts) to reuse the
same extraction logic used by getCredentialsFromConfig (or explicitly check for
other fields such as apiKey and oauth_access_token) and return the appropriate
credential object (kind/source) when found, and if no known fields are present
emit a clear warning log mentioning the parsed JSON keys and that no usable
credential was found (instead of falling through silently); reference
ClaudeConfigFile, claudeAiOauth, and getCredentialsFromConfig to locate where to
apply this change.
Comment on lines
+12
to
+20
| const setApiKeyMutation = electronTrpc.aiChat.setApiKey.useMutation({ | ||
| onSuccess: () => { | ||
| setError(null); | ||
| void utils.aiChat.getAuthStatus.invalidate(); | ||
| }, | ||
| onError: (err) => { | ||
| setError(err.message || "Failed to set API key"); | ||
| }, | ||
| }); |
Contributor
There was a problem hiding this comment.
Invalid API keys won't trigger the error path — setApiKey always returns { success: true }.
The test plan requires "Paste an invalid key: verify an error state is shown," but the backend setApiKey mutation (Lines 304-320 of index.ts) unconditionally calls setAnthropicAuthToken and returns { success: true } without validating the key against the Anthropic API. The onError handler here will never fire for an invalid key — users will only discover the key is bad when they attempt to send a message.
Consider adding a lightweight validation call (e.g., a models list request) in the setApiKey mutation before returning success.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/AuthPrompt/AuthPrompt.tsx`
around lines 12 - 20, The setApiKey backend mutation currently always returns
success so the front-end setApiKeyMutation onError never fires; update the
backend setApiKey implementation (the setApiKey function that calls
setAnthropicAuthToken) to perform a lightweight validation against Anthropic
(e.g., call a models/list or similar minimal endpoint) after calling
setAnthropicAuthToken and before returning { success: true }, and throw an error
if the validation fails so the frontend setApiKeyMutation.onError can surface
the invalid-key state and utils.aiChat.getAuthStatus.invalidate() remains
correct.
…pse permission picker - Render inline orange (#D97757) Claude sparkle SVG for anthropic provider instead of external models.dev image with dark:invert - Render inline OpenAI hexagonal logo for openai provider using currentColor - Rename "Autonomous" to "Auto" in permission mode picker - Collapse permission picker to icon-only when container is narrow (add @container to PromptInputTools)
The GPT 5.2 model uses provider "Codex" which wasn't matched by the providerToLogo function, falling back to the generic models.dev icon.
The 721x721 viewBox had large empty margins around the artwork, making the logo appear much smaller than Claude's at the same size.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ui/src/components/ai-elements/model-selector.tsx (1)
173-187: Decorative SVGs should be hidden from assistive technology.Both inline SVGs are decorative (the provider name is conveyed elsewhere). Adding
aria-hidden="true"prevents screen readers from attempting to announce them.♻️ Proposed fix
<svg viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg" className={cn("size-3", className)} + aria-hidden="true" >Same for the OpenAI SVG at Line 190.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/model-selector.tsx` around lines 173 - 187, The SVG used for the anthropic provider (inside the provider === "anthropic" branch) is decorative and should be hidden from assistive tech; add aria-hidden="true" to that <svg> element, and do the same for the OpenAI SVG used elsewhere (the other provider branch) so screen readers ignore these decorative inline icons.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ui/src/components/ai-elements/model-selector.tsx`:
- Around line 172-212: ModelSelectorLogo renders inline SVGs for "anthropic" and
"openai" but ModelSelectorLogoGroup only targets child imgs and the SVG branches
drop {...props}, causing styling and props to be lost; update
ModelSelectorLogoGroup's child selectors to include svg (e.g. add [&>svg]
alongside [&>img]) and modify ModelSelectorLogo so the SVG branches spread the
incoming props (apply {...props} and merge className via cn on the <svg>
elements) so event handlers, sizes and classes are preserved and group styles
apply consistently.
---
Nitpick comments:
In `@packages/ui/src/components/ai-elements/model-selector.tsx`:
- Around line 173-187: The SVG used for the anthropic provider (inside the
provider === "anthropic" branch) is decorative and should be hidden from
assistive tech; add aria-hidden="true" to that <svg> element, and do the same
for the OpenAI SVG used elsewhere (the other provider branch) so screen readers
ignore these decorative inline icons.
Comment on lines
+172
to
+212
| }: ModelSelectorLogoProps) => { | ||
| if (provider === "anthropic") { | ||
| return ( | ||
| <svg | ||
| viewBox="0 0 24 24" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| className={cn("size-3", className)} | ||
| > | ||
| <path | ||
| d="M4.709 15.955l4.72-2.647.08-.23-.08-.128H9.2l-.79-.048-2.698-.073-2.339-.097-2.266-.122-.571-.121L0 11.784l.055-.352.48-.321.686.06 1.52.103 2.278.158 1.652.097 2.449.255h.389l.055-.157-.134-.098-.103-.097-2.358-1.596-2.552-1.688-1.336-.972-.724-.491-.364-.462-.158-1.008.656-.722.881.06.225.061.893.686 1.908 1.476 2.491 1.833.365.304.145-.103.019-.073-.164-.274-1.355-2.446-1.446-2.49-.644-1.032-.17-.619a2.97 2.97 0 01-.104-.729L6.283.134 6.696 0l.996.134.42.364.62 1.414 1.002 2.229 1.555 3.03.456.898.243.832.091.255h.158V9.01l.128-1.706.237-2.095.23-2.695.08-.76.376-.91.747-.492.584.28.48.685-.067.444-.286 1.851-.559 2.903-.364 1.942h.212l.243-.242.985-1.306 1.652-2.064.73-.82.85-.904.547-.431h1.033l.76 1.129-.34 1.166-1.064 1.347-.881 1.142-1.264 1.7-.79 1.36.073.11.188-.02 2.856-.606 1.543-.28 1.841-.315.833.388.091.395-.328.807-1.969.486-2.309.462-3.439.813-.042.03.049.061 1.549.146.662.036h1.622l3.02.225.79.522.474.638-.079.485-1.215.62-1.64-.389-3.829-.91-1.312-.329h-.182v.11l1.093 1.068 2.006 1.81 2.509 2.33.127.578-.322.455-.34-.049-2.205-1.657-.851-.747-1.926-1.62h-.128v.17l.444.649 2.345 3.521.122 1.08-.17.353-.608.213-.668-.122-1.374-1.925-1.415-2.167-1.143-1.943-.14.08-.674 7.254-.316.37-.729.28-.607-.461-.322-.747.322-1.476.389-1.924.315-1.53.286-1.9.17-.632-.012-.042-.14.018-1.434 1.967-2.18 2.945-1.726 1.845-.414.164-.717-.37.067-.662.401-.589 2.388-3.036 1.44-1.882.93-1.086-.006-.158h-.055L4.132 18.56l-1.13.146-.487-.456.061-.746.231-.243 1.908-1.312-.006.006z" | ||
| fill="#D97757" | ||
| fillRule="nonzero" | ||
| /> | ||
| </svg> | ||
| ); | ||
| } | ||
| if (provider === "openai") { | ||
| return ( | ||
| <svg | ||
| viewBox="0 0 721 721" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| className={cn("size-3", className)} | ||
| > | ||
| <path | ||
| d="M304.246 294.611V249.028C304.246 245.189 305.687 242.309 309.044 240.392L400.692 187.612C413.167 180.415 428.042 177.058 443.394 177.058C500.971 177.058 537.44 221.682 537.44 269.182C537.44 272.54 537.44 276.379 536.959 280.218L441.954 224.558C436.197 221.201 430.437 221.201 424.68 224.558L304.246 294.611ZM518.245 472.145V363.224C518.245 356.505 515.364 351.707 509.608 348.349L389.174 278.296L428.519 255.743C431.877 253.826 434.757 253.826 438.115 255.743L529.762 308.523C556.154 323.879 573.905 356.505 573.905 388.171C573.905 424.636 552.315 458.225 518.245 472.141V472.145ZM275.937 376.182L236.592 353.152C233.235 351.235 231.794 348.354 231.794 344.515V238.956C231.794 187.617 271.139 148.749 324.4 148.749C344.555 148.749 363.264 155.468 379.102 167.463L284.578 222.164C278.822 225.521 275.942 230.319 275.942 237.039V376.186L275.937 376.182ZM360.626 425.122L304.246 393.455V326.283L360.626 294.616L417.002 326.283V393.455L360.626 425.122ZM396.852 570.989C376.698 570.989 357.989 564.27 342.151 552.276L436.674 497.574C442.431 494.217 445.311 489.419 445.311 482.699V343.552L485.138 366.582C488.495 368.499 489.936 371.379 489.936 375.219V480.778C489.936 532.117 450.109 570.985 396.852 570.985V570.989ZM283.134 463.99L191.486 411.211C165.094 395.854 147.343 363.229 147.343 331.562C147.343 294.616 169.415 261.509 203.48 247.593V356.991C203.48 363.71 206.361 368.508 212.117 371.866L332.074 441.437L292.729 463.99C289.372 465.907 286.491 465.907 283.134 463.99ZM277.859 542.68C223.639 542.68 183.813 501.895 183.813 451.514C183.813 447.675 184.294 443.836 184.771 439.997L279.295 494.698C285.051 498.056 290.812 498.056 296.568 494.698L417.002 425.127V470.71C417.002 474.549 415.562 477.429 412.204 479.346L320.557 532.126C308.081 539.323 293.206 542.68 277.854 542.68H277.859ZM396.852 599.776C454.911 599.776 503.37 558.513 514.41 503.812C568.149 489.896 602.696 439.515 602.696 388.176C602.696 354.587 588.303 321.962 562.392 298.45C564.791 288.373 566.231 278.296 566.231 268.224C566.231 199.611 510.571 148.267 446.274 148.267C433.322 148.267 420.846 150.184 408.37 154.505C386.775 133.392 357.026 119.958 324.4 119.958C266.342 119.958 217.883 161.22 206.843 215.921C153.104 229.837 118.557 280.218 118.557 331.557C118.557 365.146 132.95 397.771 158.861 421.283C156.462 431.36 155.022 441.437 155.022 451.51C155.022 520.123 210.682 571.466 274.978 571.466C287.931 571.466 300.407 569.549 312.883 565.228C334.473 586.341 364.222 599.776 396.852 599.776Z" | ||
| fill="currentColor" | ||
| /> | ||
| </svg> | ||
| ); | ||
| } | ||
| return ( | ||
| <img | ||
| {...props} | ||
| alt={`${provider} logo`} | ||
| className={cn("size-3 dark:invert", className)} | ||
| height={12} | ||
| src={`https://models.dev/logos/${provider}.svg`} | ||
| width={12} | ||
| /> | ||
| ); | ||
| }; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat packages/ui/src/components/ai-elements/model-selector.tsxRepository: superset-sh/superset
Length of output: 9130
SVG logos won't receive ModelSelectorLogoGroup styling and props are silently discarded.
ModelSelectorLogoGroup (lines 226–237) applies styling via [&>img] selectors (rounded-full, bg-background, p-px, ring-1, etc.). When ModelSelectorLogo renders inline SVGs for "anthropic" and "openai" providers, those SVG elements won't match the [&>img] selector, causing visual inconsistency in grouped layouts.
Additionally, the component type accepts img attributes (Omit<ComponentProps<"img">, "src" | "alt">), but the SVG branches silently discard any passed ...props (like height, width, or event handlers). Only the fallback <img> tag uses these props. This creates a type contract mismatch.
To fix:
- Update
ModelSelectorLogoGroupselectors to also target SVG:[&>svg]alongside[&>img], or - Wrap inline SVGs in a container element that can receive the group styling uniformly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/ai-elements/model-selector.tsx` around lines 172 -
212, ModelSelectorLogo renders inline SVGs for "anthropic" and "openai" but
ModelSelectorLogoGroup only targets child imgs and the SVG branches drop
{...props}, causing styling and props to be lost; update
ModelSelectorLogoGroup's child selectors to include svg (e.g. add [&>svg]
alongside [&>img]) and modify ModelSelectorLogo so the SVG branches spread the
incoming props (apply {...props} and merge className via cn on the <svg>
elements) so event handlers, sizes and classes are preserved and group styles
apply consistently.
Contributor
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/ui/src/components/ai-elements/model-selector.tsx`:
- Around line 172-216: The anthropic and openai SVG branches are not forwarding
{...props} and thus miss passed attributes and the ModelSelectorLogoGroup
styling; update the SVG returned in the provider === "anthropic" and provider
=== "openai" branches to spread ...props onto the <svg> (so it receives
id/aria/data props) and ensure the group styling is applied by including the
same group-aware className (the one used by ModelSelectorLogoGroup /
cn("size-3", className) pattern) so inline SVGs receive rounded/ring/background
styles just like the <img>; modify the SVG elements in those branches to accept
props and className via cn(..., className) and keep the fallback <img>
unchanged.
- Remove top border line, let container border provide separation - Rounded-xl container with thin 0.5px border - Pill-style toolbar buttons with subtle outline borders - Circular send button - Dark mode: #1C1C1C container bg, #353535 border, #212121 button bg
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/ChatInputFooter/ChatInputFooter.tsx (2)
63-63: Hardcoded hex colors bypass the design-token system
#353535,#1C1C1C(line 63) and#303030,#212121(line 76) are raw hex values rather than CSS variables. Any future dark-theme adjustment requires finding and updating these scattered strings.♻️ Suggested refactor — map to semantic tokens
If exact shades are unavailable in the token set, introduce named CSS variables in the theme file and reference them here:
- [&_[data-slot=input-group]]:dark:border-[`#353535`] [&_[data-slot=input-group]]:dark:bg-[`#1C1C1C`] + [&_[data-slot=input-group]]:dark:border-[--input-group-border] [&_[data-slot=input-group]]:dark:bg-[--input-group-bg]- [&_[data-size]]:dark:border-[`#303030`] [&_[data-size]]:dark:bg-[`#212121`] + [&_[data-size]]:dark:border-[--tool-border] [&_[data-size]]:dark:bg-[--tool-bg]Then define
--input-group-border,--input-group-bg,--tool-border, and--tool-bgin the dark-mode CSS layer.Also applies to: 76-76
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/ChatInputFooter/ChatInputFooter.tsx` at line 63, Replace the hardcoded dark-mode hex colors in ChatInputFooter.tsx (the className string that styles [data-slot=input-group] and the tool styles around line 63/76) with semantic CSS variables from the design-token system; update the className to reference variables like var(--input-group-border), var(--input-group-bg), var(--tool-border), and var(--tool-bg) (or existing token names) and then add those variables to the dark-mode theme layer (or the tokens file) so the component uses the theme tokens instead of raw values `#353535`, `#1C1C1C`, `#303030`, and `#212121`.
76-76:[data-size]selector is attribute-presence-only — potentially over-broad
[&_[data-size]]matches any descendant element that carries anydata-sizeattribute value, not a specific one. Any future child component that usesdata-sizefor a different semantic (e.g., icon sizing, breakpoint hints) will unintentionally inherit the border and background styles applied here. Prefer a more specific selector (e.g.,[data-size="sm"],[data-slot=tool-button]) or apply styles through the component's own props/variants.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/ChatInputFooter/ChatInputFooter.tsx` at line 76, The selector on PromptInputTools is too broad—`[&_[data-size]]` targets any descendant with any data-size attribute; narrow it to the intended target (e.g., use `[&_[data-size="sm"]]` if you only want small-size children, or switch to a more specific attribute like `[data-slot="tool-button"]`) or move the border/background styles into PromptInputTools' own props/variants so child components with unrelated data-size values aren't affected; update the className on PromptInputTools accordingly and adjust any affected child components to use the chosen specific attribute.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/ChatInputFooter/ChatInputFooter.tsx`:
- Line 63: The CSS rule using border-[0.5px] in the ChatInputFooter markup (the
div with [&_[data-slot=input-group]]:border-[0.5px]) and the matching
[data-size] elements should be changed to a value that renders consistently on
1× displays; replace border-[0.5px] with either border (1px) or border-[0.75px]
so Chromium will resolve to 1px on 1× and finer on higher DPRs, updating both
the [&_[data-slot=input-group]]:border-[0.5px] entry and the equivalent
[&_[data-size]]:border-[0.5px] occurrences in the ChatInputFooter component to
the chosen value.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/ChatInputFooter/ChatInputFooter.tsx`:
- Line 63: Replace the hardcoded dark-mode hex colors in ChatInputFooter.tsx
(the className string that styles [data-slot=input-group] and the tool styles
around line 63/76) with semantic CSS variables from the design-token system;
update the className to reference variables like var(--input-group-border),
var(--input-group-bg), var(--tool-border), and var(--tool-bg) (or existing token
names) and then add those variables to the dark-mode theme layer (or the tokens
file) so the component uses the theme tokens instead of raw values `#353535`,
`#1C1C1C`, `#303030`, and `#212121`.
- Line 76: The selector on PromptInputTools is too broad—`[&_[data-size]]`
targets any descendant with any data-size attribute; narrow it to the intended
target (e.g., use `[&_[data-size="sm"]]` if you only want small-size children,
or switch to a more specific attribute like `[data-slot="tool-button"]`) or move
the border/background styles into PromptInputTools' own props/variants so child
components with unrelated data-size values aren't affected; update the className
on PromptInputTools accordingly and adjust any affected child components to use
the chosen specific attribute.
| <div className="border-t bg-background px-4 py-3"> | ||
| <div className="mx-auto w-full max-w-3xl"> | ||
| <div className="bg-background px-4 py-3"> | ||
| <div className="mx-auto w-full max-w-3xl [&_[data-slot=input-group]]:rounded-xl [&_[data-slot=input-group]]:border-[0.5px] [&_[data-slot=input-group]]:shadow-none [&_[data-slot=input-group]]:dark:border-[#353535] [&_[data-slot=input-group]]:dark:bg-[#1C1C1C]"> |
Contributor
There was a problem hiding this comment.
border-[0.5px] is invisible on 1× DPI displays
0.5 CSS pixels collapses to 0 (invisible) or snaps to 1 px depending on device pixel ratio. On any 1× monitor the border won't render at all, making this purely a Retina-only decoration. Since this is an Electron/desktop app targeting multiple display densities, consider using border (1px) or border-[0.75px] which Chromium resolves to 1px at 1× while rendering finer at 2×.
This also applies to the same value on Line 76 for [data-size] elements.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/ChatInputFooter/ChatInputFooter.tsx`
at line 63, The CSS rule using border-[0.5px] in the ChatInputFooter markup (the
div with [&_[data-slot=input-group]]:border-[0.5px]) and the matching
[data-size] elements should be changed to a value that renders consistently on
1× displays; replace border-[0.5px] with either border (1px) or border-[0.75px]
so Chromium will resolve to 1px on 1× and finer on higher DPRs, updating both
the [&_[data-slot=input-group]]:border-[0.5px] entry and the equivalent
[&_[data-size]]:border-[0.5px] occurrences in the ChatInputFooter component to
the chosen value.
- InputGroup: rounded-xl with 0.5px border, no shadow - PromptInputButton: visible pill borders (0.5px neutral-400/60, dark #303030 border + #212121 bg) - PromptInputSubmit: circular (rounded-full) by default - ChatInputFooter: remove failed descendant selector overrides
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/ui/src/components/ui/input-group.tsx (1)
18-18:border-[0.5px]may render inconsistently on low-DPI displays.Sub-pixel border widths (0.5px) are rounded by some browsers/renderers — on 1× DPI screens the border may appear as 0px (invisible) or snap to 1px, producing an inconsistent look. Since this targets an Electron desktop app (typically HiDPI), it's likely fine in practice, but worth keeping in mind if you support varied display densities.
The rest of the styling changes (
rounded-xl,shadow-none) look good and align with the PR's presentational polish goals.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ui/input-group.tsx` at line 18, The class string in the InputGroup component (in input-group.tsx) uses a sub-pixel border width "border-[0.5px]" which can render inconsistently on low-DPI displays; replace that token with a consistent 1px border (e.g., "border" or "border-[1px]") in the same class list inside the InputGroup (the string that currently contains "group/input-group ... border-[0.5px] ...") to ensure consistent rendering across DPIs.packages/ui/src/components/ai-elements/prompt-input.tsx (1)
973-976: Consider using design tokens instead of hardcoded hex values.
dark:border-[#303030]anddark:bg-[#212121]hardcode palette values that bypass the project's theming layer. If the dark-mode palette evolves, these will require manual updates.♻️ Suggested refactor
Replace the hardcoded hex values with semantic CSS custom properties already defined in the design system, for example:
- "border-[0.5px] border-neutral-400/60 dark:border-[`#303030`] dark:bg-[`#212121`]", + "border-[0.5px] border-neutral-400/60 dark:border-border dark:bg-secondary",Adjust the token names to match whatever the project's dark-mode surface/border tokens are.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/ai-elements/prompt-input.tsx` around lines 973 - 976, The className in the cn(...) call inside prompt-input.tsx uses hardcoded hex values for dark mode (dark:border-[`#303030`] and dark:bg-[`#212121`]); update these to use the project's semantic design tokens/CSS custom properties instead (e.g. replace those two hardcoded classes with classes or utility tokens that reference the dark-mode border and surface variables from your design system) so theming/branding changes propagate automatically—locate the cn(...) usage in the PromptInput component and swap the hardcoded hex classes for the appropriate token names (dark border token and dark surface/background token) used across the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/ui/src/components/ai-elements/prompt-input.tsx`:
- Around line 973-976: The className in the cn(...) call inside prompt-input.tsx
uses hardcoded hex values for dark mode (dark:border-[`#303030`] and
dark:bg-[`#212121`]); update these to use the project's semantic design tokens/CSS
custom properties instead (e.g. replace those two hardcoded classes with classes
or utility tokens that reference the dark-mode border and surface variables from
your design system) so theming/branding changes propagate automatically—locate
the cn(...) usage in the PromptInput component and swap the hardcoded hex
classes for the appropriate token names (dark border token and dark
surface/background token) used across the codebase.
In `@packages/ui/src/components/ui/input-group.tsx`:
- Line 18: The class string in the InputGroup component (in input-group.tsx)
uses a sub-pixel border width "border-[0.5px]" which can render inconsistently
on low-DPI displays; replace that token with a consistent 1px border (e.g.,
"border" or "border-[1px]") in the same class list inside the InputGroup (the
string that currently contains "group/input-group ... border-[0.5px] ...") to
ensure consistent rendering across DPIs.
Collaborator
|
Closing: stale 'Simple chat' PR from Feb 17. Chat implementation has evolved significantly since. |
Collaborator
|
Hey — just a heads up, this was closed as part of an automated stale PR cleanup. If you think this was done in error, feel free to reopen it! |
Collaborator
|
Hey — this was closed by an automated cleanup of PRs with major merge conflicts that are 3+ weeks old. If you think this was done incorrectly, please feel free to reopen it. Sorry for any inconvenience! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
getAuthStatusquery andsetApiKeymutation to the ai-chat tRPC routerdurable-sessionpackage,streamsapp, and simplify session managementTest plan
Summary by CodeRabbit
New Features
Chores
UX