Skip to content

Commit 7321a92

Browse files
pmaxhoganclaude
andcommitted
fix(app): no-cache Unavailable crypto + per-account notify dedup + cancellable pause + deep-link scheme
V5-P2-1 / C5-P2-2: KeystoreCryptoProvider no longer caches the CryptoResolution::Unavailable verdict (a transient keychain-locked / missing-key condition). It caches only the stable Plaintext + Suite verdicts; Unavailable is returned without memoizing so the next op re-attempts the unwrap once the keychain unlocks. Fail-closed is preserved. V5-P2-2 / C5-P2-4: tray notification dedup is now per-account (Mutex<Option<HashMap<AccountId, NotifyState>>>); account_id is threaded from the assembly event bridge into apply_state/notify_for_state so first_sync_notified + last_error_code are per account (one account's first-sync toast no longer silences another's; cross-account errors are no longer suppressed). C5-P2-1: the timed-pause auto-resume timer is now cancellable via a per-account pause-generation token on AppState. A newer pause/resume bumps the generation; the detached timer auto-resumes only if the token still matches, so a pause(None) issued before the old timer fires is no longer clobbered. C5-P2-3: declare the `driven` scheme under plugins.deep-link.desktop.schemes in tauri.conf.json and drain any cold-start deep link via app.deep_link().get_current() in setup. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CyiRqk2DVwmJjEu5gcD1m
1 parent 3e1c8d8 commit 7321a92

4 files changed

Lines changed: 118 additions & 67 deletions

File tree

src-tauri/src/commands/sync.rs

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ use std::sync::Arc;
1010
use std::time::Duration;
1111

1212
use serde::{Deserialize, Serialize};
13-
use tauri::State;
13+
use tauri::{AppHandle, Manager, State};
1414

1515
use driven_core::orchestrator::{Orchestrator, TickSource};
16-
use driven_core::types::{OrchestratorState, SourceId};
16+
use driven_core::types::{AccountId, OrchestratorState, SourceId};
1717

