Skip to content

fix(desktop): handle clipboard and prompt errors from Sentry#2700

Merged
Kitenite merged 5 commits into
superset-sh:mainfrom
Kitenite:kitenite/check-sentry-crashes
Mar 21, 2026
Merged

fix(desktop): handle clipboard and prompt errors from Sentry#2700
Kitenite merged 5 commits into
superset-sh:mainfrom
Kitenite:kitenite/check-sentry-crashes

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented Mar 21, 2026

Summary

  • Add useCopyToClipboard hook that routes clipboard writes through Electron's native clipboard API via the existing external.copyPath tRPC procedure — eliminates DOMException: NotAllowedError when document focus is on terminal/webview
  • Migrate all navigator.clipboard.writeText() calls across 24 files to use the new hook, removing ~100 lines of try-catch boilerplate
  • Replace unsupported window.prompt() in BubbleMenuToolbar with an inline URL input field (Enter to apply, Escape to cancel)

Why this works

navigator.clipboard.writeText() is a browser API that requires the document to be focused. In Electron, focus frequently shifts to terminals, webviews, and BrowserViews — causing DOMException. Electron's native clipboard module (electron.clipboard.writeText()) has no such requirement. The existing external.copyPath tRPC procedure already calls it; this PR just makes it the single path for all clipboard writes.

What's left as-is

  • Terminal/helpers.ts — copy handler intercepts native copy events for whitespace trimming, must use browser clipboard API
  • OpenInMenuButton and ClickablePath — already use external.copyPath.useMutation() directly with onSuccess/isPending callbacks

Test plan

  • Verify clipboard copy works in context menus, toolbars, and settings when the document is focused
  • Verify no crash/error when clipboard copy is triggered while focus is on a terminal or webview
  • Verify link insertion in markdown editor bubble menu works with the new inline input (Enter applies, Escape cancels)
  • Verify removing an existing link still works via the link toolbar button

Summary by CodeRabbit

  • New Features

    • Inline URL entry for inserting links in the markdown editor (Enter to apply; Escape or blur to cancel).
  • Bug Fixes

    • Unified and more reliable copy-to-clipboard across the app, improving success feedback and reducing copy-related errors.

Add .catch(() => {}) to unprotected navigator.clipboard.writeText()
calls that throw DOMException when the document isn't focused (e.g.,
focus is on terminal/webview). Replace unsupported window.prompt() in
BubbleMenuToolbar with an inline URL input field.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 21, 2026

📝 Walkthrough
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(desktop): handle clipboard and prompt errors from Sentry' accurately summarizes the main changes: fixing clipboard handling and replacing unsupported prompt with inline input.
Description check ✅ Passed The PR description provides comprehensive context including the problem statement, solution, rationale, exceptions, and a detailed test plan covering all major functionality changes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10 issues found across 11 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/hooks/usePathActions.ts">

<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/hooks/usePathActions.ts:41">
P2: Avoid an empty catch here; it suppresses all clipboard failures and hides unexpected errors. Ignore the known `NotAllowedError` case, but log other failures.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>

<violation number="2" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/hooks/usePathActions.ts:47">
P2: Avoid an empty catch here; it suppresses all clipboard failures and hides unexpected errors. Ignore the known `NotAllowedError` case, but log other failures.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>
</file>

<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsx">

<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsx:92">
P2: Do not silently swallow all clipboard write failures; handle expected `NotAllowedError` explicitly and log unexpected errors.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>
</file>

<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BrowserErrorOverlay/BrowserErrorOverlay.tsx">

<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BrowserErrorOverlay/BrowserErrorOverlay.tsx:61">
P2: Do not use an empty catch here. Ignore the expected `NotAllowedError`, but keep a warning for unexpected clipboard failures so real issues are still observable.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>
</file>

<file name="apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/cloud/secrets/components/SecretsSettings/components/EnvironmentVariablesList/components/SecretRow/SecretRow.tsx">

<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/cloud/secrets/components/SecretsSettings/components/EnvironmentVariablesList/components/SecretRow/SecretRow.tsx:65">
P2: This catch-all swallow makes copy failures look like success (`Copied!` is shown even when clipboard write rejects). Only set copied state after a successful write, and handle/log unexpected errors.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>
</file>

<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BrowserToolbar/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx">

<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BrowserToolbar/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx:46">
P2: Avoid empty catch blocks here; handle the expected `NotAllowedError` case explicitly and warn on unexpected failures.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>
</file>

<file name="apps/desktop/src/renderer/routes/_authenticated/settings/api-keys/components/ApiKeysSettings/ApiKeysSettings.tsx">

