Conversation
📝 WalkthroughWalkthroughThis PR updates chat badge icon styling to prevent shrinking, adds async server-add flow with analytics in MCP settings, revises specific locale placeholders and re-maps translation references in EN/KO files, and changes a tray menu label from “Start a new meeting” to “Start a new recording.” Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant View as MCP Settings View
participant Mut as mutateAsync (server add)
participant API as Server/API
participant Analytics as Analytics Client
User->>View: Click "Add Server"
View->>Mut: mutateAsync({serverConfig})
Mut->>API: POST /servers (add)
API-->>Mut: 200 OK (server added)
Mut-->>View: resolve(result)
View->>Analytics: track("mcp_server_added", { distinct_id: userId })
View->>View: Reset input fields
opt Error path
Mut->>API: POST /servers
API-->>Mut: Error
Mut-->>View: reject(error)
View->>View: Log error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
plugins/tray/src/ext.rs (1)
233-241: Consider routing user-facing strings through i18nThis label is hard-coded in Rust while related strings in the app live in PO catalogs (e.g., “Start recording”). To avoid divergence across locales, consider centralizing this label in your translation pipeline (or at least aligning wording with the existing “Start recording” string).
If you want, I can help wire this tray label to your i18n layer or open a follow-up task.
apps/desktop/src/components/right-panel/components/chat/chat-input.tsx (2)
271-281: Remove redundant shrink-0 on iconsBoth the wrapper and icons have shrink-0; one is enough. Keep the wrapper (for layout consistency) and drop shrink-0 from the icons.
Apply this diff:
const getBadgeIcon = () => { switch (entityType) { case "human": - return <UserIcon className="size-3 shrink-0" />; + return <UserIcon className="size-3" />; case "organization": - return <BuildingIcon className="size-3 shrink-0" />; + return <BuildingIcon className="size-3" />; case "note": default: - return <FileTextIcon className="size-3 shrink-0" />; + return <FileTextIcon className="size-3" />; } };
376-379: Avoid double non-shrink constraintsSince the wrapper is already shrink-0, keeping it is sufficient and avoids redundant constraints on the child icon.
No code change needed here if you apply the icon-only diff above.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/desktop/src/components/right-panel/components/chat/chat-input.tsx(2 hunks)apps/desktop/src/components/settings/views/mcp.tsx(4 hunks)apps/desktop/src/locales/en/messages.po(19 hunks)apps/desktop/src/locales/ko/messages.po(19 hunks)plugins/tray/src/ext.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit Configuration File
**/*.{js,ts,tsx,rs}: 1. No error handling.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
plugins/tray/src/ext.rsapps/desktop/src/components/right-panel/components/chat/chat-input.tsxapps/desktop/src/components/settings/views/mcp.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-latest)
🔇 Additional comments (4)
plugins/tray/src/ext.rs (1)
233-241: Label text update looks goodThe new tray menu label reads clearly and matches the action.
apps/desktop/src/components/settings/views/mcp.tsx (2)
72-76: Nice: analytics event only on successful addEmitting mcp_server_added after a successful persist is the right place.
14-15: Verify distinct_id is always definedIf userId can be undefined during early app lifecycle, analytics may attribute events incorrectly or drop them. Consider a stable fallback (e.g., installation id) or guard the call.
I can scan the codebase to confirm userId invariants or add a lightweight fallback if you’d like.
apps/desktop/src/locales/ko/messages.po (1)
267-276: Ensure {0} receives localized Korean stringsBecause msgid is "{0}" and many base strings (“Save and close”, “Resume”, etc.) look removed/obsolete, confirm the code injects translated strings for these states. Otherwise, the Korean UI will show English fallbacks inside {0}.
I can help reintroduce msgids for “Save and close”, “Resume”, and “Go” (if needed) or adjust the call sites to pass t(...) values.
| try { | ||
| await saveServersMutation.mutateAsync(updatedServers); | ||
|
|
||
| analyticsCommands.event({ | ||
| event: "mcp_server_added", | ||
| distinct_id: userId, | ||
| }); | ||
|
|
||
| setNewUrl(""); | ||
| setNewHeaderKey(""); | ||
| setNewHeaderValue(""); | ||
| } catch (error) { | ||
| console.error("Failed to add MCP server:", error); | ||
| } |
There was a problem hiding this comment.
Console logging may leak secrets; also diverges from “No error handling” guideline
Logging error objects here risks printing headerValue (e.g., tokens) from updatedServers. Also, repo guidelines say “No error handling” for ts/tsx—prefer relying on react-query’s onError or sanitized logs.
Apply one of the following diffs.
Option A (preferred per guidelines: drop local try/catch and rely on mutation errors bubbling):
const handleAddServer = async () => {
if (!newUrl.trim() || isAtMaxLimit) {
return;
}
@@
- try {
- await saveServersMutation.mutateAsync(updatedServers);
-
- analyticsCommands.event({
- event: "mcp_server_added",
- distinct_id: userId,
- });
-
- setNewUrl("");
- setNewHeaderKey("");
- setNewHeaderValue("");
- } catch (error) {
- console.error("Failed to add MCP server:", error);
- }
+ await saveServersMutation.mutateAsync(updatedServers);
+
+ analyticsCommands.event({
+ event: "mcp_server_added",
+ distinct_id: userId,
+ });
+
+ setNewUrl("");
+ setNewHeaderKey("");
+ setNewHeaderValue("");
};Option B (if you must keep a guard, sanitize logs to avoid secrets):
- } catch (error) {
- console.error("Failed to add MCP server:", error);
- }
+ } catch {
+ // Intentionally not logging payload to avoid leaking header values
+ console.error("Failed to add MCP server");
+ }📝 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.
| try { | |
| await saveServersMutation.mutateAsync(updatedServers); | |
| analyticsCommands.event({ | |
| event: "mcp_server_added", | |
| distinct_id: userId, | |
| }); | |
| setNewUrl(""); | |
| setNewHeaderKey(""); | |
| setNewHeaderValue(""); | |
| } catch (error) { | |
| console.error("Failed to add MCP server:", error); | |
| } | |
| const handleAddServer = async () => { | |
| if (!newUrl.trim() || isAtMaxLimit) { | |
| return; | |
| } | |
| await saveServersMutation.mutateAsync(updatedServers); | |
| analyticsCommands.event({ | |
| event: "mcp_server_added", | |
| distinct_id: userId, | |
| }); | |
| setNewUrl(""); | |
| setNewHeaderKey(""); | |
| setNewHeaderValue(""); | |
| }; |
🤖 Prompt for AI Agents
In apps/desktop/src/components/settings/views/mcp.tsx around lines 69 to 82,
remove the local try/catch that calls console.error (which may leak header
values/tokens) and let the react-query mutation surface errors via its onError
handler (configure saveServersMutation.onError to handle/sanitize logging and
user feedback); if you must keep a local guard, do not log the full error object
— only log a minimal, non-sensitive message (e.g., "Failed to add MCP server")
or error.message and ensure updatedServers or any header values are never
included in logs.
| #. placeholder {0}: isViewingTemplate ? "Back" : "Save and close" | ||
| #. placeholder {0}: isCompact ? "Go" : "Resume" | ||
| #. placeholder {0}: disabled ? "Wait..." : isHovered ? (isCompact ? "Go" : "Resume") : (isCompact ? "End" : "Ended") | ||
| #: src/components/settings/views/templates.tsx:217 | ||
| #: src/components/editor-area/note-header/listen-button.tsx:153 | ||
| #: src/components/editor-area/note-header/listen-button.tsx:231 | ||
| #: src/components/editor-area/note-header/listen-button.tsx:253 | ||
| #: src/components/editor-area/note-header/listen-button.tsx:273 | ||
| #: src/components/settings/views/templates.tsx:217 | ||
| msgid "{0}" | ||
| msgstr "{0}" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Double-check: dynamic placeholder {0} stays translatable across locales
The note here indicates {0} may be values like “Back”, “Save and close”, “Go”, or “Resume”. Since msgid “Save and close” and “Resume” appear removed (marked obsolete elsewhere), ensure the code passes localized strings (e.g., tBack, tSave and close) into {0}; otherwise non-English UIs will show English text.
Run this quick search to verify placeholders are fed with translated strings:
🏁 Script executed:
#!/bin/bash
# Verify whether Back/Save and close/Go/Resume are wrapped by i18n in the call sites
rg -n -C2 -g 'src/**' -e 'templates\.tsx:217' -e 'listen-button\.tsx:(153|231|253|273)' \
; rg -n -C2 -g 'src/**' -e 't`Back`|t\\([\'"]Back[\'"]\\)|t`Save and close`|t\\([\'"]Save and close[\'"]\\)|t`Resume`|t\\([\'"]Resume[\'"]\\)|t`Go`|t\\([\'"]Go[\'"]\\)'Length of output: 353
🏁 Script executed:
#!/bin/bash
# Extract the section around line 217 in templates.tsx
sed -n '200,230p' apps/desktop/src/components/settings/views/templates.tsx
# Extract the section covering lines 153–273 in listen-button.tsx
sed -n '140,300p' apps/desktop/src/components/editor-area/note-header/listen-button.tsxLength of output: 6664
Ensure dynamic placeholder values are localized
The current usage of <Trans> with a generic {0} placeholder and raw English literals means non-English UIs will still display “Back”, “Save and close”, “Go”, “Resume”, etc. directly. Instead, each branch should call the translation function (or its own <Trans>…</Trans>) so that individual strings become extractable translation keys:
-
In
apps/desktop/src/components/settings/views/templates.tsx(around line 217):- <Trans>{isViewingTemplate ? "Back" : "Save and close"}</Trans> + {isViewingTemplate ? t("Back") : t("Save and close")}
• Import and initialize
useTranslation:import { useTranslation } from "react-i18next"; const { t } = useTranslation();
-
In
apps/desktop/src/components/editor-area/note-header/listen-button.tsx(around lines 153 and 231):- <Trans>{isCompact ? "Go" : "Resume"}</Trans> + {t(isCompact ? "Go" : "Resume")}
-
In the same file (around lines 253 and 273), replace the nested placeholder with explicit translations:
- <Trans>{disabled ? "Wait..." : isHovered ? (isCompact ? "Go" : "Resume") : (isCompact ? "End" : "Ended")}</Trans> + {disabled + ? t("Wait...") + : isHovered + ? t(isCompact ? "Go" : "Resume") + : t(isCompact ? "End" : "Ended")}
This change will generate separate translation entries for each literal, ensuring fully localized labels instead of a single {0} placeholder.
🤖 Prompt for AI Agents
In apps/desktop/src/locales/en/messages.po around lines 267 to 276, the PO entry
shows a single "{0}" placeholder used for multiple English literals; update the
corresponding components so each branch uses a real translation key instead of
passing raw literals as a placeholder. In
apps/desktop/src/components/settings/views/templates.tsx (around line 217)
import and initialize useTranslation and replace the placeholder usage with
calls to t for each label; in
apps/desktop/src/components/editor-area/note-header/listen-button.tsx (around
lines 153, 231, 253, 273) do the same—replace nested/raw strings with t("…")
calls (or separate <Trans> elements) for "Back", "Save and close", "Go",
"Resume", "End", "Ended", etc., so each literal becomes an extractable
translation key and the single "{0}" entry is removed/expanded into individual
translation entries.
1 participant
Summary by cubic
Adds analytics when adding an MCP server, fixes the chat input badge icon layout, updates the tray menu label to “Start a new recording,” and syncs i18n message references.
New Features
Bug Fixes