1818
use crate::app_state::AppState;
1919
use crate::commands::{CommandError, CommandResult};
@@ -90,38 +90,56 @@ pub async fn sync_now(
9090
/// `duration_secs = Some` is a timed pause (e.g. the tray "Pause for 30m");
9191
/// `None` is pause-until-manual-resume. Sets the manual-pause signal on every
9292
/// account orchestrator (DESIGN s5.7: manual pause persists across restarts).
93-
/// For a timed pause, a detached timer flips the pause back off after the
94-
/// window. If the user manually resumes (or re-pauses) before the timer fires,
95-
/// the timer's `set_paused(false)` is a harmless idempotent re-assert of the
96-
/// already-cleared signal; a fresh `pause_sync(None)` after the timer is armed
97-
/// is NOT auto-cancelled here (an accepted V1 simplicity - the rare
98-
/// timed-then-indefinite race re-pauses on the next user action / restart,
99-
/// which loads the persisted manual-pause).
93+
///
94+
/// C5-P2-1: a timed pause spawns a CANCELLABLE auto-resume timer. Each
95+
/// pause/resume bumps a per-account pause "generation"; the timer captures the
96+
/// generation at arm time and only auto-resumes if it STILL matches when it
97+
/// wakes. So a later `pause_sync(None)` (indefinite) issued before the old
98+
/// timer fires bumps the generation and CANCELS the stale timer's auto-resume -
99+
/// the indefinite pause is no longer clobbered.
100100
#[tauri::command]
101101
pub async fn pause_sync(
102+
app: AppHandle,
102103
state: State<'_, AppState>,
103104
duration_secs: Option<u64>,
104105
) -> CommandResult<()> {
105-
// Snapshot the orchestrator handles up front so the resume timer does not
106-
// need to borrow `State` (which is not `'static`).
107-
let orchestrators: Vec<Arc<dyn Orchestrator>> = state
106+
// Snapshot (account_id, orchestrator) so the resume timer does not need to
107+
// borrow `State` (which is not `'static`); bump each account's pause
108+
// generation so any in-flight timer is superseded.
109+
let entries: Vec<(AccountId, Arc<dyn Orchestrator>)> = state
108110
.accounts()
109-
.map(|(_id, handle)| handle.orchestrator.clone())
111+
.map(|(id, handle)| (*id, handle.orchestrator.clone()))
110112
.collect();
111113

112-
for orch in &orchestrators {
114+
let mut tokens: Vec<(AccountId, Arc<dyn Orchestrator>, u64)> =
115+
Vec::with_capacity(entries.len());
116+
for (id, orch) in entries {
113117
orch.set_paused(true).await;
118+
let token = state.bump_pause_generation(id);
119+
tokens.push((id, orch, token));
114120
}
115121

116122
if let Some(secs) = duration_secs {
117123
// Detached timed-resume: sleep the window, then clear the manual pause
118-
// on each account. `tokio::time::sleep` (no FakeClock here - this is a
119-
// real wall-clock UI affordance) keeps the task off the IPC path so the
124+
// ONLY for accounts whose pause generation is unchanged (no newer
125+
// pause/resume superseded this timer). `tokio::time::sleep` (real
126+
// wall-clock UI affordance) keeps the task off the IPC path so the
120127
// command returns immediately.
121128
tauri::async_runtime::spawn(async move {
122129
tokio::time::sleep(Duration::from_secs(secs)).await;
123-
for orch in &orchestrators {
124-
orch.set_paused(false).await;
130+
let Some(state) = app.try_state::<AppState>() else {
131+
return;
132+
};
133+
for (id, orch, token) in &tokens {
134+
if state.pause_generation_matches(*id, *token) {
135+
orch.set_paused(false).await;
136+
} else {
137+
tracing::debug!(
138+
target: "driven::app::sync",
139+
account_id = %id,
140+
"timed-resume superseded by a newer pause/resume; not auto-resuming"
141+
);
142+
}
125143
}
126144
});
127145
}
@@ -130,10 +148,15 @@ pub async fn pause_sync(
130148
}
131149

132150
/// `resume_sync()` - clear the manual pause on every account (SPEC s11.3).
151+
///
152+
/// C5-P2-1: bumps each account's pause generation too, so an outstanding timed
153+
/// auto-resume timer for that account is cancelled (the manual resume already
154+
/// did its job; the stale timer must not later re-resume a fresh pause).
133155
#[tauri::command]
134156
pub async fn resume_sync(state: State<'_, AppState>) -> CommandResult<()> {
135-
for (_id, handle) in state.accounts() {
157+
for (id, handle) in state.accounts() {
136158
handle.orchestrator.set_paused(false).await;
159+
let _ = state.bump_pause_generation(*id);
137160
}
138161
Ok(())
139162
}

src-tauri/src/crypto_provider_impl.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,17 @@ impl KeystoreCryptoProvider {
114114

115115
let resolution = Arc::new(self.resolve_uncached(source_id));
116116

117+
// V5-P2-1 / C5-P2-2: cache ONLY the STABLE verdicts (Plaintext, Suite).
118+
// Do NOT memoize `Unavailable`: it is a TRANSIENT condition (keychain /
119+
// Secret-Service locked at autostart, a temporarily missing key). Caching
120+
// it would strand an encrypted source as un-backupable until the app
121+
// restarts, even after the keychain unlocks. Returning it WITHOUT caching
122+
// makes the next op re-attempt the unwrap (fail-closed is preserved -
123+
// the op still errors `crypto.key_missing` until the key is available).
124+
if matches!(*resolution, CachedResolution::Unavailable) {
125+
return resolution;
126+
}
127+
117128
// Store under the lock. A concurrent resolver for the same id may have
118129
// raced us; keep whichever landed first (both compute the same verdict
119130
// from the same immutable row + keystore, so either is correct).

src-tauri/src/tray.rs

Lines changed: 58 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,10 @@
2626
//! APPROXIMATES the yellow-with-`!` badge - no glyph is drawn into the tile.
2727
//! The state machine, tooltip text, and notification routing are all real.
2828
29+
use std::collections::HashMap;
2930
use std::sync::Mutex;
3031

31-
use driven_core::types::{ErrorCode, OrchestratorState, PauseReason};
32+
use driven_core::types::{AccountId, ErrorCode, OrchestratorState, PauseReason};
3233
use tauri::image::Image;
3334
use tauri::menu::{Menu, MenuBuilder, MenuItem};
3435
use tauri::tray::{MouseButton, MouseButtonState, TrayIconBuilder, TrayIconEvent};
@@ -274,10 +275,15 @@ fn tooltip_for_error(code: ErrorCode) -> String {
274275
// Notification dedup state
275276
// -----------------------------------------------------------------------------
276277

277-
/// Module-level notification dedup state (DESIGN s117/s247): fire the
278-
/// first-sync-complete toast exactly once, and fire one error toast per
279-
/// ENTRY into an error code (not once per `StateChanged` event - the
280-
/// orchestrator broadcast can replay the current state after a `Lagged`).
278+
/// Per-account notification dedup state (DESIGN s117/s247): fire the
279+
/// first-sync-complete toast exactly once PER ACCOUNT, and fire one error toast
280+
/// per ENTRY into an error code PER ACCOUNT (not once per `StateChanged` event -
281+
/// the orchestrator broadcast can replay the current state after a `Lagged`).
282+
///
283+
/// V5-P2-2 / C5-P2-4: keyed per account so one account's first-sync toast does
284+
/// not silence another's, and an error on account B is not suppressed by the
285+
/// same code already toasted for account A.
286+
#[derive(Default)]
281287
struct NotifyState {
282288
/// True once a sync cycle has been observed running this process (so the
283289
/// next `Idle` transition is a genuine completion, not the boot `Idle`).
@@ -289,22 +295,17 @@ struct NotifyState {
289295
last_error_code: Option<ErrorCode>,
290296
}
291297

292-
impl NotifyState {
293-
const fn new() -> Self {
294-
Self {
295-
saw_active_cycle: false,
296-
first_sync_notified: false,
297-
last_error_code: None,
298-
}
299-
}
300-
}
301-
302-
static NOTIFY: Mutex<NotifyState> = Mutex::new(NotifyState::new());
303-
304-
/// Lock the dedup state, recovering a poisoned lock (HARD RULE: no panic on
305-
/// a poisoned mutex).
306-
fn notify_state() -> std::sync::MutexGuard<'static, NotifyState> {
307-
NOTIFY.lock().unwrap_or_else(|e| e.into_inner())
298+
/// Process-global map of per-account dedup state. Keyed by [`AccountId`] so the
299+
/// dedup latches are independent across accounts (V5-P2-2).
300+
static NOTIFY: Mutex<Option<HashMap<AccountId, NotifyState>>> = Mutex::new(None);
301+
302+
/// Run `f` against the dedup state for `account`, creating it on first use.
303+
/// Recovers a poisoned lock instead of panicking (HARD RULE).
304+
fn with_notify_state<R>(account: AccountId, f: impl FnOnce(&mut NotifyState) -> R) -> R {
305+
let mut guard = NOTIFY.lock().unwrap_or_else(|e| e.into_inner());
306+
let map = guard.get_or_insert_with(HashMap::new);
307+
let entry = map.entry(account).or_default();
308+
f(entry)
308309
}
309310

310311
// -----------------------------------------------------------------------------
@@ -396,7 +397,7 @@ fn on_menu_event(app: &AppHandle, id: &str) {
396397
let Some(state) = app.try_state::<crate::app_state::AppState>() else {
397398
return missing_state_err();
398399
};
399-
crate::commands::sync::pause_sync(state, Some(30 * 60)).await
400+
crate::commands::sync::pause_sync(app.clone(), state, Some(30 * 60)).await
400401
}),
401402
menu_id::RESUME => spawn_command(app, |app| async move {
402403
let Some(state) = app.try_state::<crate::app_state::AppState>() else {
@@ -479,7 +480,7 @@ fn navigate_hint(app: &AppHandle, route: &str) {
479480
/// Best-effort: a missing tray or a failed icon/tooltip set is logged, never
480481
/// panicked. `apply_state` returns `()` (the committed signature) so all
481482
/// errors are swallowed with a `tracing` line.
482-
pub fn apply_state(app: &AppHandle, state: OrchestratorState) {
483+
pub fn apply_state(app: &AppHandle, account_id: AccountId, state: OrchestratorState) {
483484
let icon = TrayIcon::for_state(&state);
484485

485486
if let Some(tray) = app.tray_by_id(TRAY_ID) {
@@ -497,7 +498,7 @@ pub fn apply_state(app: &AppHandle, state: OrchestratorState) {
497498
tracing::warn!(target: TARGET, "tray {TRAY_ID} not found; cannot apply state");
498499
}
499500

500-
notify_for_state(app, &state);
501+
notify_for_state(app, account_id, &state);
501502
}
502503

503504
/// Raise the DESIGN s117/s247 OS notifications for a state transition.
@@ -509,7 +510,7 @@ pub fn apply_state(app: &AppHandle, state: OrchestratorState) {
509510
/// case (`auth.invalid_grant` / `auth.consent_required`) is deliberately
510511
/// skipped here - it is covered by [`notify_needs_reauth`], which the shell
511512
/// calls with the account + email the state cannot carry.
512-
fn notify_for_state(app: &AppHandle, state: &OrchestratorState) {
513+
fn notify_for_state(app: &AppHandle, account_id: AccountId, state: &OrchestratorState) {
513514
match state {
514515
OrchestratorState::PowerCheck
515516
| OrchestratorState::Scanning { .. }
@@ -518,19 +519,23 @@ fn notify_for_state(app: &AppHandle, state: &OrchestratorState) {
518519
| OrchestratorState::Verifying { .. }
519520
| OrchestratorState::Backoff { .. } => {
520521
// A cycle is underway; the next Idle is a genuine completion.
521-
let mut s = notify_state();
522-
s.saw_active_cycle = true;
523-
// Leaving any error state clears the dedup latch so a recurrence
524-
// notifies again.
525-
s.last_error_code = None;
522+
with_notify_state(account_id, |s| {
523+
s.saw_active_cycle = true;
524+
// Leaving any error state clears the dedup latch so a recurrence
525+
// notifies again.
526+
s.last_error_code = None;
527+
});
526528
}
527529
OrchestratorState::Idle { .. } => {
528-
let mut s = notify_state();
529-
s.last_error_code = None;
530-
let should_fire = s.saw_active_cycle && !s.first_sync_notified;
530+
let should_fire = with_notify_state(account_id, |s| {
531+
s.last_error_code = None;
532+
let fire = s.saw_active_cycle && !s.first_sync_notified;
533+
if fire {
534+
s.first_sync_notified = true;
535+
}
536+
fire
537+
});
531538
if should_fire {
532-
s.first_sync_notified = true;
533-
drop(s);
534539
show_notification(
535540
app,
536541
rust_i18n::t!("notifications.first_sync_complete.title").into_owned(),
@@ -541,8 +546,9 @@ fn notify_for_state(app: &AppHandle, state: &OrchestratorState) {
541546
OrchestratorState::Paused { .. } => {
542547
// Pauses (battery / metered / network) are icon+tooltip only, no
543548
// toast on every blip (DESIGN s117/s247).
544-
let mut s = notify_state();
545-
s.last_error_code = None;
549+
with_notify_state(account_id, |s| {
550+
s.last_error_code = None;
551+
});
546552
}
547553
OrchestratorState::Error { detail } => {
548554
// Reauth is handled by notify_needs_reauth (needs account/email).
@@ -557,18 +563,22 @@ fn notify_for_state(app: &AppHandle, state: &OrchestratorState) {
557563
if TrayIcon::for_state(state) != TrayIcon::Error {
558564
return;
559565
}
560-
let mut s = notify_state();
561-
if s.last_error_code == Some(detail.code) {
562-
return; // already toasted this error; suppress the replay
566+
let should_fire = with_notify_state(account_id, |s| {
567+
if s.last_error_code == Some(detail.code) {
568+
false // already toasted this error; suppress the replay
569+
} else {
570+
s.last_error_code = Some(detail.code);
571+
true
572+
}
573+
});
574+
if should_fire {
575+
let body = error_notification_body(detail.code);
576+
show_notification(
577+
app,
578+
rust_i18n::t!("notifications.error.title").into_owned(),
579+
body,
580+
);
563581
}
564-
s.last_error_code = Some(detail.code);
565-
drop(s);
566-
let body = error_notification_body(detail.code);
567-
show_notification(
568-
app,
569-
rust_i18n::t!("notifications.error.title").into_owned(),
570-
body,
571-
);
572582
}
573583
}
574584
}

src-tauri/tauri.conf.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,13 @@
3131
"csp": "default-src 'self'; img-src 'self' data:; connect-src 'self' ipc: tauri:"
3232
}
3333
},
34+
"plugins": {
35+
"deep-link": {
36+
"desktop": {
37+
"schemes": ["driven"]
38+
}
39+
}
40+
},
3441
"bundle": {
3542
"active": true,
3643
"createUpdaterArtifacts": true,

0 commit comments

Comments
 (0)