fix(network): remote LAN-share UI actually works (same-origin API + safe clipboard) - #171
Conversation
… :3900 When a device opens the LAN-share URL, the SPA is served by the share listener on :5050 but client.ts hardcoded the API to :3900 — cross-origin (CORS-blocked) AND loopback-only/unreachable from another machine, so every fetch failed. The share listener serves the same app+API, so the remote SPA must hit its OWN origin. _resolveApiBase: Tauri→127.0.0.1; vite-dev→:3900 (CORS-allowed); else (share listener / docker / prod build)→window.location.origin. +6 unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
navigator.clipboard is secure-context-only (https/localhost); on a LAN-shared instance at http://<ip>:<port> it's undefined, so unguarded navigator.clipboard.writeText(...) threw 'Cannot read properties of undefined (reading writeText)' — crashing copy buttons on remote devices. Add a copyText util (clipboard API when available, hidden-textarea + execCommand fallback otherwise) and route all 9 call sites through it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR refactors API base URL resolution into a testable pure function and introduces a centralized clipboard copy utility with fallback support, replacing direct navigator.clipboard calls across nine UI components for better robustness across insecure and edge-case contexts. ChangesAPI Base URL Resolution
Clipboard Copy Utility
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| * page updates in realtime without requiring a shared store. | ||
| */ | ||
| import React, { useState, useCallback, useMemo, useEffect } from 'react'; | ||
| import { copyText } from "../utils/copyText"; |
|
| Filename | Overview |
|---|---|
| frontend/src/pages/Transcriptions.jsx | Imports copyText but then declares a local const copyText that shadows the import, causing infinite recursion on every copy action. |
| frontend/src/api/client.ts | Refactors API base URL resolution into a testable _resolveApiBase function; correctly routes Tauri→loopback, dev→:3900, and LAN-served→window.location.origin. |
| frontend/src/utils/copyText.js | New clipboard utility with secure-context guard and textarea/execCommand fallback; correctly handles undefined navigator.clipboard over plain HTTP. |
| frontend/src/components/NetworkToggle.jsx | Swaps navigator.clipboard for copyText utility; toast fires without awaiting the promise (pre-existing pattern, not a regression). |
| frontend/src/components/settings/SharingPanel.jsx | Same un-awaited copyText pattern as NetworkToggle; otherwise a clean migration to the safe clipboard utility. |
| frontend/src/api/client.apibase.test.ts | Adds 6 unit tests covering all _resolveApiBase branches including the LAN-share same-origin case. |
| frontend/src/utils/copyText.test.js | Tests secure-context clipboard path and execCommand fallback; correctly mocks navigator.clipboard absence. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[_resolveApiBase called] --> B{VITE_API_URL set?}
B -- yes --> C[Return VITE_API_URL override]
B -- no --> D{window exists?}
D -- no SSR --> E[Return 127.0.0.1:port]
D -- yes --> F{window.__TAURI__?}
F -- yes --> G[Return 127.0.0.1:port local sidecar]
F -- no --> H{import.meta.env.DEV?}
H -- yes --> I[Return hostname:port localhost:3900 in dev]
H -- no --> J[Return window.location.origin LAN share / Docker / prod]
K[copyText called] --> L{navigator.clipboard available?}
L -- yes --> M[navigator.clipboard.writeText]
M -- success --> N[return true]
M -- throws --> O[fall through to execCommand]
L -- no plain HTTP --> O
O --> P[hidden textarea + execCommand copy]
P --> Q[return ok boolean]
Comments Outside Diff (1)
-
frontend/src/pages/Transcriptions.jsx, line 77-82 (link)Infinite recursion — stack overflow on every copy
The local
const copyText = useCallback(...)shadows the importedcopyTextfrom../utils/copyText. Inside the callback body,copyText(text)resolves to the same local variable, so it calls itself instead of the utility — causing aRangeError: Maximum call stack size exceededwhenever a user tries to copy a transcription. The import is never reached.
Reviews (1): Last reviewed commit: "fix(ui): safe clipboard copy over plain ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| }; | ||
|
|
||
| const copy = (text) => { navigator.clipboard?.writeText(text); toast.success('Copied'); }; | ||
| const copy = (text) => { copyText(text); toast.success('Copied'); }; |
There was a problem hiding this comment.
toast.success fires before copy resolves
copyText(text) returns a Promise but it is not awaited, so toast.success('Copied') fires immediately regardless of whether the copy succeeds or fails. The same pattern also exists in SharingPanel.jsx. The previous navigator.clipboard?.writeText was equally un-awaited, so this isn't a regression — but the new utility makes it trivial to await and gate the toast on success.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
frontend/src/pages/Transcriptions.jsx (1)
77-82:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: name shadowing causes infinite recursion — the imported
copyTextis never called.The local
const copyText(Line 77) shadows the importedcopyTextfrom../utils/copyText(Line 11). Inside the callback body,copyText(text)resolves to the local binding itself, so invoking it recurses until a stack overflow (Maximum call stack size exceeded). Every copy action in this page (Line 210) will throw, and the new utility is never reached — directly defeating this PR's clipboard fix.Alias the import to break the shadowing:
🐛 Proposed fix
-import { copyText } from "../utils/copyText"; +import { copyText as copyTextUtil } from "../utils/copyText";const copyText = useCallback((text) => { - copyText(text).then( + copyTextUtil(text).then( () => toast.success(t('transcriptions.copied')), () => toast.error(t('transcriptions.copy_failed')) ); }, [t]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/Transcriptions.jsx` around lines 77 - 82, The local const copyText defined with useCallback shadows the imported copyText utility and causes infinite recursion; rename one of them (recommended: alias the import to copyToClipboard or rename the callback to handleCopyText) and update the callback body to call the imported function (e.g., copyToClipboard(text).then(...)) so the utility from ../utils/copyText is actually invoked; ensure references elsewhere (e.g., where copy is triggered on line ~210) use the new callback name if you renamed it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/NetworkToggle.jsx`:
- Line 51: The helper function copy currently calls the async copyText without
awaiting its Promise, causing toast.success to show even on failure; update the
copy function (the copy helper that calls copyText and toast.success) to await
copyText(text), check the returned boolean, and only call
toast.success('Copied') when it resolves true, otherwise call toast.error(...)
or another failure handler so users see correct feedback.
In `@frontend/src/components/settings/SharingPanel.jsx`:
- Line 154: The copy helper in SharingPanel.jsx currently calls copyText(text)
without awaiting it, so toast.success('Copied') runs even on failure; make the
copy function async, await copyText(text), and move toast.success into the
success path, catching errors to call toast.error (or show appropriate failure
feedback); reference the copy function and the copyText helper and ensure you
handle promise rejection from copyText before showing success.
In `@frontend/src/pages/Projects.jsx`:
- Around line 172-175: The onClick handler currently calls copyText(tr.text ||
'') without awaiting or reporting result; update the handler in Projects.jsx to
await copyText(tr.text || '') and show user feedback: on success call the
project's toast/notification helper (e.g., toast.success or showToast) to
display a "Copied" message, and on failure catch the error and show an error
toast with the error message; reference the onClick handler and the copyText
function so the change is limited to that click callback and uses existing toast
utilities in the component.
---
Outside diff comments:
In `@frontend/src/pages/Transcriptions.jsx`:
- Around line 77-82: The local const copyText defined with useCallback shadows
the imported copyText utility and causes infinite recursion; rename one of them
(recommended: alias the import to copyToClipboard or rename the callback to
handleCopyText) and update the callback body to call the imported function
(e.g., copyToClipboard(text).then(...)) so the utility from ../utils/copyText is
actually invoked; ensure references elsewhere (e.g., where copy is triggered on
line ~210) use the new callback name if you renamed it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 48571aaf-18a4-465d-8b44-134c42311805
📒 Files selected for processing (13)
frontend/src/api/client.apibase.test.tsfrontend/src/api/client.tsfrontend/src/components/BootstrapSplash.jsxfrontend/src/components/CaptureWidget.jsxfrontend/src/components/LogsFooter.jsxfrontend/src/components/NetworkToggle.jsxfrontend/src/components/settings/SharingPanel.jsxfrontend/src/pages/DubTab.jsxfrontend/src/pages/Projects.jsxfrontend/src/pages/Settings.jsxfrontend/src/pages/Transcriptions.jsxfrontend/src/utils/copyText.jsfrontend/src/utils/copyText.test.js
| }; | ||
|
|
||
| const copy = (text) => { navigator.clipboard?.writeText(text); toast.success('Copied'); }; | ||
| const copy = (text) => { copyText(text); toast.success('Copied'); }; |
There was a problem hiding this comment.
Missing await breaks error feedback.
copyText is async and returns Promise<boolean>, but this helper doesn't await it. The success toast will fire immediately, even when the copy fails. Users will see "Copied" when nothing was actually copied to the clipboard.
🔧 Proposed fix
- const copy = (text) => { copyText(text); toast.success('Copied'); };
+ const copy = async (text) => {
+ const ok = await copyText(text);
+ if (ok) toast.success('Copied');
+ else toast.error('Copy failed');
+ };📝 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.
| const copy = (text) => { copyText(text); toast.success('Copied'); }; | |
| const copy = async (text) => { | |
| const ok = await copyText(text); | |
| if (ok) toast.success('Copied'); | |
| else toast.error('Copy failed'); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/NetworkToggle.jsx` at line 51, The helper function
copy currently calls the async copyText without awaiting its Promise, causing
toast.success to show even on failure; update the copy function (the copy helper
that calls copyText and toast.success) to await copyText(text), check the
returned boolean, and only call toast.success('Copied') when it resolves true,
otherwise call toast.error(...) or another failure handler so users see correct
feedback.
| }; | ||
|
|
||
| const copy = (text) => { navigator.clipboard?.writeText(text); toast.success('Copied'); }; | ||
| const copy = (text) => { copyText(text); toast.success('Copied'); }; |
There was a problem hiding this comment.
Missing await breaks error feedback.
Same issue as NetworkToggle.jsx line 51: the helper doesn't await copyText, so the success toast fires immediately regardless of whether the copy succeeded. Users see "Copied" even when the clipboard operation fails.
🔧 Proposed fix
- const copy = (text) => { copyText(text); toast.success('Copied'); };
+ const copy = async (text) => {
+ const ok = await copyText(text);
+ if (ok) toast.success('Copied');
+ else toast.error('Copy failed');
+ };📝 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.
| const copy = (text) => { copyText(text); toast.success('Copied'); }; | |
| const copy = async (text) => { | |
| const ok = await copyText(text); | |
| if (ok) toast.success('Copied'); | |
| else toast.error('Copy failed'); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/settings/SharingPanel.jsx` at line 154, The copy
helper in SharingPanel.jsx currently calls copyText(text) without awaiting it,
so toast.success('Copied') runs even on failure; make the copy function async,
await copyText(text), and move toast.success into the success path, catching
errors to call toast.error (or show appropriate failure feedback); reference the
copy function and the copyText helper and ensure you handle promise rejection
from copyText before showing success.
| onClick: () => { | ||
| navigator.clipboard.writeText(tr.text || ''); | ||
| copyText(tr.text || ''); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Add user feedback for copy action.
The copyText call is not awaited and provides no feedback to the user. If the copy fails (e.g., in an HTTP context where the Clipboard API is unavailable), the user gets no indication. At minimum, add a success toast; ideally handle errors too.
📋 Proposed fix with feedback
- onClick: () => {
- copyText(tr.text || '');
- },
+ onClick: async () => {
+ const ok = await copyText(tr.text || '');
+ if (ok) toast.success('Copied');
+ else toast.error('Copy failed');
+ },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/pages/Projects.jsx` around lines 172 - 175, The onClick handler
currently calls copyText(tr.text || '') without awaiting or reporting result;
update the handler in Projects.jsx to await copyText(tr.text || '') and show
user feedback: on success call the project's toast/notification helper (e.g.,
toast.success or showToast) to display a "Copied" message, and on failure catch
the error and show an error toast with the error message; reference the onClick
handler and the copyText function so the change is limited to that click
callback and uses existing toast utilities in the component.
2 participants
Opening the LAN-share URL on another device showed a wall of CORS errors + a clipboard crash. Two compounding remote-only bugs:
:5050had its API client hardcoded to:3900, which is cross-origin (CORS-blocked) and loopback-only/unreachable from another machine. The share listener serves the same app+API, so the remote SPA must use its own origin._resolveApiBase: Tauri→127.0.0.1; vite-dev→:3900 (CORS-allowed); else (share listener / Docker / prod build)→window.location.origin. +6 unit tests.navigator.clipboardis secure-context-only; over plain HTTP it'sundefined, so unguardedwriteText(...)threw and crashed copy buttons. NewcopyTextutil (clipboard API → hidden-textarea/execCommandfallback), routed through all 9 call sites. +2 tests.Verified: typecheck:ci ✓, test:legacy ✓, vitest 103/103 ✓, build ✓. This is the fix that makes the (real, already-working backend) sharing usable end-to-end from a phone/second machine.
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests