Auto dismiss notification when main window focused#1488
Conversation
📝 WalkthroughWalkthroughAdds macOS notification lifecycle callbacks (confirm/dismiss) with FFI bridges and a dismiss-all API; wires handlers into the Tauri notification plugin and starts/stops analytics tasks; modifies listener Joiner to synthesize zero-filled pairs and warn on queue overflow; stores a silence-stream sender in SourceState; audio test now writes buffered samples to a WAV file; adds tracing/logging. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App
participant NotificationAPI as notification::*
participant MacOSFFI as notification-macos (Rust)
participant SwiftBridge as Swift lib
participant Handlers as Registered Handlers
rect rgb(240,248,255)
App->>NotificationAPI: show(...)
NotificationAPI->>MacOSFFI: show(...)
MacOSFFI->>SwiftBridge: display notification
end
rect rgb(245,255,240)
SwiftBridge-->>MacOSFFI: rust_on_notification_confirm(id)
MacOSFFI->>Handlers: invoke confirm handler(id)
end
rect rgb(255,248,240)
SwiftBridge-->>MacOSFFI: rust_on_notification_dismiss(id)
MacOSFFI->>Handlers: invoke dismiss handler(id)
end
note over App,SwiftBridge: Dismiss-all path
App->>NotificationAPI: clear()
NotificationAPI->>MacOSFFI: dismiss_all()
MacOSFFI->>SwiftBridge: _dismissAllNotifications()
sequenceDiagram
autonumber
participant MicQ as Mic Queue
participant SpkQ as Spk Queue
participant Joiner
participant Processor
participant Downstream as SessionEvent Consumer
MicQ-->>Joiner: push(mic chunk)
SpkQ-->>Joiner: push(spk chunk)
alt both empty
Joiner-->>Processor: None
else one empty
Joiner->>Joiner: synthesize zero-filled counterpart
Joiner-->>Processor: (present, zeros)
else both present
Joiner-->>Processor: (mic, spk)
end
Processor->>Processor: log mic_sum, spk_sum
Processor-->>Downstream: emit SessionEvent(mic, spk)
opt overflow
Joiner->>Joiner: warn and trim queue front (>10)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🧹 Nitpick comments (6)
plugins/listener/src/actors/processor.rs (3)
132-135: Use a real amplitude metric (RMS) and lower log level to avoid noisy logs.Sum trends to ~0 for typical audio and logs at 10 Hz are chatty at info.
[sort: keep existing error handling per guidelines]Apply this diff:
- let mic_sum = mic_data.iter().sum::<f32>(); - let spk_sum = spk_data.iter().sum::<f32>(); - tracing::info!("mic_sum: {} spk_sum: {}", mic_sum, spk_sum); + let mic_rms = (mic_data.iter().map(|x| x * x).sum::<f32>() / mic_data.len() as f32).sqrt(); + let spk_rms = (spk_data.iter().map(|x| x * x).sum::<f32>() / spk_data.len() as f32).sqrt(); + tracing::debug!("mic_rms: {:.4} spk_rms: {:.4}", mic_rms, spk_rms);
161-163: Demote overflow log level or it will spam under sustained load.At capacity, this warns every push; the queue is capped anyway.
Apply this diff:
- tracing::warn!("mic_queue_overflow"); + tracing::debug!("mic_queue_overflow");
169-171: Same for speaker queue overflow logging.Apply this diff:
- tracing::warn!("spk_queue_overflow"); + tracing::debug!("spk_queue_overflow");crates/audio/src/mixed/macos.rs (1)
319-337: Consider removing or conditionalizing test WAV output.The test now writes audio output to
./out.wavin the current directory. This could cause issues in CI environments or when running tests in read-only directories.Consider wrapping this in a conditional check or writing to a temporary directory:
- { + if std::env::var("SAVE_TEST_AUDIO").is_ok() { let sample_rate = stream.sample_rate(); - let mut writer = hound::WavWriter::create( - "./out.wav", + let output_path = std::env::temp_dir().join("test_audio_out.wav"); + let mut writer = hound::WavWriter::create( + output_path, hound::WavSpec { channels: 1, sample_rate, bits_per_sample: 32, sample_format: hound::SampleFormat::Float, }, ) .unwrap(); for sample in buffer { writer.write_sample(sample).unwrap(); } writer.finalize().unwrap(); }crates/notification-macos/src/lib.rs (1)
35-59: Verify null-pointer safety in FFI callbacks (optional).Swift declares rustOnNotificationConfirm and rustOnNotificationDismiss with UnsafePointer (non-optional) in crates/notification-macos/swift-lib/src/lib.swift, so nulls are unlikely; still add defensive null checks in crates/notification-macos/src/lib.rs before calling CStr::from_ptr:
pub extern "C" fn rust_on_notification_confirm(id_ptr: *const c_char) { + if id_ptr.is_null() { return; } if let Some(cb) = CONFIRM_CB.lock().unwrap().as_ref() { let id = unsafe { CStr::from_ptr(id_ptr) } .to_str() .unwrap() .to_string(); cb(id); } } pub extern "C" fn rust_on_notification_dismiss(id_ptr: *const c_char, reason_ptr: *const c_char) { + if id_ptr.is_null() || reason_ptr.is_null() { return; } if let Some(cb) = DISMISS_CB.lock().unwrap().as_ref() { let id = unsafe { CStr::from_ptr(id_ptr) } .to_str() .unwrap() .to_string(); let reason = unsafe { CStr::from_ptr(reason_ptr) } .to_str() .unwrap() .to_string(); cb(id, reason); } }crates/notification-macos/swift-lib/src/lib.swift (1)
363-363: Consider more specific dismissal reasons for better analytics.While "programmatic" is functional, consider using more specific reasons:
- Line 363: Could use "api_dismiss" or "single_dismiss"
- Line 368: Could use "api_dismiss_all" or "clear_all"
- Line 390: "limit" is already specific and good
This would provide better insights into how the programmatic dismissals are triggered.
Also applies to: 368-368, 390-390
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
crates/audio/src/mixed/macos.rs(2 hunks)crates/notification-macos/examples/test_notification.rs(1 hunks)crates/notification-macos/src/lib.rs(2 hunks)crates/notification-macos/swift-lib/src/lib.swift(8 hunks)crates/notification/src/lib.rs(1 hunks)plugins/listener/src/actors/processor.rs(2 hunks)plugins/listener/src/actors/source.rs(2 hunks)plugins/notification/src/ext.rs(2 hunks)plugins/notification/src/handler.rs(2 hunks)plugins/notification/src/lib.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
plugins/notification/src/handler.rscrates/notification/src/lib.rsplugins/listener/src/actors/processor.rsplugins/notification/src/lib.rsplugins/notification/src/ext.rscrates/notification-macos/examples/test_notification.rsplugins/listener/src/actors/source.rscrates/audio/src/mixed/macos.rscrates/notification-macos/src/lib.rs
🧬 Code graph analysis (8)
plugins/notification/src/handler.rs (1)
crates/notification/src/lib.rs (2)
clear(44-46)clear(52-52)
crates/notification/src/lib.rs (2)
crates/notification-macos/src/lib.rs (2)
dismiss_all(76-80)show(61-74)crates/notification-macos/swift-lib/src/lib.swift (1)
show(346-357)
plugins/notification/src/lib.rs (1)
plugins/windows/src/ext.rs (1)
from_str(49-68)
plugins/notification/src/ext.rs (1)
crates/notification/src/lib.rs (2)
clear(44-46)clear(52-52)
crates/notification-macos/swift-lib/src/lib.swift (1)
crates/notification-macos/examples/test_notification.rs (1)
main(34-71)
crates/notification-macos/examples/test_notification.rs (1)
crates/notification-macos/src/lib.rs (2)
setup_notification_confirm_handler(28-33)setup_notification_dismiss_handler(21-26)
plugins/listener/src/actors/source.rs (1)
crates/audio/src/lib.rs (2)
silence(48-63)get_default_mic_name(82-90)
crates/audio/src/mixed/macos.rs (1)
crates/audio/src/mixed/mod.rs (2)
sample_rate(54-63)stream(28-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci (macos, macos-14)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (24)
plugins/listener/src/actors/processor.rs (1)
87-95: AGC used in Mixed path looks swapped — confirm intent.You process Mixed with
agc_mbut then route it as speaker (last_spk,push_spk). Likely should beagc_s.Apply this diff if that’s the case:
- st.agc_m.process(&mut c.data); + st.agc_s.process(&mut c.data);plugins/notification/src/lib.rs (2)
1-1: LGTM: Necessary trait importBringing
FromStrinto scope enables the UFCS callHyprWindow::from_str(...).Mutexis used bySharedState. No unused imports here.
125-126: Confirm unwrap safety across platformsAssuming
clear_notifications()is always available and returnsOk(())(no‑op on non‑macOS),unwrap()is safe. Please confirm this invariant.plugins/notification/src/handler.rs (2)
12-12: LGTM! Clear notification trigger added.The new
Clearvariant provides a clean way to dismiss all notifications through the existing trigger system.
64-66: LGTM! Clear handling properly integrated.The handler correctly dispatches to
hypr_notification::clear()without any precondition checks, which is appropriate for an explicit clear action.plugins/notification/src/ext.rs (2)
10-10: LGTM! API extension for clearing notifications.The new
clear_notificationsmethod follows the existing trait pattern and provides consistent error handling.
51-55: LGTM! Simple delegation to platform-specific implementation.The implementation correctly delegates to
hypr_notification::clear()with proper tracing instrumentation.crates/audio/src/mixed/macos.rs (1)
108-108: LGTM! Comment removal improves code clarity.Removing the redundant comment makes the aggregate device configuration more concise.
plugins/listener/src/actors/source.rs (3)
76-76: Good addition of silence stream management.Moving the silence stream creation into
pre_startand storing the sender in state ensures proper lifecycle management.
80-80: LGTM! Helpful debug instrumentation added.The tracing statements for
mic_deviceanduse_mixedwill aid in debugging audio routing issues.Also applies to: 180-180
155-155: Proper cleanup in post_stop.Good practice to explicitly drop the silence stream sender during shutdown.
crates/notification/src/lib.rs (2)
43-46: LGTM! Platform-specific clear implementation.The macOS implementation correctly delegates to
hypr_notification_macos::dismiss_all().
51-52: LGTM! No-op for non-macOS platforms.The empty implementation is appropriate for platforms that don't support notification management.
crates/notification-macos/examples/test_notification.rs (1)
50-55: LGTM! Test handlers demonstrate lifecycle callbacks.The example properly demonstrates how to use the new notification lifecycle handlers for both confirmation and dismissal events.
crates/notification-macos/src/lib.rs (4)
1-3: LGTM! Necessary imports for FFI.The added imports for C string handling are required for the new FFI callback functions.
18-19: Thread-safe storage for callbacks well implemented.Using
Mutexensures thread-safe access to the callback handlers.
21-33: Clean API for lifecycle handlers.The setup functions provide a clear interface for registering notification event handlers with appropriate trait bounds.
76-80: LGTM! Clean wrapper for dismissing all notifications.The function properly wraps the Swift FFI call with appropriate unsafe block.
crates/notification-macos/swift-lib/src/lib.swift (6)
21-21: LGTM! Consistent dismissal reason for timeout events.The timeout dismissal now properly identifies itself with the "timeout" reason, which enables better tracking of how notifications are being dismissed.
27-36: LGTM! Well-implemented dismissal callback mechanism.The implementation properly invokes the Rust callback with both notification ID and dismissal reason before animating the notification dismissal. The use of
withCStringensures safe C string interop.
137-145: Confirm callback should precede URL opening for proper event ordering.The current implementation calls
rustOnNotificationConfirmbefore opening the URL, which is correct. This ensures that the confirm event is registered even if the URL opening fails or takes time.
218-218: LGTM! Appropriate dismissal reason for close button.The "close" reason clearly identifies user-initiated dismissals via the close button.
648-654: LGTM! Consistent handling for action button clicks.The action button properly triggers the confirm callback before opening the URL and dismissing with "click" reason, maintaining consistency with the clickable view behavior.
826-830: LGTM! Clean implementation of dismiss-all functionality.The
_dismissAllNotificationsfunction provides a simple and effective way to clear all active notifications, properly exposing it through the FFI boundary.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/desktop/src-tauri/src/ext.rs(1 hunks)crates/notification-macos/examples/test_notification.rs(1 hunks)crates/notification-macos/src/lib.rs(2 hunks)crates/notification-macos/swift-lib/src/lib.swift(12 hunks)crates/notification/src/lib.rs(2 hunks)plugins/notification/src/ext.rs(4 hunks)plugins/notification/src/lib.rs(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- plugins/notification/src/lib.rs
- crates/notification/src/lib.rs
- crates/notification-macos/examples/test_notification.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
plugins/notification/src/ext.rsapps/desktop/src-tauri/src/ext.rscrates/notification-macos/src/lib.rs
🧬 Code graph analysis (2)
plugins/notification/src/ext.rs (2)
crates/notification/src/lib.rs (3)
clear(51-54)setup_notification_confirm_handler(64-70)setup_notification_dismiss_handler(56-62)crates/notification-macos/src/lib.rs (2)
setup_notification_confirm_handler(28-33)setup_notification_dismiss_handler(21-26)
crates/notification-macos/src/lib.rs (1)
crates/notification/src/lib.rs (2)
setup_notification_dismiss_handler(56-62)setup_notification_confirm_handler(64-70)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci (macos, macos-14)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (11)
plugins/notification/src/ext.rs (3)
10-10: API addition looks good.Exposing clear_notifications on the trait makes the macOS dismiss-all reachable from app code.
31-32: Analytics start/stop API surface is reasonable.Naming is consistent with existing start_/stop_ pattern.
54-58: clear_notifications implementation is fine.Directly delegates to hypr_notification::clear and returns Ok(()).
apps/desktop/src-tauri/src/ext.rs (1)
117-120: Verify plugin init order and OS behavior for analytics start.
- Ensure tauri_plugin_notification is initialized before this call; otherwise state::() inside the plugin may panic during startup.
- Call is unconditional (also on non‑macOS). That’s fine given the macOS‑gated internals, but please confirm this runs after plugin setup in all modes.
crates/notification-macos/src/lib.rs (2)
72-76: dismiss_all wrapper LGTM.Matches the Swift @_cdecl and provides a simple Rust API surface.
18-55: Don’t invoke callbacks while holding the Mutex; avoid UTF‑8 unwrap panics.Holding CONFIRM_CB/DISMISS_CB locks during cb(id) risks deadlocks and blocks Swift’s main thread; to_str().unwrap() can also panic on invalid UTF‑8. Clone an Arc out of the lock, drop the lock, then invoke; use to_string_lossy.
-use std::sync::Mutex; +use std::sync::{Arc, Mutex}; @@ -static CONFIRM_CB: Mutex<Option<Box<dyn Fn(String) + Send + Sync>>> = Mutex::new(None); -static DISMISS_CB: Mutex<Option<Box<dyn Fn(String) + Send + Sync>>> = Mutex::new(None); +static CONFIRM_CB: Mutex<Option<Arc<dyn Fn(String) + Send + Sync>>> = Mutex::new(None); +static DISMISS_CB: Mutex<Option<Arc<dyn Fn(String) + Send + Sync>>> = Mutex::new(None); @@ pub fn setup_notification_dismiss_handler<F>(f: F) where F: Fn(String) + Send + Sync + 'static, { - *DISMISS_CB.lock().unwrap() = Some(Box::new(f)); + *DISMISS_CB.lock().unwrap() = Some(Arc::new(f)); } @@ pub fn setup_notification_confirm_handler<F>(f: F) where F: Fn(String) + Send + Sync + 'static, { - *CONFIRM_CB.lock().unwrap() = Some(Box::new(f)); + *CONFIRM_CB.lock().unwrap() = Some(Arc::new(f)); } @@ #[no_mangle] pub extern "C" fn rust_on_notification_confirm(id_ptr: *const c_char) { - if let Some(cb) = CONFIRM_CB.lock().unwrap().as_ref() { - let id = unsafe { CStr::from_ptr(id_ptr) } - .to_str() - .unwrap() - .to_string(); - cb(id); - } + let cb = CONFIRM_CB.lock().unwrap().as_ref().cloned(); + if let Some(cb) = cb { + let id = unsafe { CStr::from_ptr(id_ptr) }.to_string_lossy().into_owned(); + cb(id); + } } @@ #[no_mangle] pub extern "C" fn rust_on_notification_dismiss(id_ptr: *const c_char) { - if let Some(cb) = DISMISS_CB.lock().unwrap().as_ref() { - let id = unsafe { CStr::from_ptr(id_ptr) } - .to_str() - .unwrap() - .to_string(); - cb(id); - } + let cb = DISMISS_CB.lock().unwrap().as_ref().cloned(); + if let Some(cb) = cb { + let id = unsafe { CStr::from_ptr(id_ptr) }.to_string_lossy().into_owned(); + cb(id); + } }crates/notification-macos/swift-lib/src/lib.swift (5)
41-46: Good: user‑initiated dismiss emits a Rust callback before closing.This cleanly separates user vs programmatic dismiss paths.
139-146: Confirm and then dismiss on user actions—LGTM.ClickableView and ActionButton both notify Rust (confirm) prior to dismissal; CloseButton routes through dismissWithUserAction.
Also applies to: 647-656, 219-219
159-161: UI retune cluster looks consistent.Sizing/typography reductions and spacing adjustments are coherent across components.
Also applies to: 294-294, 571-571, 576-579, 585-587, 591-591, 599-599, 610-610
728-730: Window fronting: verify focus behavior.Using orderFrontRegardless + makeKeyAndOrderFront on a non‑activating panel can still influence focus in edge cases. Please verify it doesn’t steal focus and that auto‑dismiss‑on‑main‑window‑focus still triggers as intended. If needed, prefer only orderFrontRegardless().
828-832: FFI dismissAll bridge LGTM.Matches Rust swift!(fn _dismiss_all_notifications) signature.
| fn start_notification_analytics(&self, user_id: String) -> Result<(), Error> { | ||
| let state = self.state::<crate::SharedState>(); | ||
| let mut guard = state.lock().unwrap(); | ||
|
|
||
| let analytics_task = { | ||
| use hypr_notification::NotificationMutation; | ||
| use tauri_plugin_analytics::{AnalyticsPayload, AnalyticsPluginExt}; | ||
|
|
||
| let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel::<NotificationMutation>(); | ||
| let app_handle = self.app_handle().clone(); | ||
|
|
||
| let confirm_tx = tx.clone(); | ||
| hypr_notification::setup_notification_confirm_handler(move |_id| { | ||
| confirm_tx.send(NotificationMutation::Confirm).unwrap(); | ||
| }); | ||
|
|
||
| let dismiss_tx = tx.clone(); | ||
| hypr_notification::setup_notification_dismiss_handler(move |_id| { | ||
| dismiss_tx.send(NotificationMutation::Dismiss).unwrap(); | ||
| }); | ||
|
|
||
| tokio::spawn(async move { | ||
| while let Some(event) = rx.recv().await { | ||
| match event { | ||
| NotificationMutation::Confirm => { | ||
| let _ = app_handle | ||
| .event( | ||
| AnalyticsPayload::for_user(user_id.clone()) | ||
| .event("notification_confirm") | ||
| .build(), | ||
| ) | ||
| .await; | ||
| } | ||
| NotificationMutation::Dismiss => { | ||
| let _ = app_handle | ||
| .event( | ||
| AnalyticsPayload::for_user(user_id.clone()) | ||
| .event("notification_dismiss") | ||
| .build(), | ||
| ) | ||
| .await; | ||
| } | ||
| } | ||
| } | ||
| }) | ||
| }; | ||
|
|
||
| guard.analytics_task = Some(analytics_task); | ||
| Ok(()) |
There was a problem hiding this comment.
🧩 Analysis chain
Avoid panics on channel send; abort any prior task; don’t hold the state lock while registering callbacks/spawning.
- tx.send(...).unwrap() will panic once stop_notification_analytics drops rx.
- start_notification_analytics overwrites guard.analytics_task without aborting an existing task → orphaned task and leaked receiver.
- Registering callbacks and spawning while holding the SharedState lock is unnecessary and can prolong the critical section.
Apply this diff:
fn start_notification_analytics(&self, user_id: String) -> Result<(), Error> {
- let state = self.state::<crate::SharedState>();
- let mut guard = state.lock().unwrap();
+ let state = self.state::<crate::SharedState>();
let analytics_task = {
use hypr_notification::NotificationMutation;
use tauri_plugin_analytics::{AnalyticsPayload, AnalyticsPluginExt};
let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel::<NotificationMutation>();
let app_handle = self.app_handle().clone();
let confirm_tx = tx.clone();
hypr_notification::setup_notification_confirm_handler(move |_id| {
- confirm_tx.send(NotificationMutation::Confirm).unwrap();
+ let _ = confirm_tx.send(NotificationMutation::Confirm);
});
let dismiss_tx = tx.clone();
hypr_notification::setup_notification_dismiss_handler(move |_id| {
- dismiss_tx.send(NotificationMutation::Dismiss).unwrap();
+ let _ = dismiss_tx.send(NotificationMutation::Dismiss);
});
tokio::spawn(async move {
while let Some(event) = rx.recv().await {
match event {
NotificationMutation::Confirm => {
let _ = app_handle
.event(
- AnalyticsPayload::for_user(user_id.clone())
+ AnalyticsPayload::for_user(&user_id)
.event("notification_confirm")
.build(),
)
.await;
}
NotificationMutation::Dismiss => {
let _ = app_handle
.event(
- AnalyticsPayload::for_user(user_id.clone())
+ AnalyticsPayload::for_user(&user_id)
.event("notification_dismiss")
.build(),
)
.await;
}
}
}
})
};
- guard.analytics_task = Some(analytics_task);
+ let mut guard = state.lock().unwrap();
+ if let Some(h) = guard.analytics_task.take() {
+ h.abort();
+ }
+ guard.analytics_task = Some(analytics_task);
Ok(())
}Please confirm AnalyticsPayload::for_user accepts &str (borrow) so we can avoid cloning per above.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching repository for AnalyticsPayload / AnalyticsPluginExt / tauri_plugin_analytics / for_user..."
rg -n --hidden --no-ignore 'AnalyticsPayload|AnalyticsPluginExt|tauri_plugin_analytics|for_user\(' -S || true
echo
echo "Show files that mention AnalyticsPayload (if any) with context..."
rg -n --hidden --no-ignore 'AnalyticsPayload' -S -C3 || true
echo
echo "Show Cargo.toml and workspace Cargo.toml (first 200 lines) for dependency lookups..."
if [ -f Cargo.toml ]; then
sed -n '1,200p' Cargo.toml
else
echo "No top-level Cargo.toml found"
fi
for f in $(rg -n --hidden --no-ignore 'tauri_plugin_analytics' -S -l || true); do
echo
echo "---- Showing $f (first 200 lines) ----"
sed -n '1,200p' "$f"
doneLength of output: 46916
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find SharedState definition and analytics_task field..."
rg -n --hidden --no-ignore "struct SharedState|enum SharedState|type SharedState" -S || true
echo
echo "Show files mentioning 'analytics_task' or '.analytics_task' with context..."
rg -n --hidden --no-ignore '\.analytics_task\b|analytics_task\b' -S -C3 || true
echo
echo "Show definition of crates/analytics AnalyticsPayload::for_user..."
sed -n '1,240p' crates/analytics/src/lib.rs || true
echo
echo "Show the start_notification_analytics function region (plugins/notification/src/ext.rs) for full context..."
sed -n '1,400p' plugins/notification/src/ext.rs || trueLength of output: 17746
Avoid panics on channel send; abort previous analytics task; don't hold SharedState lock while registering/spawning
- Replace confirm_tx.send(...).unwrap() / dismiss_tx.send(...).unwrap() with let _ = confirm_tx.send(...) / let _ = dismiss_tx.send(...); send can fail when rx is closed.
- Create the channel, register hypr_notification handlers, and spawn the tokio task outside the SharedState lock; only lock to abort any existing task and set guard.analytics_task.
- Before assigning guard.analytics_task, abort any existing handle: if let Some(h) = guard.analytics_task.take() { h.abort(); }.
- AnalyticsPayload::for_user has signature pub fn for_user(user_id: impl Into) — passing &user_id is accepted (apps/desktop uses for_user(&user_id)); use AnalyticsPayload::for_user(&user_id) inside the task to avoid per-event cloning.
- Make stop_notification_analytics robust: replace guard.analytics_task.take().unwrap().abort() with the conditional abort above to avoid panics.
🤖 Prompt for AI Agents
In plugins/notification/src/ext.rs around lines 218 to 266, avoid panics and
holding the SharedState lock while registering/spawning: create the unbounded
channel, register hypr_notification confirm/dismiss handlers and spawn the tokio
task outside the SharedState lock; inside the handlers replace
confirm_tx.send(...).unwrap() / dismiss_tx.send(...).unwrap() with let _ =
confirm_tx.send(...) / let _ = dismiss_tx.send(...) so failed sends don't panic;
when setting guard.analytics_task, first abort any existing task with if let
Some(h) = guard.analytics_task.take() { h.abort(); } and then assign the new
handle; inside the spawned loop use AnalyticsPayload::for_user(&user_id)
(borrow) to avoid cloning per-event; also update stop_notification_analytics to
conditionally take() and abort the task instead of unwrapping to prevent panics.
1 participant
No description provided.