feat(ui): clipboard observer, system tray & notifications#21
Merged
Conversation
…fications Add clipboard monitoring that polls the system clipboard every 500ms, extracts URLs via regex, and deduplicates them. Implement system tray with quick actions (Pause/Resume All, Clipboard toggle, Open Window, Quit) and desktop notification bridge for download completion/failure events. Add ClipboardIndicator component to StatusBar with toggle. Closes #22
github-actions
Bot
added
documentation
|
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:
📝 WalkthroughWalkthroughAdds clipboard monitoring with regex URL extraction and deduplication, system tray controls and click-to-focus behavior, desktop notifications, Tauri IPC to toggle/query clipboard monitoring, Rust adapters (clipboard/tray/notification), a frontend hook and indicator, TypeScript event types, and tests/mocks. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as ClipboardIndicator<br/>(React)
participant Hook as useClipboardMonitoring<br/>(Hook)
participant IPC as Tauri IPC
participant Backend as CommandBus
participant Observer as ClipboardObserver
participant Config as ConfigStore
User->>UI: Click toggle
UI->>Hook: toggle(!isEnabled)
Hook->>IPC: invoke("clipboard_toggle", { enabled })
IPC->>Backend: handle_toggle_clipboard(enabled)
Backend->>Config: write ConfigPatch(clipboard_monitoring=enabled)
Backend->>Observer: start() / stop()
Observer->>Observer: spawn/abort polling task (500ms)
Backend-->>IPC: return enabled
IPC-->>Hook: result boolean
Hook->>UI: update isEnabled (server-confirmed)
UI-->>User: reflect new state
sequenceDiagram
participant Sys as System<br/>Clipboard
participant Obs as ClipboardObserver<br/>(poll)
participant Regex as URL<br/>Extractor
participant Bus as Domain<br/>EventBus
participant Front as Frontend<br/>(listener)
Obs->>Sys: read clipboard
Sys-->>Obs: text
Obs->>Regex: extract URLs (dedupe)
Regex-->>Obs: new URLs
Obs->>Bus: emit ClipboardUrlDetected{urls}
Bus->>Front: emit "clipboard-url-detected"
Front->>Front: update UI / trigger downloads
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds a clipboard observer (500ms polling), system tray with menu, and a desktop notification bridge, plus frontend Two issues in
Confidence Score: 4/5Safe to merge with the understanding that tray-based clipboard toggling is non-functional until the two tray issues are fixed. Two P1 findings in system_tray.rs: the tray "Clipboard Monitoring" item emits an event no code handles (toggle is a no-op from the tray), and the CheckMenuItem is always initialized as checked regardless of persisted config. The StatusBar indicator and IPC path work correctly; only the tray interaction path is broken. src-tauri/src/adapters/driven/tray/system_tray.rs — both P1 findings are here Important Files Changed
Sequence DiagramsequenceDiagram
participant CB as Clipboard (OS)
participant CO as TauriClipboardObserver
participant EB as EventBus
participant TB as TauriBridge
participant NB as NotificationBridge
participant FE as Frontend
Note over CO: Polls every 500ms
CB-->>CO: read_text()
CO->>CO: extract_urls() + dedup
CO->>EB: publish(ClipboardUrlDetected)
EB->>TB: on_event → emit("clipboard-url-detected")
TB-->>FE: clipboard-url-detected
Note over FE: User clicks tray "Clipboard Monitoring"
FE->>FE: tray-clipboard-toggle emitted (unhandled ⚠)
Note over FE: User clicks StatusBar ClipboardIndicator
FE->>FE: toggle(enabled)
FE->>+TB: invoke("clipboard_toggle", {enabled})
TB->>CO: start() / stop()
TB->>TB: emit("clipboard-monitoring-changed")
TB-->>-FE: clipboard-monitoring-changed → setIsEnabled
Note over EB: Download finishes
EB->>NB: on_event(DownloadCompleted / DownloadFailed)
NB-->>FE: desktop notification
Reviews (1): Last reviewed commit: "feat(ui): implement clipboard observer, ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+94
to
+98
| "clipboard-toggle" => { | ||
| if let Err(e) = app.emit("tray-clipboard-toggle", ()) { | ||
| warn!("Failed to emit clipboard-toggle: {e}"); | ||
| } | ||
| } |
There was a problem hiding this comment.
"clipboard-toggle" emits tray-clipboard-toggle, but no frontend code in this PR (nor in useClipboardMonitoring) listens to that event. The TauriEventMap and the hook only handle clipboard-monitoring-changed. So clicking the tray item visually checks/unchecks the CheckMenuItem, but the actual monitoring state never changes.
The tray handler should call the backend directly (or emit clipboard-monitoring-changed after toggling) instead of emitting an unhandled intermediate event. For example, call clipboard_toggle IPC from the backend side, or use a shared AppState reference to invoke handle_toggle_clipboard.
Until this is wired to AppState, the tray toggle does nothing observable.
Comment on lines
+22
to
+29
| let clipboard_toggle = CheckMenuItem::with_id( | ||
| app, | ||
| "clipboard-toggle", | ||
| "Clipboard Monitoring", | ||
| true, // enabled | ||
| true, // checked by default | ||
| None::<&str>, | ||
| )?; |
There was a problem hiding this comment.
CheckMenuItem::with_id is constructed with checked: true unconditionally. The default clipboard_monitoring in AppConfig is false (the TauriClipboardObserver also starts with enabled = false). On every app launch the tray will show clipboard monitoring as active even when the config says otherwise, and the visual state diverges from ClipboardIndicator in the status bar.
The actual config value should be read at setup time and passed in:
let clipboard_enabled = app.state::<AppState>()
.command_bus
.config_store()
.get_config()
.map(|c| c.clipboard_monitoring)
.unwrap_or(false);
let clipboard_toggle = CheckMenuItem::with_id(
app,
"clipboard-toggle",
"Clipboard Monitoring",
true,
clipboard_enabled, // <-- use actual config value
None::<&str>,
)?;
Comment on lines
+11
to
+13
| regex::Regex::new(r"(https?://|ftp://|magnet:\?)[^\s]+") | ||
| .expect("URL regex is a compile-time constant") | ||
| }); |
There was a problem hiding this comment.
[^\s]+ is greedy and will consume trailing punctuation that is not part of the URL. For example "See https://example.com." yields "https://example.com." (trailing .), and "(https://example.com)" yields "https://example.com)". These mis-extracted strings will be added to the download queue as invalid URLs.
A common fix is to post-process matches to trim trailing .,;:!?):
fn extract_urls(text: &str) -> Vec<String> {
URL_REGEX
.find_iter(text)
.map(|m| m.as_str().trim_end_matches(|c: char| ".,;:!?)\"'".contains(c)).to_string())
.collect()
}
Comment on lines
+49
to
+53
| fn start(&self) -> Result<(), DomainError> { | ||
| // Re-entrancy guard: if already running, just ensure enabled | ||
| if self.enabled.swap(true, Ordering::SeqCst) { | ||
| return Ok(()); | ||
| } |
There was a problem hiding this comment.
stop() during rapid stop → start
start() uses swap(true) as a guard. If stop() sets enabled = false and start() is called before the spawned task observes the flag and exits the loop, two polling tasks will briefly coexist. Both share the same seen_urls and detected_urls, so they won't produce duplicates, but they will each sleep 500 ms and poll independently, effectively doubling clipboard read frequency during the overlap window.
Consider using a JoinHandle stored in the struct (or an Arc<Mutex<Option<JoinHandle<_>>>>) so a new start() can abort() the previous task before spawning a replacement.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
src-tauri/src/application/commands/toggle_clipboard.rs (1)
381-407: Add failure-path tests for toggle behavior.Current tests validate only success flows. Add cases for
clipboard_observer().start()/stop()failure andconfig_store().update_config()failure to lock in error handling and rollback behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/application/commands/toggle_clipboard.rs` around lines 381 - 407, Add tests that exercise failure paths for handle_toggle_clipboard: create command buses (via make_command_bus) configured/mocked so clipboard_observer().start() and .stop() return Err and so config_store().update_config() returns Err, then call handle_toggle_clipboard(true) and handle_toggle_clipboard(false) and assert they return Err; also assert the system rolled back/no partial change by checking clipboard_observer() state and config_store().get_config() remains unchanged. Specifically add cases where clipboard_observer().start() fails, clipboard_observer().stop() fails, and config_store().update_config() fails, referencing the existing helpers make_command_bus, handle_toggle_clipboard, clipboard_observer(), and config_store().update_config() to locate and modify mocks. Ensure each test verifies error is returned and pre-call state is preserved.src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs (1)
61-61: Usetauri::async_runtime::spawninstead oftokio::spawnfor proper runtime integration.Tauri v2 manages its own Tokio runtime. Using
tauri::async_runtime::spawnensures tasks run on the correct runtime handle and prevents panics like "there is no reactor running" that occur with directtokio::spawncalls in adapters. Official Tauri v2 documentation recommends this API for all async tasks within the app context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs` at line 61, Replace the direct call to tokio::spawn with tauri::async_runtime::spawn so the clipboard observer task runs on Tauri's runtime; in the code that currently calls tokio::spawn(async move { ... }) (the clipboard observer closure in tauri_clipboard_observer.rs), change it to tauri::async_runtime::spawn(async move { ... }) and add the tauri::async_runtime import if necessary to ensure the task is scheduled on Tauri's runtime handle.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Line 136: Update the CHANGELOG entry for useClipboardMonitoring to remove
"optimistic state updates" and instead describe its confirmation-driven
behavior: mention that state is updated on successful operations via the
onSuccess callback and synchronized through event listeners rather than
optimistic updates; reference the useClipboardMonitoring hook and its onSuccess
+ event sync behavior so readers understand the correct semantics.
In `@src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs`:
- Around line 49-65: The start/stop race can spawn duplicate polling loops
because stop() only flips enabled and a quick restart can set enabled true
before the old task exits; add a generation token (e.g., an AtomicU64 field like
generation) that's incremented in stop() and captured by start() when spawning
the task, pass that token into the async loop and have the loop periodically
compare the captured token to the current generation (or check once at top of
loop) and exit if they differ; update stop() to increment generation and clear
enabled, and update start() to read and capture the current generation before
tokio::spawn so only the latest generation's loop runs.
- Around line 67-75: The spawn_blocking call that sets clipboard_text currently
ignores JoinError; update the await handling for the tokio::task::spawn_blocking
invocation (the block that uses app_handle.clone() and
handle.clipboard().read_text()) to match on the Result from .await and log a
warning when Err(join_err) occurs so panics/cancellations are visible, then
handle the error path (e.g., treat clipboard_text as None or return early)
instead of silently proceeding; apply the same change to the other
spawn_blocking usage around lines 113-116 to consistently handle JoinError.
In `@src-tauri/src/adapters/driven/notification/notification_bridge.rs`:
- Around line 22-28: The notification currently exposes raw error text in
DomainEvent::DownloadFailed handling (see DomainEvent::DownloadFailed { id,
error } and the app_handle.notification().builder().body(...) call); change the
notification body to a generic, user-friendly message like "Download #<id>
failed" and remove the {error} from the OS-level notification, and instead log
the detailed error via your existing logger (or app_handle.log/error) so the
full error remains in logs but not shown in the desktop notification.
In `@src-tauri/src/adapters/driven/tray/system_tray.rs`:
- Around line 22-29: The tray checkbox is being created with a hardcoded checked
value; change the third boolean argument passed to CheckMenuItem::with_id
(currently the second true at "Clipboard Monitoring") to use the persisted
config value instead (e.g., read your saved setting like
config.clipboard_monitoring or app_state.get_config().clipboard_monitoring) so
clipboard_toggle is initialized from persisted state; ensure you fetch the
config before creating clipboard_toggle and pass that boolean into
CheckMenuItem::with_id, preserving the enabled flag and None::<&str> parameter.
- Around line 47-76: The TrayIcon created by TrayIconBuilder is being dropped
because the handle (_tray) isn't stored long-term; modify setup_system_tray to
persist the returned TrayIcon in application-managed state instead of letting it
fall out of scope: create or reuse a persistent holder (e.g., an AppState struct
with a field like tray: OnceLock<tray::TrayIcon> or Option<tray::TrayIcon> and
register it via app.manage(AppState::new()) or set the OnceLock at module
scope), then move the built TrayIcon into that holder (e.g.,
AppState.tray.set(tray) or store Some(tray)) rather than returning or discarding
it; update any call sites (where setup_system_tray() is invoked) to not drop the
return value and ensure the TrayIcon handle is kept alive for the app lifetime.
In `@src-tauri/src/adapters/driving/tauri_ipc.rs`:
- Around line 325-328: The call to app.emit("clipboard-monitoring-changed", ...)
currently ignores errors; change it to handle the Result from app.emit returned
by the tauri::AppHandle (the app.emit(...) expression) and log a warning if it
Errs so failures aren't silently swallowed — include the event name
"clipboard-monitoring-changed", the payload (serde_json::json!({ "enabled":
result })), and the error in the log message (use the project's logger/tracing
facility, e.g., tracing::warn! or the existing logger used elsewhere) so
listeners' failures are visible.
In `@src-tauri/src/application/commands/toggle_clipboard.rs`:
- Around line 7-16: The clipboard observer and persisted config can drift if
start()/stop() succeeds but config_store().update_config(...) fails; modify the
toggle logic so you perform the desired observer action
(clipboard_observer().start() or .stop()), then call
config_store().update_config(ConfigPatch { clipboard_monitoring: Some(enabled),
..Default::default() }) and if update_config returns an error, immediately
attempt a rollback by calling the opposite observer action (call .stop() if you
had started, or .start() if you had stopped) and return the original update
error (and if rollback itself errors, combine or surface both errors). Ensure
both the observer calls and update_config errors are propagated so callers know
failure occurred.
In `@src-tauri/src/lib.rs`:
- Around line 75-76: The clipboard IPC handlers clipboard_toggle and
clipboard_state require a managed Tauri State<AppState> but AppState is not
registered yet; fix by wiring the app state before these commands are
registered—call app.manage(your_app_state_instance) (e.g., AppState::default()
or constructed state) prior to adding/attaching the clipboard_toggle and
clipboard_state commands so State<'_, AppState> is available to those handlers.
In `@src/components/ClipboardIndicator.tsx`:
- Around line 11-23: Add an ARIA pressed state to the toggle button so assistive
tech can announce the two-state control: update the button element in
ClipboardIndicator (the JSX with onClick={() => toggle(!isEnabled)}) to include
aria-pressed={isEnabled} (ensuring it passes a boolean) so the enabled/paused
state is programmatically exposed to screen readers.
---
Nitpick comments:
In `@src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs`:
- Line 61: Replace the direct call to tokio::spawn with
tauri::async_runtime::spawn so the clipboard observer task runs on Tauri's
runtime; in the code that currently calls tokio::spawn(async move { ... }) (the
clipboard observer closure in tauri_clipboard_observer.rs), change it to
tauri::async_runtime::spawn(async move { ... }) and add the tauri::async_runtime
import if necessary to ensure the task is scheduled on Tauri's runtime handle.
In `@src-tauri/src/application/commands/toggle_clipboard.rs`:
- Around line 381-407: Add tests that exercise failure paths for
handle_toggle_clipboard: create command buses (via make_command_bus)
configured/mocked so clipboard_observer().start() and .stop() return Err and so
config_store().update_config() returns Err, then call
handle_toggle_clipboard(true) and handle_toggle_clipboard(false) and assert they
return Err; also assert the system rolled back/no partial change by checking
clipboard_observer() state and config_store().get_config() remains unchanged.
Specifically add cases where clipboard_observer().start() fails,
clipboard_observer().stop() fails, and config_store().update_config() fails,
referencing the existing helpers make_command_bus, handle_toggle_clipboard,
clipboard_observer(), and config_store().update_config() to locate and modify
mocks. Ensure each test verifies error is returned and pre-call state is
preserved.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 77f0143a-c625-430e-923b-8557d00f0b34
⛔ Files ignored due to path filters (5)
src-tauri/Cargo.lockis excluded by!**/*.locksrc-tauri/gen/schemas/acl-manifests.jsonis excluded by!**/gen/**src-tauri/gen/schemas/capabilities.jsonis excluded by!**/gen/**src-tauri/gen/schemas/desktop-schema.jsonis excluded by!**/gen/**src-tauri/gen/schemas/linux-schema.jsonis excluded by!**/gen/**
📒 Files selected for processing (27)
CHANGELOG.mdsrc-tauri/Cargo.tomlsrc-tauri/capabilities/default.jsonsrc-tauri/src/adapters/driven/clipboard/mod.rssrc-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rssrc-tauri/src/adapters/driven/event/tauri_bridge.rssrc-tauri/src/adapters/driven/mod.rssrc-tauri/src/adapters/driven/notification/mod.rssrc-tauri/src/adapters/driven/notification/notification_bridge.rssrc-tauri/src/adapters/driven/tray/mod.rssrc-tauri/src/adapters/driven/tray/system_tray.rssrc-tauri/src/adapters/driving/tauri_ipc.rssrc-tauri/src/application/commands/mod.rssrc-tauri/src/application/commands/toggle_clipboard.rssrc-tauri/src/domain/event.rssrc-tauri/src/lib.rssrc/components/ClipboardIndicator.tsxsrc/components/__tests__/ClipboardIndicator.test.tsxsrc/hooks/__tests__/useClipboardMonitoring.test.tssrc/hooks/useClipboardMonitoring.tssrc/layouts/StatusBar.tsxsrc/layouts/__tests__/AppLayout.test.tsxsrc/layouts/__tests__/StatusBar.test.tsxsrc/test/__mocks__/tauri.tssrc/test/setup.tssrc/types/events.tsvite.config.ts
Comment on lines
+47
to
+76
| let _tray = TrayIconBuilder::new() | ||
| .icon( | ||
| app.default_window_icon() | ||
| .cloned() | ||
| .ok_or("app must have a default icon configured in tauri.conf.json")?, | ||
| ) | ||
| .menu(&menu) | ||
| .show_menu_on_left_click(false) | ||
| .tooltip("Vortex Download Manager") | ||
| .on_menu_event(move |app, event| { | ||
| handle_tray_menu_event(app, event.id().as_ref()); | ||
| }) | ||
| .on_tray_icon_event(|tray, event| { | ||
| use tauri::tray::{MouseButton, MouseButtonState, TrayIconEvent}; | ||
| if let TrayIconEvent::Click { | ||
| button: MouseButton::Left, | ||
| button_state: MouseButtonState::Up, | ||
| .. | ||
| } = event | ||
| { | ||
| let app = tray.app_handle(); | ||
| if let Some(window) = app.get_webview_window("main") { | ||
| let _ = window.unminimize(); | ||
| let _ = window.show(); | ||
| let _ = window.set_focus(); | ||
| } | ||
| } | ||
| }) | ||
| .build(app)?; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Tauri v2, does dropping the TrayIconreturned byTrayIconBuilder::build(app) remove/unregister the tray icon? What is the recommended lifetime management pattern?
💡 Result:
In Tauri v2, dropping the TrayIcon returned by TrayIconBuilder::build(app) removes/unregisters the tray icon from the system tray. The recommended lifetime management pattern is to store the TrayIcon in a location that keeps it alive for the duration of the app, such as within the app's setup closure or a struct that outlives the event loop (e.g., assigned to a variable in the scope where app.run is called). This ensures the icon persists until explicitly dropped or the app exits. Dropping it prematurely (e.g., at the end of a limited scope) will cause the icon to disappear, as seen in tray-icon library behavior where the icon must remain alive alongside the event loop.
Citations:
- 1: https://github.com/tauri-apps/tray-icon/
- 2: Tray Icon fails to create when inside curly braces tauri-apps/tray-icon#256
- 3: https://docs.rs/tray-icon/latest/tray_icon/struct.TrayIconBuilder.html
- 4: https://github.com/tauri-apps/tray-icon/blob/dev/examples/tao.rs
- 5: https://docs.rs/tauri/latest/tauri/tray/struct.TrayIconBuilder.html
- 6: https://docs.rs/tauri/latest/tauri/tray/struct.TrayIcon.html
🏁 Script executed:
#!/bin/bash
# Find calls to setup_system_tray
rg "setup_system_tray" --type rs -B 2 -A 2Repository: mpiton/vortex
Length of output: 83
🏁 Script executed:
#!/bin/bash
# Also check the function signature in the file itself
cat -n src-tauri/src/adapters/driven/tray/system_tray.rs | head -60Repository: mpiton/vortex
Length of output: 2401
🏁 Script executed:
#!/bin/bash
# Find all calls to setup_system_tray
rg "setup_system_tray" --glob "*.rs" -B 2 -A 2Repository: mpiton/vortex
Length of output: 1424
🏁 Script executed:
#!/bin/bash
# Get context around the setup_system_tray call in lib.rs
cat -n src-tauri/src/lib.rs | sed -n '1,150p' | tail -80Repository: mpiton/vortex
Length of output: 3990
The tray icon must be stored in an app-managed structure, not just returned from setup.
Dropping _tray at the end of setup_system_tray removes the icon from the system tray. Tauri v2 keeps the icon alive only while the TrayIcon handle exists. The proposed fix of returning the TrayIcon from the function is incomplete because the call site in lib.rs line 47–49 discards the return value. The icon would still be dropped immediately.
Instead, store the TrayIcon in a structure that persists for the app's lifetime (e.g., AppState managed via app.manage()), or use an OnceLock at module scope to hold it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/adapters/driven/tray/system_tray.rs` around lines 47 - 76, The
TrayIcon created by TrayIconBuilder is being dropped because the handle (_tray)
isn't stored long-term; modify setup_system_tray to persist the returned
TrayIcon in application-managed state instead of letting it fall out of scope:
create or reuse a persistent holder (e.g., an AppState struct with a field like
tray: OnceLock<tray::TrayIcon> or Option<tray::TrayIcon> and register it via
app.manage(AppState::new()) or set the OnceLock at module scope), then move the
built TrayIcon into that holder (e.g., AppState.tray.set(tray) or store
Some(tray)) rather than returning or discarding it; update any call sites (where
setup_system_tray() is invoked) to not drop the return value and ensure the
TrayIcon handle is kept alive for the app lifetime.
Comment on lines
+75
to
+76
| clipboard_toggle, | ||
| clipboard_state, |
There was a problem hiding this comment.
Clipboard IPC commands are exposed before AppState is wired.
clipboard_toggle and clipboard_state require State<'_, AppState>, but the app setup still does not call app.manage(...) (see context around Line 51). These commands will fail at runtime until state wiring is in place.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/lib.rs` around lines 75 - 76, The clipboard IPC handlers
clipboard_toggle and clipboard_state require a managed Tauri State<AppState> but
AppState is not registered yet; fix by wiring the app state before these
commands are registered—call app.manage(your_app_state_instance) (e.g.,
AppState::default() or constructed state) prior to adding/attaching the
clipboard_toggle and clipboard_state commands so State<'_, AppState> is
available to those handlers.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
8 issues found across 32 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="src-tauri/src/adapters/driven/tray/system_tray.rs">
<violation number="1" location="src-tauri/src/adapters/driven/tray/system_tray.rs:27">
P2: The tray "Clipboard Monitoring" check state is hardcoded to checked and can drift from persisted config on startup.</violation>
<violation number="2" location="src-tauri/src/adapters/driven/tray/system_tray.rs:85">
P1: Tray menu actions emit events that have no handlers, so Pause/Resume/Clipboard toggle are effectively no-ops.</violation>
</file>
<file name="src-tauri/capabilities/default.json">
<violation number="1" location="src-tauri/capabilities/default.json:9">
P1: `clipboard-manager:allow-read-text` exposes clipboard-read IPC to the main webview even though clipboard reads are performed in Rust. This unnecessarily increases impact if renderer code is compromised.</violation>
<violation number="2" location="src-tauri/capabilities/default.json:10">
P2: `notification:default` grants a full notification command surface to the main webview. Use a narrower permission to reduce unnecessary IPC exposure.</violation>
</file>
<file name="src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs">
<violation number="1" location="src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs:51">
P1: Rapid stop/start can create multiple concurrent clipboard polling tasks because the old spawned loop is not synchronized with restart.</violation>
<violation number="2" location="src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs:93">
P2: `MAX_SEEN_URLS` is not actually enforced during insertion, so `seen_urls` can grow past the intended limit.</violation>
</file>
<file name="src-tauri/src/lib.rs">
<violation number="1" location="src-tauri/src/lib.rs:75">
P1: `clipboard_toggle`/`clipboard_state` are exposed before `AppState` is managed, so invoking them will fail at runtime due to missing `State<AppState>`.</violation>
</file>
<file name="src-tauri/src/application/commands/toggle_clipboard.rs">
<violation number="1" location="src-tauri/src/application/commands/toggle_clipboard.rs:7">
P2: Runtime observer state is changed before config persistence, so a config write failure can leave runtime and persisted clipboard-monitoring state inconsistent.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Fix URL regex to strip trailing punctuation (`.`, `)`, `;`, etc.) - Add JoinHandle-based task cancellation to prevent duplicate polling loops on rapid stop→start cycles - Handle spawn_blocking join errors explicitly - Persist config before toggling observer to avoid state drift, with rollback on observer failure - Sanitize/truncate error messages in desktop notifications (100 chars) - Initialize tray CheckMenuItem from config state, not hardcoded true - Wire tray menu actions (Pause/Resume All, Clipboard toggle) through AppState when available, with graceful fallback when not yet wired - Remove clipboard-manager:allow-read-text capability (reads are Rust-only, no webview IPC needed) - Narrow notification permissions to allow-notify, request-permission, is-permission-granted - Log event emission failures instead of silently discarding - Add aria-pressed attribute for accessibility on ClipboardIndicator - Fix CHANGELOG wording (server-confirmed, not optimistic) - Enforce MAX_SEEN_URLS with >= instead of > for correct cap
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src-tauri/src/adapters/driven/notification/notification_bridge.rs (1)
22-31:⚠️ Potential issue | 🟠 MajorDon’t include failure text in OS notification body.
Even truncated, this can still leak sensitive details. Keep user-facing text generic and log full details internally.
Proposed fix
DomainEvent::DownloadFailed { id, error } => { - // Truncate and sanitize error to avoid leaking internal details - // (paths, credentials) in desktop notifications visible on lock screen - let safe_error: String = error.chars().take(100).collect(); + warn!(download_id = id.0, error = %error, "download failed"); if let Err(e) = app_handle .notification() .builder() .title("Download Failed") - .body(format!("Download #{}: {safe_error}", id.0)) + .body(format!("Download #{} failed", id.0)) .show() { warn!("Failed to show error notification: {e}"); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/adapters/driven/notification/notification_bridge.rs` around lines 22 - 31, The notification currently includes (even truncated) error text in DomainEvent::DownloadFailed handling; change the notification.body to a generic message like "A download failed. Check the application for details" and remove inclusion of safe_error, while logging the full error internally (e.g., via the existing logger/tracing) instead of displaying it; update the code around app_handle.notification().builder() in the DomainEvent::DownloadFailed match arm to show only the generic text and emit the full error to the internal log.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src-tauri/src/adapters/driven/notification/notification_bridge.rs`:
- Around line 22-31: The notification currently includes (even truncated) error
text in DomainEvent::DownloadFailed handling; change the notification.body to a
generic message like "A download failed. Check the application for details" and
remove inclusion of safe_error, while logging the full error internally (e.g.,
via the existing logger/tracing) instead of displaying it; update the code
around app_handle.notification().builder() in the DomainEvent::DownloadFailed
match arm to show only the generic text and emit the full error to the internal
log.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bd7852f0-c621-46d6-b322-08143b28ed91
⛔ Files ignored due to path filters (1)
src-tauri/gen/schemas/capabilities.jsonis excluded by!**/gen/**
📒 Files selected for processing (9)
CHANGELOG.mdsrc-tauri/capabilities/default.jsonsrc-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rssrc-tauri/src/adapters/driven/notification/notification_bridge.rssrc-tauri/src/adapters/driven/tray/system_tray.rssrc-tauri/src/adapters/driving/tauri_ipc.rssrc-tauri/src/application/commands/toggle_clipboard.rssrc-tauri/src/lib.rssrc/components/ClipboardIndicator.tsx
✅ Files skipped from review due to trivial changes (2)
- src/components/ClipboardIndicator.tsx
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (5)
- src-tauri/src/adapters/driving/tauri_ipc.rs
- src-tauri/src/lib.rs
- src-tauri/src/adapters/driven/tray/system_tray.rs
- src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs
- src-tauri/src/application/commands/toggle_clipboard.rs
There was a problem hiding this comment.
2 issues found across 10 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="src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs">
<violation number="1" location="src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs:14">
P2: The URL regex strips a trailing `]`, which can truncate valid IPv6 host URLs (for example `http://[::1]`) into invalid results.</violation>
</file>
<file name="src-tauri/src/adapters/driven/notification/notification_bridge.rs">
<violation number="1" location="src-tauri/src/adapters/driven/notification/notification_bridge.rs:25">
P2: This line truncates but does not sanitize, so sensitive error details can still be exposed in notifications.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Remove `]` from URL regex exclusion set to preserve IPv6 host URLs (e.g. http://[::1]/path) - Replace truncated error messages in notifications with generic text to fully avoid leaking sensitive details; log full error for debugging - Add IPv6 URL extraction test
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs (1)
124-127: Consider bounded eviction instead of fullseenreset.Clearing the entire set at capacity can re-emit many previously seen URLs at once. A FIFO/LRU-style bounded eviction keeps dedupe behavior smoother.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs` around lines 124 - 127, Currently the code calls seen.clear() when seen.len() >= MAX_SEEN_URLS which can re-emit many URLs; replace this full reset with bounded eviction by tracking insertion order and removing only the oldest entries when at capacity. Concretely, keep the existing HashSet seen but add an ordered structure (e.g., a VecDeque seen_queue or use lru::LruCache) and on insert of a new URL, if seen.len() >= MAX_SEEN_URLS pop from seen_queue and remove that value from seen until size < MAX_SEEN_URLS, then push the new URL into seen_queue and insert into seen; update the code around the current seen.clear() location and references to seen to use both structures (or swap to LruCache) to implement FIFO/LRU eviction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs`:
- Around line 11-12: Remove the duplicated rustdoc sentence "Strips common
trailing punctuation that is not part of the URL." so only one instance remains
in the documentation comment for the clipboard URL-stripping logic (the rustdoc
block in tauri_clipboard_observer.rs that describes stripping trailing
punctuation from URLs). Leave a single copy of the sentence and ensure
surrounding punctuation and formatting of the doc comment stays intact.
- Line 15: The regex in the Regex::new call currently uses two consecutive
character class matches ([^\s]+[^\s.,)>;:'\x22]) which forces at least two
characters after the scheme and misses shortest URLs like "http://a"; update the
pattern used in Regex::new (the Regex::new(...) invocation) to a single
character class that excludes whitespace and trailing punctuation with a +
quantifier (so it can match one or more allowed characters) — i.e., replace the
two-part sequence with one class like [^\s.,)>;:'"\x22]+ (properly escaped) so
single-character hosts after the scheme are matched.
---
Nitpick comments:
In `@src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs`:
- Around line 124-127: Currently the code calls seen.clear() when seen.len() >=
MAX_SEEN_URLS which can re-emit many URLs; replace this full reset with bounded
eviction by tracking insertion order and removing only the oldest entries when
at capacity. Concretely, keep the existing HashSet seen but add an ordered
structure (e.g., a VecDeque seen_queue or use lru::LruCache) and on insert of a
new URL, if seen.len() >= MAX_SEEN_URLS pop from seen_queue and remove that
value from seen until size < MAX_SEEN_URLS, then push the new URL into
seen_queue and insert into seen; update the code around the current seen.clear()
location and references to seen to use both structures (or swap to LruCache) to
implement FIFO/LRU eviction.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 09eb508c-94c4-4974-b51b-787e5e5af04f
📒 Files selected for processing (2)
src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rssrc-tauri/src/adapters/driven/notification/notification_bridge.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src-tauri/src/adapters/driven/notification/notification_bridge.rs
There was a problem hiding this comment.
1 issue found across 2 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="src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs">
<violation number="1" location="src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs:15">
P2: The updated URL regex now accepts trailing `]`, so URLs wrapped in brackets can be extracted with a stray closing bracket (e.g. `https://example.com]`), producing malformed URLs.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Remove duplicated rustdoc comment on URL_REGEX - Change regex quantifier from + to * to match single-char hosts - Restore ] in regex exclusion set to prevent stray brackets in extracted URLs (e.g. markdown [link](url) patterns) - Add tests for short host URLs and bracket stripping
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs (1)
53-63:⚠️ Potential issue | 🔴 CriticalSerialize lifecycle transitions; current start/stop interleaving can lose the active task.
Line 152 flips
enabledbefore Line 155 acquirestask_handle, while Line 55 instart()checks/setsenabledbefore Line 60 acquires that same lock. A concurrentstop()+start()can abort the newly spawned task and leaveenabledout of sync with reality. Please make start/stop atomic as one transition (or reintroduce a generation token guard).Suggested fix (generation token guard)
-use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; @@ pub struct TauriClipboardObserver { @@ task_handle: Mutex<Option<JoinHandle<()>>>, + generation: Arc<AtomicU64>, } @@ last_content: Arc::new(Mutex::new(String::new())), task_handle: Mutex::new(None), + generation: Arc::new(AtomicU64::new(0)), } } @@ if self.enabled.swap(true, Ordering::SeqCst) { return Ok(()); } + let generation = Arc::clone(&self.generation); + let my_generation = generation.fetch_add(1, Ordering::SeqCst) + 1; @@ let handle = tokio::spawn(async move { loop { if !enabled.load(Ordering::SeqCst) { break; } + if generation.load(Ordering::SeqCst) != my_generation { + break; + } @@ fn stop(&self) -> Result<(), DomainError> { + self.generation.fetch_add(1, Ordering::SeqCst); self.enabled.store(false, Ordering::SeqCst);Also applies to: 151-158
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs` around lines 53 - 63, The start/stop race occurs because start() flips the AtomicBool enabled before grabbing task_handle, allowing stop() to abort a newly spawned task and leave enabled inconsistent; fix by making the lifecycle transition atomic: either (A) move the enabled.store(true, Ordering::SeqCst) to after acquiring the task_handle lock (and only set it once the new task has been stored), or (B) implement a generation token (e.g., an increasing u64 stored alongside task_handle) that start() captures and the spawned clipboard task checks before registering itself and when stop() increments/invalidates the token and aborts the handle; update start(), stop(), enabled and task_handle usage so the spawned task only becomes "active" if the token/lock aligns, preventing stop() from aborting a subsequently started task.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs`:
- Around line 12-14: The URL extraction regex defined as URL_REGEX is currently
excluding the closing bracket ] which causes IPv6 URLs like "http://[::1]" to
lose their trailing ] — update the regex pattern used in URL_REGEX (and the
duplicate pattern found around the other occurrence) to allow a terminal ']' to
be part of the match while still trimming surrounding punctuation, and add a
unit test that asserts "http://[::1]" is extracted whole; locate the symbol
URL_REGEX in tauri_clipboard_observer.rs (and the duplicate regex at the later
occurrence) and modify the terminal character class so it does not strip the
IPv6 closing bracket, then add a concise test case ensuring full bracketed IPv6
host URLs are preserved.
---
Duplicate comments:
In `@src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs`:
- Around line 53-63: The start/stop race occurs because start() flips the
AtomicBool enabled before grabbing task_handle, allowing stop() to abort a newly
spawned task and leave enabled inconsistent; fix by making the lifecycle
transition atomic: either (A) move the enabled.store(true, Ordering::SeqCst) to
after acquiring the task_handle lock (and only set it once the new task has been
stored), or (B) implement a generation token (e.g., an increasing u64 stored
alongside task_handle) that start() captures and the spawned clipboard task
checks before registering itself and when stop() increments/invalidates the
token and aborts the handle; update start(), stop(), enabled and task_handle
usage so the spawned task only becomes "active" if the token/lock aligns,
preventing stop() from aborting a subsequently started task.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b119b72d-133a-4c88-8b7d-71c0190c97e9
📒 Files selected for processing (1)
src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs
There was a problem hiding this comment.
1 issue found across 1 file (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="src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs">
<violation number="1" location="src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs:13">
P2: The updated URL regex breaks valid host-only IPv6 URLs by stripping the closing `]` (e.g. `http://[::1]` becomes `http://[::1`).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Replace complex terminal character class in regex with greedy \S+ match plus a post-processing step (strip_trailing_punctuation) that handles bracket-aware stripping: ] is only removed when no matching [ exists in the URL, preserving IPv6 hosts like http://[::1] while still stripping wrapper brackets from [https://example.com].
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs`:
- Around line 155-159: The detected_urls buffer (locked in detected_urls) is
extended with new_urls indefinitely causing unbounded growth; modify the
clipboard observer code (around detected_urls, new_urls, get_urls(), and
seen_urls) so after extending the buffer you enforce a capacity cap and
deduplicate against seen_urls: introduce a MAX_DETECTED_URLS constant, extend
buffer with only URLs not already in seen_urls, then if buffer.len() >
MAX_DETECTED_URLS truncate/retain the newest MAX_DETECTED_URLS entries (or use a
VecDeque and pop_front until size <= MAX_DETECTED_URLS); ensure this logic runs
while holding the detected_urls lock so growth is bounded.
- Around line 44-49: The length-based filter in extract_urls (the .filter(|u|
u.len() > "http://".len())) is incorrectly dropping short but valid schemes like
ftp://a; remove that filter so URL_REGEX.find_iter(...).map(...).collect()
returns all matches (after applying Self::strip_trailing_punctuation) because
the regex already enforces at least one non-whitespace char after the scheme;
update the body of extract_urls to rely on URL_REGEX and
strip_trailing_punctuation only.
- Around line 54-64: strip_trailing_punctuation incorrectly keeps an extra ']'
for inputs like "http://[::1]]" because the b']' branch only checks
url[..end].contains('['); update the logic in strip_trailing_punctuation to
strip a trailing ']' only when it is unmatched by comparing counts (or
positions) of '[' vs ']' in the substring url[..end] — e.g., replace the current
b']' arm with a check that counts '[' and ']' in &url[..end] and decrements end
when ']' count > '[' count (so unmatched trailing ']' get removed) while
preserving a single closing bracket when properly matched (use the existing url
and end variables and the strip_trailing_punctuation function name to locate and
modify this behavior).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0ed5e544-ac43-4d70-a166-1e2d687816e9
📒 Files selected for processing (1)
src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs
Comment on lines
+54
to
+64
| fn strip_trailing_punctuation(url: &str) -> &str { | ||
| let bytes = url.as_bytes(); | ||
| let mut end = bytes.len(); | ||
| while end > 0 { | ||
| match bytes[end - 1] { | ||
| b'.' | b',' | b')' | b';' | b':' | b'>' | b'\'' | b'"' => { | ||
| end -= 1; | ||
| } | ||
| b']' if !url[..end].contains('[') => { | ||
| end -= 1; | ||
| } |
There was a problem hiding this comment.
Trailing ] logic fails for wrapped IPv6 URLs with an extra bracket.
Line 62 only checks whether [ exists anywhere, so http://[::1]] keeps both closing brackets. This breaks the “strip wrapper punctuation” behavior for IPv6-in-wrapper cases.
Proposed fix
- b']' if !url[..end].contains('[') => {
- end -= 1;
- }
+ b']' => {
+ let candidate = &url[..end];
+ let open = candidate.bytes().filter(|b| *b == b'[').count();
+ let close = candidate.bytes().filter(|b| *b == b']').count();
+ if close > open {
+ end -= 1; // strip unmatched wrapper bracket
+ } else {
+ break;
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs` around
lines 54 - 64, strip_trailing_punctuation incorrectly keeps an extra ']' for
inputs like "http://[::1]]" because the b']' branch only checks
url[..end].contains('['); update the logic in strip_trailing_punctuation to
strip a trailing ']' only when it is unmatched by comparing counts (or
positions) of '[' vs ']' in the substring url[..end] — e.g., replace the current
b']' arm with a check that counts '[' and ']' in &url[..end] and decrements end
when ']' count > '[' count (so unmatched trailing ']' get removed) while
preserving a single closing bracket when properly matched (use the existing url
and end variables and the strip_trailing_punctuation function name to locate and
modify this behavior).
Comment on lines
+155
to
+159
| if !new_urls.is_empty() { | ||
| debug!(count = new_urls.len(), "clipboard: new URLs detected"); | ||
| let mut buffer = detected_urls.lock().unwrap(); | ||
| buffer.extend(new_urls); | ||
| } |
There was a problem hiding this comment.
Detected URL buffer can grow without bound.
Line 158 appends indefinitely. If get_urls() is delayed or not wired in some flows, memory usage can keep increasing even though seen_urls is capped.
Proposed fix
const MAX_SEEN_URLS: usize = 1000;
+const MAX_DETECTED_URLS: usize = 1000;
const MAX_CLIPBOARD_LEN: usize = 1_000_000;
@@
if !new_urls.is_empty() {
debug!(count = new_urls.len(), "clipboard: new URLs detected");
let mut buffer = detected_urls.lock().unwrap();
+ if buffer.len() + new_urls.len() > MAX_DETECTED_URLS {
+ let overflow = buffer.len() + new_urls.len() - MAX_DETECTED_URLS;
+ let drop_n = overflow.min(buffer.len());
+ buffer.drain(0..drop_n);
+ }
buffer.extend(new_urls);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs` around
lines 155 - 159, The detected_urls buffer (locked in detected_urls) is extended
with new_urls indefinitely causing unbounded growth; modify the clipboard
observer code (around detected_urls, new_urls, get_urls(), and seen_urls) so
after extending the buffer you enforce a capacity cap and deduplicate against
seen_urls: introduce a MAX_DETECTED_URLS constant, extend buffer with only URLs
not already in seen_urls, then if buffer.len() > MAX_DETECTED_URLS
truncate/retain the newest MAX_DETECTED_URLS entries (or use a VecDeque and
pop_front until size <= MAX_DETECTED_URLS); ensure this logic runs while holding
the detected_urls lock so growth is bounded.
There was a problem hiding this comment.
2 issues found across 1 file (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="src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs">
<violation number="1" location="src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs:48">
P2: The fixed-length filter drops valid short URLs for non-HTTP schemes (for example `ftp://a`).</violation>
<violation number="2" location="src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs:62">
P2: The `]` trimming condition is too broad and preserves unmatched trailing `]` whenever `[` exists anywhere in the URL.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| b'.' | b',' | b')' | b';' | b':' | b'>' | b'\'' | b'"' => { | ||
| end -= 1; | ||
| } | ||
| b']' if !url[..end].contains('[') => { |
There was a problem hiding this comment.
P2: The ] trimming condition is too broad and preserves unmatched trailing ] whenever [ exists anywhere in the URL.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src-tauri/src/adapters/driven/clipboard/tauri_clipboard_observer.rs, line 62:
<comment>The `]` trimming condition is too broad and preserves unmatched trailing `]` whenever `[` exists anywhere in the URL.</comment>
<file context>
@@ -44,9 +44,29 @@ impl TauriClipboardObserver {
+ b'.' | b',' | b')' | b';' | b':' | b'>' | b'\'' | b'"' => {
+ end -= 1;
+ }
+ b']' if !url[..end].contains('[') => {
+ end -= 1;
+ }
</file context>
Suggested change
| b']' if !url[..end].contains('[') => { | |
| b']' if url[..end].matches(']').count() > url[..end].matches('[').count() => { |
The regex already requires at least one non-whitespace char after the scheme, making the http:// length filter unnecessary. It also incorrectly dropped valid short FTP URLs like ftp://a.
This was referenced Apr 11, 2026
This was referenced Apr 25, 2026
This was referenced May 2, 2026
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
1 participant
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
tauri-plugin-clipboard-manager, extracts URLs (http/https/ftp/magnet), deduplicates with seen-set (capped at 1000 entries), re-entrancy guard onstart()DownloadCompletedandDownloadFailedviatauri-plugin-notificationclipboard_toggleIPC command with config persistence andclipboard-monitoring-changedevent emissionClipboardIndicatorin StatusBar with toggle,useClipboardMonitoringhook with async store syncTest plan
cargo test --workspace— 350 tests pass, 0 warningsnpx vitest run— 253 tests pass (8 new: 4 ClipboardIndicator + 4 useClipboardMonitoring)npx tsc --noEmit— TypeScript strict cleancargo clippy -- -D warnings— 0 warningsKnown gaps (deferred)
get_urls()dispatch loop: added when AppState is wired (task-16 TODO)Summary by cubic
Adds clipboard monitoring, a system tray, and desktop notifications to improve background control and feedback. Implements Linear #22 by auto-detecting copied URLs (bracket-aware trimming preserves IPv6 like http://[::1], supports single‑char and short hosts like ftp://a), quick tray actions, and reliable completion/failure alerts.
New Features
ClipboardUrlDetected; removed redundant length filter so short FTP URLs are properly detected; JoinHandle-based cancellation prevents duplicate loops.AppStatewhen available.clipboard_toggle(persist-before-toggle with rollback on failure) andclipboard_state; emitsclipboard-monitoring-changed.DownloadCompletedandDownloadFailedwith generic failure text; full error is logged.ClipboardIndicatorin StatusBar with toggle andaria-pressed;useClipboardMonitoringhook; tests and Tauri API mocks added (IPv6, short-host, bracket-stripping).Dependencies
taurifeaturestray-iconandimage-png; addtauri-plugin-clipboard-manager,tauri-plugin-notification, andregex. Update capabilities to includecore:tray:defaultand notification permissions (notification:allow-notify,notification:allow-request-permission,notification:allow-is-permission-granted).Written for commit 954855f. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests
Documentation