<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/settings/api-keys/components/ApiKeysSettings/ApiKeysSettings.tsx:104">
P2: Do not swallow clipboard write failures with an empty catch; this hides errors and can show a false “copied” state. Log the failure and update `copied` only after a successful write.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>
</file>

<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/CommitItem/CommitItem.tsx">

<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/CommitItem/CommitItem.tsx:40">
P2: This empty catch swallows all clipboard failures. Ignore only the expected `NotAllowedError` and log unexpected errors so real regressions remain diagnosable.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>
</file>

<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/TaskMarkdownRenderer/components/CodeBlockView/CodeBlockView.tsx">

<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/TaskMarkdownRenderer/components/CodeBlockView/CodeBlockView.tsx:34">
P2: Do not show the "copied" success state when `navigator.clipboard.writeText` fails.</violation>
</file>

<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerToolbar/FileViewerToolbar.tsx">

<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerToolbar/FileViewerToolbar.tsx:60">
P2: Handle clipboard write failures instead of swallowing them; the current flow can show a successful copy state even when copy fails.

(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

… hook

Replace all navigator.clipboard.writeText() calls with a useCopyToClipboard
hook that routes through Electron's native clipboard API via IPC. This
eliminates DOMException errors when the document isn't focused (terminal,
webview, etc.) and removes ~100 lines of try-catch boilerplate.

Also replaces unsupported window.prompt() in BubbleMenuToolbar with an
inline URL input field.
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 25 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/components/EditableCodeBlockView/EditableCodeBlockView.tsx">

<violation number="1" location="apps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/components/EditableCodeBlockView/EditableCodeBlockView.tsx:37">
P2: `copied` UI state is set unconditionally, so users can see a success indicator even when the clipboard mutation fails.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/DiffViewerContextMenu/DiffViewerContextMenu.tsx (1)

67-110: ⚠️ Potential issue | 🟡 Minor

Add copyToClipboard to the useCallback dependency array.

The getEditor callback uses copyToClipboard but it's not listed in the dependency array. This could cause stale closure issues where the callback references an outdated copyToClipboard function.

🔧 Proposed fix
-	}, [containerRef, getSelectionLines]);
+	}, [containerRef, getSelectionLines, copyToClipboard]);
🤖 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/FileViewerPane/components/DiffViewerContextMenu/DiffViewerContextMenu.tsx`
around lines 67 - 110, getEditor (the callback created with useCallback) closes
over copyToClipboard but the hook's dependency array only lists containerRef and
getSelectionLines; add copyToClipboard to the dependency array so the callback
updates when the copier changes. Update the useCallback([...]) for getEditor to
include copyToClipboard (alongside containerRef and getSelectionLines) to avoid
stale closures referencing an outdated copyToClipboard implementation.
🤖 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/hooks/useCopyToClipboard.ts`:
- Around line 10-12: The hook useCopyToClipboard currently returns only a
fire-and-forget mutate callback which prevents callers from awaiting completion
or reading error/state; change it to call
electronTrpc.external.copyPath.useMutation and expose the mutation's async API
and state by returning an async handler (e.g., copyToClipboardAsync that uses
mutateAsync), plus the mutation state flags and error (e.g., isPending/isLoading
and error) so callers can await, catch errors, and render success/failure UI;
update references to the existing mutate usage in useCopyToClipboard to use
mutateAsync and return { copyToClipboardAsync, isPending, error } (or similarly
named fields) instead of a single callback.

In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx`:
- Around line 220-225: The current handleCopyPath in WorkspaceListItem.tsx calls
copyToClipboard(worktreePath) and immediately shows toast.success which can
mislead if the clipboard mutation fails; update handleCopyPath to either remove
the toast entirely to match other refactored components, or invoke the
useCopyToClipboard mutation callbacks (or await its result) and call
toast.success on success and toast.error on failure — locate the
useCopyToClipboard hook and the handleCopyPath function in WorkspaceListItem.tsx
and mirror the pattern used in ClickablePath.tsx (use onSuccess/onError or
promise resolution) to provide accurate feedback.

---

Outside diff comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/DiffViewerContextMenu/DiffViewerContextMenu.tsx`:
- Around line 67-110: getEditor (the callback created with useCallback) closes
over copyToClipboard but the hook's dependency array only lists containerRef and
getSelectionLines; add copyToClipboard to the dependency array so the callback
updates when the copier changes. Update the useCallback([...]) for getEditor to
include copyToClipboard (alongside containerRef and getSelectionLines) to avoid
stale closures referencing an outdated copyToClipboard implementation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 14dcc6ab-54f2-475c-84d5-4df32015fabb

📥 Commits

Reviewing files that changed from the base of the PR and between a306fa9 and 6e0a51b.

📒 Files selected for processing (25)
  • apps/desktop/src/renderer/components/Chat/ChatInterface/components/ModelPicker/hooks/useAnthropicOAuth/useAnthropicOAuth.ts
  • apps/desktop/src/renderer/components/Chat/ChatInterface/components/ModelPicker/hooks/useOpenAIOAuth/useOpenAIOAuth.ts
  • apps/desktop/src/renderer/components/MarkdownRenderer/components/SelectionContextMenu/SelectionContextMenu.tsx
  • apps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/components/EditableCodeBlockView/EditableCodeBlockView.tsx
  • apps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsx
  • apps/desktop/src/renderer/hooks/useCopyToClipboard.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/TaskActionMenu/TaskActionMenu.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/TaskMarkdownRenderer/components/CodeBlockView/CodeBlockView.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/page.test.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTableView/components/TaskContextMenu/TaskContextMenu.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceChat/components/WorkspaceChatInterface/components/ChatMessageList/components/UserMessage/UserMessage.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/api-keys/components/ApiKeysSettings/ApiKeysSettings.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/cloud/secrets/components/SecretsSettings/components/EnvironmentVariablesList/components/SecretRow/SecretRow.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BrowserErrorOverlay/BrowserErrorOverlay.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BrowserToolbar/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/components/UserMessage/UserMessage.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/hooks/useChatRawSnapshot/useChatRawSnapshot.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/DiffViewerContextMenu/DiffViewerContextMenu.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerToolbar/FileViewerToolbar.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/EditorContextMenu/useEditorActions.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/CommitItem/CommitItem.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/hooks/usePathActions.ts
✅ Files skipped from review due to trivial changes (2)
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BrowserToolbar/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/CommitItem/CommitItem.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/api-keys/components/ApiKeysSettings/ApiKeysSettings.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BrowserErrorOverlay/BrowserErrorOverlay.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTableView/components/TaskContextMenu/TaskContextMenu.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/TaskMarkdownRenderer/components/CodeBlockView/CodeBlockView.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/cloud/secrets/components/SecretsSettings/components/EnvironmentVariablesList/components/SecretRow/SecretRow.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/hooks/usePathActions.ts

Comment on lines +10 to +12
export function useCopyToClipboard() {
const { mutate } = electronTrpc.external.copyPath.useMutation();
return useCallback((text: string) => mutate(text), [mutate]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n "useCopyToClipboard\\(" --type ts --type tsx
rg -n -U "copyToClipboard\\([^)]+\\);\\s*(toast\\.success|setCopied\\(true\\)|setOauthError\\(null\\))" --type ts --type tsx

Repository: superset-sh/superset

Length of output: 119


🏁 Script executed:

#!/bin/bash
# Search for useCopyToClipboard hook imports/usage
rg -n "useCopyToClipboard" -g "*.ts" -g "*.tsx"

Repository: superset-sh/superset

Length of output: 9435


🏁 Script executed:

#!/bin/bash
# Read the hook file
cat -n apps/desktop/src/renderer/hooks/useCopyToClipboard.ts

Repository: superset-sh/superset

Length of output: 643


🏁 Script executed:

#!/bin/bash
# Check the trpc mutation definition
find apps/desktop -name "*.ts" -o -name "*.tsx" | xargs grep -l "copyPath.*Mutation\|copyPath.*Procedure" | head -5

Repository: superset-sh/superset

Length of output: 503


🏁 Script executed:

#!/bin/bash
# Read the external router to understand copyPath mutation
cat -n apps/desktop/src/lib/trpc/routers/external/index.ts

Repository: superset-sh/superset

Length of output: 5954


🏁 Script executed:

#!/bin/bash
# Sample usage from one of the main call sites
cat -n apps/desktop/src/renderer/routes/_authenticated/settings/api-keys/components/ApiKeysSettings/ApiKeysSettings.tsx | sed -n '100,120p'

Repository: superset-sh/superset

Length of output: 656


🏁 Script executed:

#!/bin/bash
# Another representative usage
cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/components/UserMessage/UserMessage.tsx | sed -n '60,90p'

Repository: superset-sh/superset

Length of output: 1128


🏁 Script executed:

#!/bin/bash
# Sample more usage patterns to see if any handle errors/loading
cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerToolbar/FileViewerToolbar.tsx | sed -n '55,75p'

Repository: superset-sh/superset

Length of output: 885


🏁 Script executed:

#!/bin/bash
# Check if clipboard.writeText is wrapped with error handling anywhere
rg -n "clipboard\.writeText|copyPath\(" apps/desktop/src/lib/trpc --type ts

Repository: superset-sh/superset

Length of output: 244


🏁 Script executed:

#!/bin/bash
# Check if mutateAsync or isPending from the mutation is used anywhere in call sites
rg -A5 "useCopyToClipboard()" apps/desktop/src/renderer --type ts --type tsx | grep -E "mutateAsync|isPending|\.isError|\.isLoading"

Repository: superset-sh/superset

Length of output: 91


🏁 Script executed:

#!/bin/bash
# Verify call sites don't use any mutation state (isPending, isError, etc)
rg -g "*.ts" -g "*.tsx" -A3 "useCopyToClipboard()" apps/desktop/src/renderer | grep -E "isPending|isError|isLoading|mutateAsync" || echo "No mutation state usage found"

Repository: superset-sh/superset

Length of output: 93


Expose mutation state and async handler from the copy hook.

useCopyToClipboard currently returns only a fire-and-forget mutate callback, forcing all 45+ call sites to assume success and immediately set UI state (setCopied(true)) without awaiting completion or checking for errors. This prevents callers from detecting clipboard failures or responding to mutation state.

The tRPC mutation itself lacks error handling and all examined call sites follow the same anti-pattern: call copyToClipboard(), then immediately show success UI.

Expose copyToClipboardAsync, isPending, and error support so callers can properly gate success/error UI on actual mutation completion:

Proposed refactor
 export function useCopyToClipboard() {
-	const { mutate } = electronTrpc.external.copyPath.useMutation();
-	return useCallback((text: string) => mutate(text), [mutate]);
+	const mutation = electronTrpc.external.copyPath.useMutation();
+
+	const copyToClipboard = useCallback(
+		(text: string, opts?: Parameters<typeof mutation.mutate>[1]) =>
+			mutation.mutate(text, opts),
+		[mutation],
+	);
+
+	const copyToClipboardAsync = useCallback(
+		(text: string) => mutation.mutateAsync(text),
+		[mutation],
+	);
+
+	return { copyToClipboard, copyToClipboardAsync, isPending: mutation.isPending };
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/renderer/hooks/useCopyToClipboard.ts` around lines 10 - 12,
The hook useCopyToClipboard currently returns only a fire-and-forget mutate
callback which prevents callers from awaiting completion or reading error/state;
change it to call electronTrpc.external.copyPath.useMutation and expose the
mutation's async API and state by returning an async handler (e.g.,
copyToClipboardAsync that uses mutateAsync), plus the mutation state flags and
error (e.g., isPending/isLoading and error) so callers can await, catch errors,
and render success/failure UI; update references to the existing mutate usage in
useCopyToClipboard to use mutateAsync and return { copyToClipboardAsync,
isPending, error } (or similarly named fields) instead of a single callback.

Comment on lines 220 to 225
const copyToClipboard = useCopyToClipboard();
const handleCopyPath = () => {
if (!worktreePath) return;
try {
await navigator.clipboard.writeText(worktreePath);
toast.success("Path copied to clipboard");
} catch {
toast.error("Failed to copy path");
}
copyToClipboard(worktreePath);
toast.success("Path copied to clipboard");
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Unconditional success toast may mislead users on clipboard failures.

The toast.success is called synchronously after copyToClipboard(), but the hook's underlying mutation is fire-and-forget. If the clipboard operation fails, the user still sees "Path copied to clipboard" despite the failure.

Consider using the mutation's callbacks directly (as done in ClickablePath.tsx) or removing the toast to be consistent with other components in this PR that silently handle clipboard operations.

🔧 Option 1: Remove success toast (consistent with other refactored components)
 	const copyToClipboard = useCopyToClipboard();
 	const handleCopyPath = () => {
 		if (!worktreePath) return;
 		copyToClipboard(worktreePath);
-		toast.success("Path copied to clipboard");
 	};
🔧 Option 2: Use mutation callbacks for accurate feedback
-	const copyToClipboard = useCopyToClipboard();
+	const copyPath = electronTrpc.external.copyPath.useMutation({
+		onSuccess: () => toast.success("Path copied to clipboard"),
+		onError: (error) => toast.error(`Failed to copy path: ${error.message}`),
+	});
 	const handleCopyPath = () => {
 		if (!worktreePath) return;
-		copyToClipboard(worktreePath);
-		toast.success("Path copied to clipboard");
+		copyPath.mutate(worktreePath);
 	};
📝 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
const copyToClipboard = useCopyToClipboard();
const handleCopyPath = () => {
if (!worktreePath) return;
try {
await navigator.clipboard.writeText(worktreePath);
toast.success("Path copied to clipboard");
} catch {
toast.error("Failed to copy path");
}
copyToClipboard(worktreePath);
toast.success("Path copied to clipboard");
};
const copyToClipboard = useCopyToClipboard();
const handleCopyPath = () => {
if (!worktreePath) return;
copyToClipboard(worktreePath);
};
🤖 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/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx`
around lines 220 - 225, The current handleCopyPath in WorkspaceListItem.tsx
calls copyToClipboard(worktreePath) and immediately shows toast.success which
can mislead if the clipboard mutation fails; update handleCopyPath to either
remove the toast entirely to match other refactored components, or invoke the
useCopyToClipboard mutation callbacks (or await its result) and call
toast.success on success and toast.error on failure — locate the
useCopyToClipboard hook and the handleCopyPath function in WorkspaceListItem.tsx
and mirror the pattern used in ClickablePath.tsx (use onSuccess/onError or
promise resolution) to provide accurate feedback.

useCopyToClipboard now uses mutateAsync and manages copied state
internally, so callers no longer show false "Copied!" UI on failure.
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found across 24 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/desktop/src/renderer/hooks/useCopyToClipboard.ts">

<violation number="1" location="apps/desktop/src/renderer/hooks/useCopyToClipboard.ts:19">
P1: Handle `mutateAsync` failures inside this hook (or make all callers await/catch) to avoid unhandled promise rejections.

(Based on your team's feedback about awaiting/catching rejecting async calls.) [FEEDBACK_USED]</violation>

<violation number="2" location="apps/desktop/src/renderer/hooks/useCopyToClipboard.ts:21">
P2: Consecutive copy actions race because previous reset timers are not cleared, so `copied` can turn false earlier than the configured timeout.</violation>
</file>

<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/hooks/useChatRawSnapshot/useChatRawSnapshot.ts">

<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/hooks/useChatRawSnapshot/useChatRawSnapshot.ts:33">
P2: `copyToClipboard` is async, but this handler calls it without `await`/error handling. A failed clipboard mutation can raise an unhandled rejection and still show a success toast.

(Based on your team's feedback about awaiting and catching rejecting async calls.) [FEEDBACK_USED]</violation>
</file>

<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx">

<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx:223">
P2: Wrap the awaited copyToClipboard call in try/catch and surface an error toast so failures don’t bubble as unhandled promise rejections or incorrectly report success.

(Based on your team's feedback about handling async calls with proper await and catch.) [FEEDBACK_USED]</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment on lines +19 to +21
await mutateAsync(text);
setCopied(true);
setTimeout(() => setCopied(false), timeout);
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Handle mutateAsync failures inside this hook (or make all callers await/catch) to avoid unhandled promise rejections.

(Based on your team's feedback about awaiting/catching rejecting async calls.)

View Feedback

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/hooks/useCopyToClipboard.ts, line 19:

<comment>Handle `mutateAsync` failures inside this hook (or make all callers await/catch) to avoid unhandled promise rejections.

(Based on your team's feedback about awaiting/catching rejecting async calls.) </comment>

<file context>
@@ -1,13 +1,27 @@
+
+	const copyToClipboard = useCallback(
+		async (text: string) => {
+			await mutateAsync(text);
+			setCopied(true);
+			setTimeout(() => setCopied(false), timeout);
</file context>
Suggested change
await mutateAsync(text);
setCopied(true);
setTimeout(() => setCopied(false), timeout);
try {
await mutateAsync(text);
} catch (error) {
console.warn("[useCopyToClipboard] clipboard write failed", error);
return;
}
setCopied(true);
setTimeout(() => setCopied(false), timeout);
Fix with Cubic

async (text: string) => {
await mutateAsync(text);
setCopied(true);
setTimeout(() => setCopied(false), timeout);
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Consecutive copy actions race because previous reset timers are not cleared, so copied can turn false earlier than the configured timeout.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/hooks/useCopyToClipboard.ts, line 21:

<comment>Consecutive copy actions race because previous reset timers are not cleared, so `copied` can turn false earlier than the configured timeout.</comment>

<file context>
@@ -1,13 +1,27 @@
+		async (text: string) => {
+			await mutateAsync(text);
+			setCopied(true);
+			setTimeout(() => setCopied(false), timeout);
+		},
+		[mutateAsync, timeout],
</file context>
Fix with Cubic

Comment on lines +33 to 36
const { copyToClipboard } = useCopyToClipboard();

const handleCopyRawSnapshot = useCallback(() => {
const rawSnapshot = rawSnapshotRef.current;
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: copyToClipboard is async, but this handler calls it without await/error handling. A failed clipboard mutation can raise an unhandled rejection and still show a success toast.

(Based on your team's feedback about awaiting and catching rejecting async calls.)

View Feedback

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/hooks/useChatRawSnapshot/useChatRawSnapshot.ts, line 33:

<comment>`copyToClipboard` is async, but this handler calls it without `await`/error handling. A failed clipboard mutation can raise an unhandled rejection and still show a success toast.

(Based on your team's feedback about awaiting and catching rejecting async calls.) </comment>

<file context>
@@ -30,7 +30,7 @@ export function useChatRawSnapshot({
 	}, []);
 
-	const copyToClipboard = useCopyToClipboard();
+	const { copyToClipboard } = useCopyToClipboard();
 
 	const handleCopyRawSnapshot = useCallback(() => {
</file context>
Fix with Cubic

Comment on lines +223 to +224
await copyToClipboard(worktreePath);
toast.success("Path copied to clipboard");
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Wrap the awaited copyToClipboard call in try/catch and surface an error toast so failures don’t bubble as unhandled promise rejections or incorrectly report success.

(Based on your team's feedback about handling async calls with proper await and catch.)

View Feedback

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx, line 223:

<comment>Wrap the awaited copyToClipboard call in try/catch and surface an error toast so failures don’t bubble as unhandled promise rejections or incorrectly report success.

(Based on your team's feedback about handling async calls with proper await and catch.) </comment>

<file context>
@@ -217,10 +217,10 @@ export function WorkspaceListItem({
+	const handleCopyPath = async () => {
 		if (!worktreePath) return;
-		copyToClipboard(worktreePath);
+		await copyToClipboard(worktreePath);
 		toast.success("Path copied to clipboard");
 	};
</file context>
Suggested change
await copyToClipboard(worktreePath);
toast.success("Path copied to clipboard");
try {
await copyToClipboard(worktreePath);
toast.success("Path copied to clipboard");
} catch (error) {
toast.error(
`Failed to copy path: ${error instanceof Error ? error.message : String(error)}`,
);
}
Fix with Cubic

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/components/UserMessage/UserMessage.tsx (1)

45-72: ⚠️ Potential issue | 🟠 Major

Remove duplicate local copied state; use hook-provided copied only.

This currently shows success immediately (setCopied(true)) even if the copy fails, which defeats the new centralized success gating.

Proposed fix
-import { useCallback, useState } from "react";
+import { useCallback } from "react";
@@
-	const [copied, setCopied] = useState(false);
@@
-	const { copyToClipboard } = useCopyToClipboard();
+	const { copyToClipboard, copied } = useCopyToClipboard(1500);
 	const handleCopy = useCallback(() => {
 		if (!fullText) return;
-		copyToClipboard(fullText);
-		setCopied(true);
-		setTimeout(() => setCopied(false), 1500);
+		void copyToClipboard(fullText);
 	}, [fullText, copyToClipboard]);
🤖 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/ChatPaneInterface/components/ChatMessageList/components/UserMessage/UserMessage.tsx`
around lines 45 - 72, Remove the local copied useState and all uses of setCopied
in this file and rely on the centralized hook-provided copied state from
useCopyToClipboard; specifically delete the line declaring const [copied,
setCopied] = useState(false) and remove the setCopied(true)/setCopied(false) and
setTimeout call from handleCopy, then call copyToClipboard(fullText) and return
(or await) its result so the hook can manage success gating and copied state;
keep references to useCopyToClipboard, copyToClipboard, and handleCopy intact.
🤖 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/components/Chat/ChatInterface/components/ModelPicker/hooks/useOpenAIOAuth/useOpenAIOAuth.ts`:
- Around line 104-108: In useOpenAIOAuth, the copyOAuthUrl callback clears
oauthError unconditionally which hides errors if copying fails; modify
copyOAuthUrl to attempt copyToClipboard(oauthUrl) and only call
setOauthError(null) when that copy succeeds (handle promise/return value from
copyToClipboard), otherwise preserve or set an appropriate error via
setOauthError; update the callback to rely on the copyToClipboard result and
keep references to copyOAuthUrl, copyToClipboard, setOauthError, and oauthUrl so
the error state is only cleared on successful copy.

In `@apps/desktop/src/renderer/hooks/useCopyToClipboard.ts`:
- Around line 17-23: The useCopyToClipboard hook's copyToClipboard callback must
catch/reject-safe the mutateAsync call and manage the timeout timer: wrap the
await mutateAsync(text) in try/catch (log or swallow the error) so rejections
don't become unhandled, and track the active timer id in a ref (e.g. timerRef)
so you clearTimeout(timerRef.current) before creating a new timer and assign the
id; also add a useEffect cleanup that clears the timer on unmount. Update
references to copyToClipboard, mutateAsync, setCopied, timeout, and useCallback
accordingly.

---

Outside diff comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/components/UserMessage/UserMessage.tsx`:
- Around line 45-72: Remove the local copied useState and all uses of setCopied
in this file and rely on the centralized hook-provided copied state from
useCopyToClipboard; specifically delete the line declaring const [copied,
setCopied] = useState(false) and remove the setCopied(true)/setCopied(false) and
setTimeout call from handleCopy, then call copyToClipboard(fullText) and return
(or await) its result so the hook can manage success gating and copied state;
keep references to useCopyToClipboard, copyToClipboard, and handleCopy intact.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 967658bd-605c-4665-8838-0669dcdcbe88

📥 Commits

Reviewing files that changed from the base of the PR and between 6e0a51b and 3514fd4.

📒 Files selected for processing (24)
  • apps/desktop/src/renderer/components/Chat/ChatInterface/components/ModelPicker/hooks/useAnthropicOAuth/useAnthropicOAuth.ts
  • apps/desktop/src/renderer/components/Chat/ChatInterface/components/ModelPicker/hooks/useOpenAIOAuth/useOpenAIOAuth.ts
  • apps/desktop/src/renderer/components/MarkdownRenderer/components/SelectionContextMenu/SelectionContextMenu.tsx
  • apps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/components/EditableCodeBlockView/EditableCodeBlockView.tsx
  • apps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsx
  • apps/desktop/src/renderer/hooks/useCopyToClipboard.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/TaskActionMenu/TaskActionMenu.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/TaskMarkdownRenderer/components/CodeBlockView/CodeBlockView.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTableView/components/TaskContextMenu/TaskContextMenu.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceChat/components/WorkspaceChatInterface/components/ChatMessageList/components/UserMessage/UserMessage.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/api-keys/components/ApiKeysSettings/ApiKeysSettings.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/cloud/secrets/components/SecretsSettings/components/EnvironmentVariablesList/components/SecretRow/SecretRow.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BrowserErrorOverlay/BrowserErrorOverlay.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BrowserToolbar/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/components/UserMessage/UserMessage.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/hooks/useChatRawSnapshot/useChatRawSnapshot.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/DiffViewerContextMenu/DiffViewerContextMenu.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/FileViewerToolbar/FileViewerToolbar.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/EditorContextMenu/useEditorActions.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/CommitItem/CommitItem.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/hooks/usePathActions.ts
✅ Files skipped from review due to trivial changes (5)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTableView/components/TaskContextMenu/TaskContextMenu.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/CommitItem/CommitItem.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/BrowserToolbar/components/BrowserOverflowMenu/BrowserOverflowMenu.tsx
  • apps/desktop/src/renderer/routes/_authenticated/settings/api-keys/components/ApiKeysSettings/ApiKeysSettings.tsx
  • apps/desktop/src/renderer/components/MarkdownRenderer/components/TipTapMarkdownRenderer/components/EditableCodeBlockView/EditableCodeBlockView.tsx
🚧 Files skipped from review as they are similar to previous changes (12)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/components/DiffViewerContextMenu/DiffViewerContextMenu.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceChat/components/WorkspaceChatInterface/components/ChatMessageList/components/UserMessage/UserMessage.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/TaskMarkdownRenderer/components/CodeBlockView/CodeBlockView.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/TaskActionMenu/TaskActionMenu.tsx
  • apps/desktop/src/renderer/components/Chat/ChatInterface/components/ModelPicker/hooks/useAnthropicOAuth/useAnthropicOAuth.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/hooks/usePathActions.ts
  • apps/desktop/src/renderer/components/MarkdownRenderer/components/SelectionContextMenu/SelectionContextMenu.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/EditorContextMenu/useEditorActions.ts
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/hooks/useChatRawSnapshot/useChatRawSnapshot.ts

Comment on lines +104 to +108
const copyOAuthUrl = useCallback(() => {
if (!oauthUrl) return;
try {
await navigator.clipboard.writeText(oauthUrl);
setOauthError(null);
} catch (error) {
setOauthError(getErrorMessage(error, "Failed to copy URL"));
}
}, [oauthUrl]);
copyToClipboard(oauthUrl);
setOauthError(null);
}, [oauthUrl, copyToClipboard]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Don’t clear oauthError before copy success is known.

setOauthError(null) runs regardless of copy outcome. This can suppress a useful error state when copying fails.

Proposed fix
-const copyOAuthUrl = useCallback(() => {
+const copyOAuthUrl = useCallback(async () => {
 	if (!oauthUrl) return;
-	copyToClipboard(oauthUrl);
-	setOauthError(null);
+	try {
+		await copyToClipboard(oauthUrl);
+		setOauthError(null);
+	} catch (error) {
+		setOauthError(getErrorMessage(error, "Failed to copy OAuth URL"));
+	}
 }, [oauthUrl, copyToClipboard]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/components/Chat/ChatInterface/components/ModelPicker/hooks/useOpenAIOAuth/useOpenAIOAuth.ts`
around lines 104 - 108, In useOpenAIOAuth, the copyOAuthUrl callback clears
oauthError unconditionally which hides errors if copying fails; modify
copyOAuthUrl to attempt copyToClipboard(oauthUrl) and only call
setOauthError(null) when that copy succeeds (handle promise/return value from
copyToClipboard), otherwise preserve or set an appropriate error via
setOauthError; update the callback to rely on the copyToClipboard result and
keep references to copyOAuthUrl, copyToClipboard, setOauthError, and oauthUrl so
the error state is only cleared on successful copy.

Comment on lines +17 to +23
const copyToClipboard = useCallback(
async (text: string) => {
await mutateAsync(text);
setCopied(true);
setTimeout(() => setCopied(false), timeout);
},
[mutateAsync, timeout],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n apps/desktop/src/renderer/hooks/useCopyToClipboard.ts

Repository: superset-sh/superset

Length of output: 1102


🏁 Script executed:

# Search for usages of useCopyToClipboard hook in the codebase
rg "useCopyToClipboard" --type ts --type tsx -B 2 -A 2

Repository: superset-sh/superset

Length of output: 91


🏁 Script executed:

# Check if there are any test files for this hook
fd "useCopyToClipboard" --type f

Repository: superset-sh/superset

Length of output: 117


🏁 Script executed:

# Search for usages of useCopyToClipboard hook in the codebase (correcting the type)
rg "useCopyToClipboard" --type ts -B 2 -A 2

Repository: superset-sh/superset

Length of output: 41235


🏁 Script executed:

# Check the trpc mutation to understand what mutateAsync does
rg "copyPath" --type ts -B 3 -A 3

Repository: superset-sh/superset

Length of output: 42055


🏁 Script executed:

# Check how callbacks that return promises are typically handled in this codebase
rg "useCallback.*async" --type ts -A 5 | head -50

Repository: superset-sh/superset

Length of output: 4807


🏁 Script executed:

# Check a few specific call sites to confirm none are awaiting
rg "copyToClipboard\(" apps/desktop/src/renderer -A 1 | head -40

Repository: superset-sh/superset

Length of output: 4450


🏁 Script executed:

# Look for any error boundaries or error handlers around these calls
rg -B 5 "copyToClipboard\(" apps/desktop/src/renderer/hooks/useCopyToClipboard.ts -A 5

Repository: superset-sh/superset

Length of output: 46


Add error handling and timer cleanup to useCopyToClipboard hook.

The hook currently lacks error handling for mutateAsync rejections and doesn't clean up timers on unmount. Since nearly all callers invoke copyToClipboard(text) without await, rejected mutations become unhandled promise rejections. Additionally, multiple rapid calls create overlapping timers that can incorrectly reset state after the intended timeout.

Proposed fix
-import { useCallback, useState } from "react";
+import { useCallback, useEffect, useRef, useState } from "react";
 import { electronTrpc } from "renderer/lib/electron-trpc";
@@
 export function useCopyToClipboard(timeout = 2000) {
 	const { mutateAsync } = electronTrpc.external.copyPath.useMutation();
 	const [copied, setCopied] = useState(false);
+	const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+
+	useEffect(
+		() => () => {
+			if (timeoutRef.current) {
+				clearTimeout(timeoutRef.current);
+			}
+		},
+		[],
+	);
 
 	const copyToClipboard = useCallback(
 		async (text: string) => {
-			await mutateAsync(text);
-			setCopied(true);
-			setTimeout(() => setCopied(false), timeout);
+			try {
+				await mutateAsync(text);
+				setCopied(true);
+				if (timeoutRef.current) {
+					clearTimeout(timeoutRef.current);
+				}
+				timeoutRef.current = setTimeout(() => setCopied(false), timeout);
+				return true;
+			} catch {
+				return false;
+			}
 		},
 		[mutateAsync, timeout],
 	);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/renderer/hooks/useCopyToClipboard.ts` around lines 17 - 23,
The useCopyToClipboard hook's copyToClipboard callback must catch/reject-safe
the mutateAsync call and manage the timeout timer: wrap the await
mutateAsync(text) in try/catch (log or swallow the error) so rejections don't
become unhandled, and track the active timer id in a ref (e.g. timerRef) so you
clearTimeout(timerRef.current) before creating a new timer and assign the id;
also add a useEffect cleanup that clears the timer on unmount. Update references
to copyToClipboard, mutateAsync, setCopied, timeout, and useCallback
accordingly.

@Kitenite Kitenite merged commit 5f32394 into superset-sh:main Mar 21, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant