Skip to content

Commit 951ea4b

Browse files
AzimovSclaude
andcommitted
fix(detection): address code-review P1s + P2s (001, 002, 004, 005)
- set_recording moved to Arc<AtomicBool> on DetectionService; poll loop syncs it into state before each advance. Removes the try_lock/spawn fallback that could reorder rapid start/stop calls and wrongly gate the MeetingEnded banner. - DetectorPhaseSnapshot.bundle_id exposed for non-idle phases so agents can act on get_detection_state alone (dismiss_detected_meeting requires the raw bundle_id). - meeting-detected / meeting-ended Tauri events gated on the matching notification preference and emit DetectedMeetingEvent { display_name } only. bundle_id stays Rust-side; agents read it via the command surface, not the event bus. - NSMicrophoneUsageDescription updated to cover the new process- enumeration capability introduced by CoreAudio mic-holder detection. The NotificationManager OnceCell migration (003) was dropped after review: the race it addressed is narrow (racing first-callers during startup eager-init) and the fallout benign (duplicate settings-file write, idempotent delegate re-registration). Not worth ~100 LOC of churn across every notification command handler. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6869a33 commit 951ea4b

4 files changed

Lines changed: 119 additions & 26 deletions

File tree

frontend/src-tauri/Info.plist

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
<plist version="1.0">
44
<dict>
55
<key>NSMicrophoneUsageDescription</key>
6-
<string>This application needs access to your microphone to record meeting audio.</string>
6+
<string>Meetily uses your microphone to record meetings you ask it to capture, and observes which apps are currently using the microphone so it can offer to auto-record when a meeting starts.</string>
77
<key>NSScreenCaptureUsageDescription</key>
88
<string>This application needs screen recording permission to capture system audio during meetings.</string>
99
<key>NSAudioCaptureUsageDescription</key>

frontend/src-tauri/src/detection/service.rs

Lines changed: 67 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use tokio::sync::Mutex;
1818

1919
use crate::detection::signals::mic_activity;
2020
use crate::detection::state::{DetectorConfig, DetectorState};
21-
use crate::detection::types::DetectionEvent;
21+
use crate::detection::types::{DetectedMeetingEvent, DetectionEvent};
2222
use crate::notifications::commands::{
2323
show_meeting_detected_notification, show_meeting_ended_notification,
2424
NotificationManagerState,
@@ -31,13 +31,19 @@ const POLL_INTERVAL: Duration = Duration::from_secs(1);
3131
pub struct DetectionService {
3232
state: Arc<Mutex<DetectorState>>,
3333
running: Arc<AtomicBool>,
34+
/// External recording flag, pushed in from the audio layer.
35+
/// Lock-free so `set_recording` stays sync and strictly ordered
36+
/// even if the poll loop is mid-`advance()`. The poll loop
37+
/// snapshots this into `DetectorState` before each tick.
38+
is_recording: Arc<AtomicBool>,
3439
}
3540

3641
impl DetectionService {
3742
fn new(config: DetectorConfig, running: Arc<AtomicBool>) -> Self {
3843
Self {
3944
state: Arc::new(Mutex::new(DetectorState::new(config))),
4045
running,
46+
is_recording: Arc::new(AtomicBool::new(false)),
4147
}
4248
}
4349

@@ -49,24 +55,12 @@ impl DetectionService {
4955
debug!("DetectionService::shutdown signalled");
5056
}
5157

52-
/// Record that Meetily started/stopped recording. Gates the
53-
/// MeetingEnded banner inside the state machine.
58+
/// Record that Meetily started/stopped recording. Lock-free atomic
59+
/// write — safe to call from any thread, sync or async, in any
60+
/// order, without contending on the state-machine mutex.
5461
pub fn set_recording(&self, recording: bool) {
55-
// Runs from audio commands which are async; we need a blocking
56-
// lock here. This is called at most once per start/stop.
57-
if let Ok(mut guard) = self.state.try_lock() {
58-
guard.set_recording(recording);
59-
debug!("DetectionService: set_recording({})", recording);
60-
} else {
61-
// Fall back: spawn to unblock once the poll loop lock
62-
// releases. Still quick — advance() holds the lock briefly.
63-
let state = self.state.clone();
64-
tauri::async_runtime::spawn(async move {
65-
let mut guard = state.lock().await;
66-
guard.set_recording(recording);
67-
debug!("DetectionService: set_recording({}) (async fallback)", recording);
68-
});
69-
}
62+
self.is_recording.store(recording, Ordering::Release);
63+
debug!("DetectionService: set_recording({})", recording);
7064
}
7165

7266
/// Dismiss a detected bundle so further MeetingDetected banners
@@ -96,6 +90,7 @@ where
9690
let service = DetectionService::new(DetectorConfig::DEFAULT, running.clone());
9791
let state = service.state.clone();
9892
let running_for_task = running.clone();
93+
let is_recording_for_task = service.is_recording.clone();
9994

10095
let sampler = match mic_activity::create() {
10196
Ok(s) => s,
@@ -126,8 +121,12 @@ where
126121
}
127122
};
128123

124+
// Sync the externally-pushed recording flag into the state
125+
// machine right before advancing. Done under the state lock
126+
// so `advance()` sees a coherent value.
129127
let event = {
130128
let mut guard = state.lock().await;
129+
guard.set_recording(is_recording_for_task.load(Ordering::Acquire));
131130
guard.advance(Instant::now(), &snapshot)
132131
};
133132

@@ -138,10 +137,16 @@ where
138137
DetectionEvent::MeetingDetected(m) => {
139138
info!("Meeting detection: DETECTED {}", m.display_name);
140139
debug!("Meeting detection: DETECTED bundle={}", m.bundle_id);
141-
// Emit Tauri event so frontend / agents can observe
142-
// alongside the notification banner.
143-
if let Err(e) = app.emit("meeting-detected", &m) {
144-
debug!("failed to emit meeting-detected event: {}", e);
140+
// Only emit the event when the corresponding banner
141+
// preference is enabled. Keeps the event and the
142+
// user-facing banner under the same user control,
143+
// and strips `bundle_id` from the payload — agents
144+
// can still read it via `get_detection_state`.
145+
if pref_show_meeting_detected(mgr_state.inner()).await {
146+
let payload = DetectedMeetingEvent { display_name: m.display_name.clone() };
147+
if let Err(e) = app.emit("meeting-detected", &payload) {
148+
debug!("failed to emit meeting-detected event: {}", e);
149+
}
145150
}
146151
if let Err(e) = show_meeting_detected_notification(
147152
&app, mgr_state.inner(), m.display_name,
@@ -152,8 +157,11 @@ where
152157
DetectionEvent::MeetingEnded(m) => {
153158
info!("Meeting detection: ENDED {}", m.display_name);
154159
debug!("Meeting detection: ENDED bundle={}", m.bundle_id);
155-
if let Err(e) = app.emit("meeting-ended", &m) {
156-
debug!("failed to emit meeting-ended event: {}", e);
160+
if pref_show_meeting_ended(mgr_state.inner()).await {
161+
let payload = DetectedMeetingEvent { display_name: m.display_name.clone() };
162+
if let Err(e) = app.emit("meeting-ended", &payload) {
163+
debug!("failed to emit meeting-ended event: {}", e);
164+
}
157165
}
158166
if let Err(e) = show_meeting_ended_notification(
159167
&app, mgr_state.inner(), m.display_name,
@@ -169,3 +177,38 @@ where
169177

170178
service
171179
}
180+
181+
/// Read the `show_meeting_detected` preference from the live notification
182+
/// manager. Defaults to `true` if the manager isn't initialized yet so
183+
/// the first detection after startup isn't silently dropped.
184+
async fn pref_show_meeting_detected<R: Runtime>(
185+
manager_state: &NotificationManagerState<R>,
186+
) -> bool {
187+
let guard = manager_state.read().await;
188+
match guard.as_ref() {
189+
Some(manager) => {
190+
manager
191+
.get_settings()
192+
.await
193+
.notification_preferences
194+
.show_meeting_detected
195+
}
196+
None => true,
197+
}
198+
}
199+
200+
async fn pref_show_meeting_ended<R: Runtime>(
201+
manager_state: &NotificationManagerState<R>,
202+
) -> bool {
203+
let guard = manager_state.read().await;
204+
match guard.as_ref() {
205+
Some(manager) => {
206+
manager
207+
.get_settings()
208+
.await
209+
.notification_preferences
210+
.show_meeting_ended
211+
}
212+
None => true,
213+
}
214+
}

frontend/src-tauri/src/detection/state.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ pub struct DetectorPhaseSnapshot {
7575
/// One of `"idle" | "sustaining" | "detected" | "ending"`.
7676
pub phase: &'static str,
7777
pub display_name: Option<String>,
78+
/// Raw bundle ID for the current candidate — the identity
79+
/// `dismiss_detected_meeting` expects. `None` in `Idle`.
80+
pub bundle_id: Option<String>,
7881
pub elapsed_ms: Option<u64>,
7982
pub remaining_ms: Option<u64>,
8083
pub is_recording: bool,
@@ -308,6 +311,7 @@ impl DetectorState {
308311
Phase::Idle => DetectorPhaseSnapshot {
309312
phase: "idle",
310313
display_name: None,
314+
bundle_id: None,
311315
elapsed_ms: None,
312316
remaining_ms: None,
313317
is_recording: self.is_recording,
@@ -322,6 +326,7 @@ impl DetectorState {
322326
DetectorPhaseSnapshot {
323327
phase: "sustaining",
324328
display_name: Some(matcher::display_name(bundle).to_string()),
329+
bundle_id: Some(bundle.clone()),
325330
elapsed_ms: Some(elapsed.as_millis() as u64),
326331
remaining_ms: Some(threshold.saturating_sub(elapsed).as_millis() as u64),
327332
is_recording: self.is_recording,
@@ -330,6 +335,7 @@ impl DetectorState {
330335
Phase::Detected { bundle } => DetectorPhaseSnapshot {
331336
phase: "detected",
332337
display_name: Some(matcher::display_name(bundle).to_string()),
338+
bundle_id: Some(bundle.clone()),
333339
elapsed_ms: None,
334340
remaining_ms: None,
335341
is_recording: self.is_recording,
@@ -343,6 +349,7 @@ impl DetectorState {
343349
DetectorPhaseSnapshot {
344350
phase: "ending",
345351
display_name: Some(matcher::display_name(bundle).to_string()),
352+
bundle_id: Some(bundle.clone()),
346353
elapsed_ms: Some(elapsed.as_millis() as u64),
347354
remaining_ms: Some(
348355
self.config.end_silence.saturating_sub(elapsed).as_millis() as u64,
@@ -673,6 +680,38 @@ mod tests {
673680
assert!(snap.remaining_ms.unwrap() <= 7_000 && snap.remaining_ms.unwrap() > 6_500);
674681
}
675682

683+
#[test]
684+
fn phase_snapshot_exposes_bundle_id_for_non_idle_phases() {
685+
let mut s = DetectorState::new(test_config());
686+
let t0 = Instant::now();
687+
688+
// Idle → no bundle_id
689+
assert_eq!(s.phase_snapshot(t0).bundle_id, None);
690+
691+
// Sustaining → bundle_id present
692+
s.advance(t0, &snapshot(&["us.zoom.xos"]));
693+
assert_eq!(
694+
s.phase_snapshot(t0).bundle_id.as_deref(),
695+
Some("us.zoom.xos")
696+
);
697+
698+
// Detected → bundle_id present
699+
let t_detect = t0 + Duration::from_secs(10);
700+
s.advance(t_detect, &snapshot(&["us.zoom.xos"]));
701+
assert_eq!(
702+
s.phase_snapshot(t_detect).bundle_id.as_deref(),
703+
Some("us.zoom.xos")
704+
);
705+
706+
// Ending → bundle_id still present (the app we were tracking)
707+
let t_release = t_detect + Duration::from_secs(5);
708+
s.advance(t_release, &snapshot(&[]));
709+
assert_eq!(
710+
s.phase_snapshot(t_release).bundle_id.as_deref(),
711+
Some("us.zoom.xos")
712+
);
713+
}
714+
676715
#[test]
677716
fn dismissed_reaper_removes_expired_entries() {
678717
let mut s = DetectorState::new(test_config());

frontend/src-tauri/src/detection/types.rs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,23 @@
11
use serde::{Deserialize, Serialize};
22

3-
/// A meeting detected by the mic-activity signal.
3+
/// A meeting detected by the mic-activity signal. Used internally by
4+
/// the detector; not emitted on the Tauri event bus — see
5+
/// [`DetectedMeetingEvent`] for the public wire shape.
46
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
57
pub struct DetectedMeeting {
68
pub bundle_id: String,
79
pub display_name: String,
810
}
911

12+
/// Public payload for `meeting-detected` / `meeting-ended` Tauri events.
13+
/// Omits `bundle_id` so Tauri events don't stream app-identity
14+
/// telemetry to every webview. Agents needing `bundle_id` can read it
15+
/// via the `get_detection_state` command.
16+
#[derive(Debug, Clone, Serialize)]
17+
pub struct DetectedMeetingEvent {
18+
pub display_name: String,
19+
}
20+
1021
/// Events the detector emits as its internal state advances.
1122
#[derive(Debug, Clone, PartialEq, Eq)]
1223
pub enum DetectionEvent {

0 commit comments

Comments
 (0)